]> granicus.if.org Git - postgresql/commitdiff
Fix unsafe references to errno within error messaging logic.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 30 Jan 2014 01:04:11 +0000 (20:04 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 30 Jan 2014 01:04:11 +0000 (20:04 -0500)
Various places were supposing that errno could be expected to hold still
within an ereport() nest or similar contexts.  This isn't true necessarily,
though in some cases it accidentally failed to fail depending on how the
compiler chanced to order the subexpressions.  This class of thinko
explains recent reports of odd failures on clang-built versions, typically
missing or inappropriate HINT fields in messages.

Problem identified by Christian Kruse, who also submitted the patch this
commit is based on.  (I fixed a few issues in his patch and found a couple
of additional places with the same disease.)

Back-patch as appropriate to all supported branches.

src/backend/commands/tablespace.c
src/backend/port/sysv_sema.c
src/backend/port/sysv_shmem.c

index aab665901673ae84238bf88d6b98e404f956ccfe..45c81299abb13d1bb67fad946c66d64d1f6765d8 100644 (file)
@@ -764,10 +764,14 @@ remove_symlink:
        else
        {
                if (unlink(linkloc) < 0)
-                       ereport(redo ? LOG : (errno == ENOENT ? WARNING : ERROR),
+               {
+                       int                     saved_errno = errno;
+
+                       ereport(redo ? LOG : (saved_errno == ENOENT ? WARNING : ERROR),
                                        (errcode_for_file_access(),
                                         errmsg("could not remove symbolic link \"%s\": %m",
                                                        linkloc)));
+               }
        }
 
        pfree(linkloc_with_version_dir);
index 6ff3091dee269e0a8029cd6edfea60af255fec15..8366e0d8e7b3629a658823172e254f5749f9e1ff 100644 (file)
@@ -94,15 +94,17 @@ InternalIpcSemaphoreCreate(IpcSemaphoreKey semKey, int numSems)
 
        if (semId < 0)
        {
+               int                     saved_errno = errno;
+
                /*
                 * Fail quietly if error indicates a collision with existing set. One
                 * would expect EEXIST, given that we said IPC_EXCL, but perhaps we
                 * could get a permission violation instead?  Also, EIDRM might occur
                 * if an old set is slated for destruction but not gone yet.
                 */
-               if (errno == EEXIST || errno == EACCES
+               if (saved_errno == EEXIST || saved_errno == EACCES
 #ifdef EIDRM
-                       || errno == EIDRM
+                       || saved_errno == EIDRM
 #endif
                        )
                        return -1;
@@ -115,7 +117,7 @@ InternalIpcSemaphoreCreate(IpcSemaphoreKey semKey, int numSems)
                                 errdetail("Failed system call was semget(%lu, %d, 0%o).",
                                                   (unsigned long) semKey, numSems,
                                                   IPC_CREAT | IPC_EXCL | IPCProtection),
-                                (errno == ENOSPC) ?
+                                (saved_errno == ENOSPC) ?
                                 errhint("This error does *not* mean that you have run out of disk space.\n"
                  "It occurs when either the system limit for the maximum number of "
                         "semaphore sets (SEMMNI), or the system wide maximum number of "
@@ -141,13 +143,17 @@ IpcSemaphoreInitialize(IpcSemaphoreId semId, int semNum, int value)
 
        semun.val = value;
        if (semctl(semId, semNum, SETVAL, semun) < 0)
+       {
+               int                     saved_errno = errno;
+
                ereport(FATAL,
                                (errmsg_internal("semctl(%d, %d, SETVAL, %d) failed: %m",
                                                                 semId, semNum, value),
-                                (errno == ERANGE) ?
+                                (saved_errno == ERANGE) ?
                                 errhint("You possibly need to raise your kernel's SEMVMX value to be at least "
                                  "%d.  Look into the PostgreSQL documentation for details.",
                                                 value) : 0));
+       }
 }
 
 /*
index 3b5df03c1c1a81636c91a7479db734386e6f4f62..c4f9bd756cb5a83cdda2e1048603ba6c27998f20 100644 (file)
@@ -79,15 +79,17 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
 
        if (shmid < 0)
        {
+               int                     shmget_errno = errno;
+
                /*
                 * Fail quietly if error indicates a collision with existing segment.
                 * One would expect EEXIST, given that we said IPC_EXCL, but perhaps
                 * we could get a permission violation instead?  Also, EIDRM might
                 * occur if an old seg is slated for destruction but not gone yet.
                 */
-               if (errno == EEXIST || errno == EACCES
+               if (shmget_errno == EEXIST || shmget_errno == EACCES
 #ifdef EIDRM
-                       || errno == EIDRM
+                       || shmget_errno == EIDRM
 #endif
                        )
                        return NULL;
@@ -101,10 +103,8 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
                 * against SHMMIN in the preexisting-segment case, so we will not get
                 * EINVAL a second time if there is such a segment.
                 */
-               if (errno == EINVAL)
+               if (shmget_errno == EINVAL)
                {
-                       int                     save_errno = errno;
-
                        shmid = shmget(memKey, 0, IPC_CREAT | IPC_EXCL | IPCProtection);
 
                        if (shmid < 0)
@@ -130,8 +130,6 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
                                        elog(LOG, "shmctl(%d, %d, 0) failed: %m",
                                                 (int) shmid, IPC_RMID);
                        }
-
-                       errno = save_errno;
                }
 
                /*
@@ -143,12 +141,13 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
                 * it should be.  SHMMNI violation is ENOSPC, per spec.  Just plain
                 * not-enough-RAM is ENOMEM.
                 */
+               errno = shmget_errno;
                ereport(FATAL,
                                (errmsg("could not create shared memory segment: %m"),
                  errdetail("Failed system call was shmget(key=%lu, size=%lu, 0%o).",
                                        (unsigned long) memKey, (unsigned long) size,
                                        IPC_CREAT | IPC_EXCL | IPCProtection),
-                                (errno == EINVAL) ?
+                                (shmget_errno == EINVAL) ?
                                 errhint("This error usually means that PostgreSQL's request for a shared memory "
                  "segment exceeded your kernel's SHMMAX parameter.  You can either "
                                                 "reduce the request size or reconfigure the kernel with larger SHMMAX.  "
@@ -161,7 +160,7 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
                "The PostgreSQL documentation contains more information about shared "
                                                 "memory configuration.",
                                                 (unsigned long) size, NBuffers, MaxBackends) : 0,
-                                (errno == ENOMEM) ?
+                                (shmget_errno == ENOMEM) ?
                                 errhint("This error usually means that PostgreSQL's request for a shared "
                                   "memory segment exceeded available memory or swap space, "
                                                 "or exceeded your kernel's SHMALL parameter.  You can either "
@@ -172,7 +171,7 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
                "The PostgreSQL documentation contains more information about shared "
                                                 "memory configuration.",
                                                 (unsigned long) size, NBuffers, MaxBackends) : 0,
-                                (errno == ENOSPC) ?
+                                (shmget_errno == ENOSPC) ?
                                 errhint("This error does *not* mean that you have run out of disk space. "
                                                 "It occurs either if all available shared memory IDs have been taken, "
                                                 "in which case you need to raise the SHMMNI parameter in your kernel, "