Fix memory leak on freopen error return (bug 32140)

Florian Weimer fweimer@redhat.com
Thu Sep 5 08:31:06 GMT 2024


* Joseph Myers:

> As reported in bug 32140, freopen leaks the FILE object when it
> returns NULL: there is no valid use of the FILE * pointer (including
> passing to freopen again or to fclose) after such an error return, so
> the underlying object should be freed.  Add code to free it.
>
> Note 1: while I think it's clear from the relevant standards that the
> object should be freed and the FILE * can't be used after the call in
> this case (the stream is closed, which ends the lifetime of the FILE),
> it's entirely possible that some existing code does in fact try to use
> the existing FILE * in some way and could be broken by this change.
> (Though the most common case for freopen may be stdin / stdout /
> stderr, which _IO_deallocate_file explicitly checks for and does not
> deallocate.)
>
> Note 2: the deallocation is only done in the _IO_IS_FILEBUF case.
> Other kinds of streams bypass all the freopen logic handling closing
> the file, meaning a call to _IO_deallocate_file would neither be safe
> (the FILE might still be linked into the list of all open FILEs) nor
> sufficient (other internal memory allocations associated with the file
> would not have been freed).  I think the validity of freopen for any
> other kind of stream will need clarifying with the Austin Group, but
> if it is valid in any such case (where "valid" means "not undefined
> behavior so required to close the stream" rather than "required to
> successfully associate the stream with the new file in cases where
> fopen would work"), more significant changes would be needed to ensure
> the stream gets fully closed.
>
> Tested for x86_64.

This looks okay to me as far as the change itself goes.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

We can put it into Fedora rawhide and observe what the compatibility
impact is.  Hopefully it's going to be okay.  If it's bad, we need to
consider symbol-versioning it, or maintaining a freelist of FILE
objects.

Thanks,
Florian



More information about the Libc-alpha mailing list