mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Feasibility of FD_CLOEXEC on all streams
@ 2021-12-18 16:33 Markus Wichmann
  2021-12-18 17:14 ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Markus Wichmann @ 2021-12-18 16:33 UTC (permalink / raw)
  To: musl

Hi all,

I was recently reading the source code of popen(), and noticed that it
has to iterate over all open files to close all the open pipe FDs the
child might inherit. And that made me wonder:

1. Does POSIX allow for all FILE streams to have FD_CLOEXEC applied by
default?

2. Is that something we might wish to explore?

Number two I will just have to open to debate here on the list (and
let's be honest, Rich is going to be the one to have final say on the
matter).

As for number one, obviously ISO C isn't going to say anything on the
matter one way or the other, seeing as ISO C doesn't know about exec.
And POSIX has a chapter talking about the relationship between FDs and
streams, that says explicitly that after exec all streams are going to
be closed, no matter what FDs remain open.

I could find nothing condemning or condoning this approach. So it
appears to be a valid implementation choice.

To be clear, I am basically only talking about adding O_CLOEXEC to the
open() call in fopen(), and keeping the FD_CLOEXEC flag set on the pipe
FD in popen(). fdopen() would remain as is. That means that fopen() with
"e" in the mode string is still possible, only it does nothing other
than without the "e".

The technical benefits are minor, admittedly. The loop that closes all
pipe FDs in popen() could be removed. And that is mostly it. Programs
using fopen() that spawn subprocesses can no longer forget to close
those FDs, limiting FD leakage. Which usually is not a security problem,
but can be. But in most instances where it is, the program is buggy with
glibc, so the bug would need to be fixed on the application level
(programs cannot rely on this behavior). So on the advantages side, we
would be moving closer to "security-by-default".

Still, I don't foresee too many technical drawbacks, either. The only
case I can think of that would fail now is if a program were to open a
file with fopen(), and try to bestow the FD to a subprocess, and only
dup() it if it does not equal an expected value. E.g.

    FILE *f = fopen(...);
    ...
    if (fileno(f) != 3)
        dup2(fileno(f), 3);
    exec(program that does something with FD 3);

But I would expect such usage to be extremely rare.

Thoughts?

Ciao,
Markus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] Feasibility of FD_CLOEXEC on all streams
  2021-12-18 16:33 [musl] Feasibility of FD_CLOEXEC on all streams Markus Wichmann
@ 2021-12-18 17:14 ` Rich Felker
  2021-12-18 17:26   ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2021-12-18 17:14 UTC (permalink / raw)
  To: Markus Wichmann; +Cc: musl

On Sat, Dec 18, 2021 at 05:33:20PM +0100, Markus Wichmann wrote:
> Hi all,
> 
> I was recently reading the source code of popen(), and noticed that it
> has to iterate over all open files to close all the open pipe FDs the
> child might inherit. And that made me wonder:
> 
> 1. Does POSIX allow for all FILE streams to have FD_CLOEXEC applied by
> default?

No. Accessing fileno(f) is permissible subject to following the rules
for active handle:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05_01

and that entails being able to use them according to the rules for how
fds are inherited across exec.

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] Feasibility of FD_CLOEXEC on all streams
  2021-12-18 17:14 ` Rich Felker
@ 2021-12-18 17:26   ` Rich Felker
  2021-12-19 14:54     ` Alex Xu (Hello71)
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2021-12-18 17:26 UTC (permalink / raw)
  To: Markus Wichmann; +Cc: musl

On Sat, Dec 18, 2021 at 12:14:15PM -0500, Rich Felker wrote:
> On Sat, Dec 18, 2021 at 05:33:20PM +0100, Markus Wichmann wrote:
> > Hi all,
> > 
> > I was recently reading the source code of popen(), and noticed that it
> > has to iterate over all open files to close all the open pipe FDs the
> > child might inherit. And that made me wonder:
> > 
> > 1. Does POSIX allow for all FILE streams to have FD_CLOEXEC applied by
> > default?
> 
> No. Accessing fileno(f) is permissible subject to following the rules
> for active handle:
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05_01
> 
> and that entails being able to use them according to the rules for how
> fds are inherited across exec.

Also, the POSIX spec for fopen is rather explicit:

    "[CX] The file descriptor associated with the opened stream shall
    be allocated and opened as if by a call to open() with the
    following flags: ..."

https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] Feasibility of FD_CLOEXEC on all streams
  2021-12-18 17:26   ` Rich Felker
@ 2021-12-19 14:54     ` Alex Xu (Hello71)
  2021-12-19 15:22       ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Xu (Hello71) @ 2021-12-19 14:54 UTC (permalink / raw)
  To: musl, Markus Wichmann

