path: root/src
diff options
authorRich Felker <>2018-11-02 12:31:19 -0400
committerRich Felker <>2018-11-02 12:31:19 -0400
commit4a086030264f5cf423ea76453ef721e2c8e2e093 (patch)
tree8e09c785e7715b22a42189793c596881dfcfa6cd /src
parent00bd3b7d3006c5d350959c994fa65358bf65e6a2 (diff)
fix deadlock and buffered data loss race in fclose
fflush(NULL) and __stdio_exit lock individual FILEs while holding the open file list lock to walk the list. since fclose first locked the FILE to be closed, then the ofl lock, it could deadlock with these functions. also, because fclose removed the FILE to be closed from the open file list before flushing and closing it, a concurrent fclose or exit could complete successfully before fclose flushed the FILE it was closing, resulting in data loss. reorder the body of fclose to first flush and close the file, then remove it from the open file list only after unlocking it. this creates a window where consumers of the open file list can see dead FILE objects, but in the absence of undefined behavior on the part of the application, such objects will be in an inactive-buffer state and processing them will have no side effects. __unlist_locked_file is also moved so that it's performed only for non-permanent files. this change is not necessary, but preserves consistency (and thereby provides safety/hardening) in the case where an application uses one of the standard streams after closing it while holding an explicit lock on it. such usage is of course undefined behavior.
Diffstat (limited to 'src')
1 files changed, 19 insertions, 13 deletions
diff --git a/src/stdio/fclose.c b/src/stdio/fclose.c
index 889b96d2..d594532b 100644
--- a/src/stdio/fclose.c
+++ b/src/stdio/fclose.c
@@ -7,26 +7,32 @@ weak_alias(dummy, __unlist_locked_file);
int fclose(FILE *f)
int r;
- int perm;
+ r = fflush(f);
+ r |= f->close(f);
- __unlist_locked_file(f);
+ /* Past this point, f is closed and any further explict access
+ * to it is undefined. However, it still exists as an entry in
+ * the open file list and possibly in the thread's locked files
+ * list, if it was closed while explicitly locked. Functions
+ * which process these lists must tolerate dead FILE objects
+ * (which necessarily have inactive buffer pointers) without
+ * producing any side effects. */
- if (!(perm = f->flags & F_PERM)) {
- FILE **head = __ofl_lock();
- if (f->prev) f->prev->next = f->next;
- if (f->next) f->next->prev = f->prev;
- if (*head == f) *head = f->next;
- __ofl_unlock();
- }
+ if (f->flags & F_PERM) return r;
- r = fflush(f);
- r |= f->close(f);
+ __unlist_locked_file(f);
+ FILE **head = __ofl_lock();
+ if (f->prev) f->prev->next = f->next;
+ if (f->next) f->next->prev = f->prev;
+ if (*head == f) *head = f->next;
+ __ofl_unlock();
- if (!perm) free(f);
- else FUNLOCK(f);
+ free(f);
return r;