correct locking in stdio functions that tried to be lock-free

these functions must behave as if they obtain the lock via flockfile
to satisfy POSIX requirements. since another thread can provably hold
the lock when they are called, they must wait to obtain the lock
before they can return, even if the correct return value could be
obtained without locking. in the case of fclose and freopen, failure
to do so could cause correct (albeit obscure) programs to crash or
otherwise misbehave; in the case of feof, ferror, and fwide, failure
to obtain the lock could sometimes return incorrect results. in any
case, having these functions proceed and return while another thread
held the lock was wrong.

diff --git a/src/stdio/fclose.c b/src/stdio/fclose.c
index 8fdc3f7d2b667d125e4aa7bc115d53f68e852331..92bf7ff8858dd214bca90c3961df31062cfd5a23 100644 (file)
--- a/src/stdio/fclose.c
+++ b/src/stdio/fclose.c
@@ -3,9 +3,12 @@
 int fclose(FILE *f)
 {
        int r;
-       int perm = f->flags & F_PERM;
+       int perm;
+       
+       /* This lock is not paired with any unlock. */
+       FLOCK(f);
 
-       if (!perm) {
+       if (!(perm = f->flags & F_PERM)) {
                OFLLOCK();
                if (f->prev) f->prev->next = f->next;
                if (f->next) f->next->prev = f->prev;

diff --git a/src/stdio/feof.c b/src/stdio/feof.c
index 5d7f4b02332b8825f01dd6745ed08bf4ffa66602..56da6b917c0af7913ec18f51288eccef822d763c 100644 (file)
--- a/src/stdio/feof.c
+++ b/src/stdio/feof.c
@@ -4,7 +4,10 @@
 
 int feof(FILE *f)
 {
-       return !!(f->flags & F_EOF);
+       FLOCK(f);
+       int ret = !!(f->flags & F_EOF);
+       FUNLOCK(f);
+       return ret;
 }
 
 weak_alias(feof, feof_unlocked);

diff --git a/src/stdio/ferror.c b/src/stdio/ferror.c
index 8288a93d96713babe9dc76ae5dab03a2aab14037..d692eed9418ea1f0cf36c948bacad20d6afc526c 100644 (file)
--- a/src/stdio/ferror.c
+++ b/src/stdio/ferror.c
@@ -4,7 +4,10 @@
 
 int ferror(FILE *f)
 {
-       return !!(f->flags & F_ERR);
+       FLOCK(f);
+       int ret = !!(f->flags & F_ERR);
+       FUNLOCK(f);
+       return ret;
 }
 
 weak_alias(ferror, ferror_unlocked);

diff --git a/src/stdio/fileno.c b/src/stdio/fileno.c
index 9ffb26d54d17754bc6b5bac465281a87a338564f..ba7f9391b1ec9baa6023f9a66f569b46a1f85ba2 100644 (file)
--- a/src/stdio/fileno.c
+++ b/src/stdio/fileno.c
@@ -2,6 +2,11 @@
 
 int fileno(FILE *f)
 {
+       /* f->fd never changes, but the lock must be obtained and released
+        * anyway since this function cannot return while another thread
+        * holds the lock. */
+       FLOCK(f);
+       FUNLOCK(f);
        return f->fd;
 }
 

diff --git a/src/stdio/freopen.c b/src/stdio/freopen.c
index c80ce3b438e67e7de275cd2ef1a3a0513d5e05cd..7ae116d8d245708dd3f3a6a0a1b1409793e7c5b6 100644 (file)
--- a/src/stdio/freopen.c
+++ b/src/stdio/freopen.c
@@ -4,8 +4,9 @@
  * hack the necessary parts of the new FILE into the old one, then
  * close the new FILE. */
 
-/* Locking is not necessary because, in the event of failure, the stream
- * passed to freopen is invalid as soon as freopen is called. */
+/* Locking IS necessary because another thread may provably hold the
+ * lock, via flockfile or otherwise, when freopen is called, and in that
+ * case, freopen cannot act until the lock is released. */
 
 int __dup3(int, int, int);
 
@@ -14,6 +15,8 @@ FILE *freopen(const char *restrict filename, const char *restrict mode, FILE *re
        int fl = __fmodeflags(mode);
        FILE *f2;
 
+       FLOCK(f);
+
        fflush(f);
 
        if (!filename) {
@@ -22,21 +25,22 @@ FILE *freopen(const char *restrict filename, const char *restrict mode, FILE *re
                fl &= ~(O_CREAT|O_EXCL|O_CLOEXEC);
                if (syscall(SYS_fcntl, f->fd, F_SETFL, fl) < 0)
                        goto fail;
-               return f;
        } else {
                f2 = fopen(filename, mode);
                if (!f2) goto fail;
                if (f2->fd == f->fd) f2->fd = -1; /* avoid closing in fclose */
                else if (__dup3(f2->fd, f->fd, fl&O_CLOEXEC)<0) goto fail2;
-       }
 
-       f->flags = (f->flags & F_PERM) | f2->flags;
-       f->read = f2->read;
-       f->write = f2->write;
-       f->seek = f2->seek;
-       f->close = f2->close;
+               f->flags = (f->flags & F_PERM) | f2->flags;
+               f->read = f2->read;
+               f->write = f2->write;
+               f->seek = f2->seek;
+               f->close = f2->close;
 
-       fclose(f2);
+               fclose(f2);
+       }
+
+       FUNLOCK(f);
        return f;
 
 fail2:

diff --git a/src/stdio/fwide.c b/src/stdio/fwide.c
index f4da47f6da8ceca422dee4f6c4b1ad45af045ca5..48480685aa4b63a664ccc842548aa8f663c9b662 100644 (file)
--- a/src/stdio/fwide.c
+++ b/src/stdio/fwide.c
@@ -5,6 +5,8 @@
 
 int fwide(FILE *f, int mode)
 {
-       if (!f->mode) f->mode = NORMALIZE(mode);
-       return f->mode;
+       FLOCK(f);
+       if (!f->mode) mode = f->mode = NORMALIZE(mode);
+       FUNLOCK(f);
+       return mode;
 }