Excerpts from Rich Felker's message of December 18, 2021 12:26 pm:
> On Sat, Dec 18, 2021 at 12:14:15PM -0500, Rich Felker wrote:
>> On Sat, Dec 18, 2021 at 05:33:20PM +0100, Markus Wichmann wrote:
>> > Hi all,
>> > 
>> > I was recently reading the source code of popen(), and noticed that it
>> > has to iterate over all open files to close all the open pipe FDs the
>> > child might inherit. And that made me wonder:
>> > 
>> > 1. Does POSIX allow for all FILE streams to have FD_CLOEXEC applied by
>> > default?
>> 
>> No. Accessing fileno(f) is permissible subject to following the rules
>> for active handle:
>> 
>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05_01
>> 
>> and that entails being able to use them according to the rules for how
>> fds are inherited across exec.
> 
> Also, the POSIX spec for fopen is rather explicit:
> 
>     "[CX] The file descriptor associated with the opened stream shall
>     be allocated and opened as if by a call to open() with the
>     following flags: ..."
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
> 
> Rich
> 

Playing devil's advocate here, can't the implementation unset FD_CLOEXEC 
when fileno is called? This doesn't fix the latter issue, but if that's 
the only problem then I would argue that it can be sufficiently covered 
by the as-if rule. It also wouldn't fix the popen loop, but would still 
add some hardening for poorly written programs.

Cheers,
Alex.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] Feasibility of FD_CLOEXEC on all streams
  2021-12-19 14:54     ` Alex Xu (Hello71)
@ 2021-12-19 15:22       ` Rich Felker
  0 siblings, 0 replies; 5+ messages in thread
From: Rich Felker @ 2021-12-19 15:22 UTC (permalink / raw)
  To: Alex Xu (Hello71); +Cc: musl, Markus Wichmann

On Sun, Dec 19, 2021 at 09:54:32AM -0500, Alex Xu (Hello71) wrote:
> Excerpts from Rich Felker's message of December 18, 2021 12:26 pm:
> > On Sat, Dec 18, 2021 at 12:14:15PM -0500, Rich Felker wrote:
> >> On Sat, Dec 18, 2021 at 05:33:20PM +0100, Markus Wichmann wrote:
> >> > Hi all,
> >> > 
> >> > I was recently reading the source code of popen(), and noticed that it
> >> > has to iterate over all open files to close all the open pipe FDs the
> >> > child might inherit. And that made me wonder:
> >> > 
> >> > 1. Does POSIX allow for all FILE streams to have FD_CLOEXEC applied by
> >> > default?
> >> 
> >> No. Accessing fileno(f) is permissible subject to following the rules
> >> for active handle:
> >> 
> >> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05_01
> >> 
> >> and that entails being able to use them according to the rules for how
> >> fds are inherited across exec.
> > 
> > Also, the POSIX spec for fopen is rather explicit:
> > 
> >     "[CX] The file descriptor associated with the opened stream shall
> >     be allocated and opened as if by a call to open() with the
> >     following flags: ..."
> > 
> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
> 
> Playing devil's advocate here, can't the implementation unset FD_CLOEXEC 
> when fileno is called? This doesn't fix the latter issue, but if that's 
> the only problem then I would argue that it can be sufficiently covered 
> by the as-if rule.

No, because file descriptors are required to be assigned in
lowest-unused order and some really bad software could skip fileno and
just assume it got the fd number it wanted(*). In fact this is somewhat
common practice for the standard streams, albeit awful, and usually
with just plain open not fopen.

(*) One could argue that this is invalid usage, as library functions
are allowed to open and close file descriptors for their own internal
use as long as it's not visible to the application. However,
interpreted too loosely, that would effectively nullify the
lowest-unused requirement, so I would read that allowance with an
as-if rule, that the "lowest-unused" has to be assigned as if the set
of already-used fds was the same as at the time of call.

> It also wouldn't fix the popen loop, but would still 
> add some hardening for poorly written programs.

This "hardening" should be understood as avoiding a potential fd leak
in erroneous programs at the expense of *introducing use-after-close
bugs* in very-bad-style-but-correct programs. This does not seem like
a reasonable tradeoff at all.

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-19 15:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-18 16:33 [musl] Feasibility of FD_CLOEXEC on all streams Markus Wichmann
2021-12-18 17:14 ` Rich Felker
2021-12-18 17:26   ` Rich Felker
2021-12-19 14:54     ` Alex Xu (Hello71)
2021-12-19 15:22       ` Rich Felker

Code repositories for project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).