Change vfork() to posix_spawn()?

2012-09-14 Thread Erik Cederstrand
Hello hackers,

I'm looking through the Clang Analyzer scans on 
http://scan.freebsd.your.org/freebsd-head looking for false positives to report 
back to LLVM. There are quite a list of reports suggesting to change vfork() 
calls to posix_spawn(). Example from /bin/rpc: 
http://scan.freebsd.your.org/freebsd-head/bin.rcp/2012-09-12-amd64/report-nsOV80.html#EndPath

I know nothing about this but I can see fork and posix_spawn have been 
discussed on this list previously. Is this a legitimate warning (in this case 
and in general in FreeBSD base)?

Thanks,
Erik___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: Change vfork() to posix_spawn()?

2012-09-14 Thread Ivan Voras
On 14/09/2012 09:49, Erik Cederstrand wrote:
 Hello hackers,
 
 I'm looking through the Clang Analyzer scans on 
 http://scan.freebsd.your.org/freebsd-head looking for false positives to 
 report back to LLVM. There are quite a list of reports suggesting to change 
 vfork() calls to posix_spawn(). Example from /bin/rpc: 
 http://scan.freebsd.your.org/freebsd-head/bin.rcp/2012-09-12-amd64/report-nsOV80.html#EndPath
 
 I know nothing about this but I can see fork and posix_spawn have been 
 discussed on this list previously. Is this a legitimate warning (in this case 
 and in general in FreeBSD base)?

Currently (on 9-stable at least), posix_spawn() is implemented as a
wrapper around vfork(), so I doubt replacing one with the other would do
much.



signature.asc
Description: OpenPGP digital signature


Re: Change vfork() to posix_spawn()?

2012-09-14 Thread Erik Cederstrand
Den 14/09/2012 kl. 13.03 skrev Ivan Voras ivo...@freebsd.org:

 On 14/09/2012 09:49, Erik Cederstrand wrote:
 Hello hackers,
 
 I'm looking through the Clang Analyzer scans on 
 http://scan.freebsd.your.org/freebsd-head looking for false positives to 
 report back to LLVM. There are quite a list of reports suggesting to change 
 vfork() calls to posix_spawn(). Example from /bin/rpc: 
 http://scan.freebsd.your.org/freebsd-head/bin.rcp/2012-09-12-amd64/report-nsOV80.html#EndPath
 
 I know nothing about this but I can see fork and posix_spawn have been 
 discussed on this list previously. Is this a legitimate warning (in this 
 case and in general in FreeBSD base)?
 
 Currently (on 9-stable at least), posix_spawn() is implemented as a
 wrapper around vfork(), so I doubt replacing one with the other would do
 much.

The analyzer added this warning in January. The release notes link to this 
explanation: 
https://www.securecoding.cert.org/confluence/display/seccode/POS33-C.+Do+not+use+vfork()

I guess this is the important part:

Because of the implementation of the vfork() function, the parent process is 
suspended while the child process executes. If a user sends a signal to the 
child process, delaying its execution, the parent process (which is privileged) 
is also blocked. This means that an unprivileged process can cause a privileged 
process to halt, which is a privilege inversion resulting in a denial of 
service.

Erik___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: Change vfork() to posix_spawn()?

2012-09-14 Thread mdf
On Fri, Sep 14, 2012 at 4:45 AM, Erik Cederstrand e...@cederstrand.dk wrote:
 Den 14/09/2012 kl. 13.03 skrev Ivan Voras ivo...@freebsd.org:

 On 14/09/2012 09:49, Erik Cederstrand wrote:
 Hello hackers,

 I'm looking through the Clang Analyzer scans on 
 http://scan.freebsd.your.org/freebsd-head looking for false positives to 
 report back to LLVM. There are quite a list of reports suggesting to change 
 vfork() calls to posix_spawn(). Example from /bin/rpc: 
 http://scan.freebsd.your.org/freebsd-head/bin.rcp/2012-09-12-amd64/report-nsOV80.html#EndPath

 I know nothing about this but I can see fork and posix_spawn have been 
 discussed on this list previously. Is this a legitimate warning (in this 
 case and in general in FreeBSD base)?

 Currently (on 9-stable at least), posix_spawn() is implemented as a
 wrapper around vfork(), so I doubt replacing one with the other would do
 much.

 The analyzer added this warning in January. The release notes link to this 
 explanation: 
 https://www.securecoding.cert.org/confluence/display/seccode/POS33-C.+Do+not+use+vfork()

 I guess this is the important part:

 Because of the implementation of the vfork() function, the parent process is 
 suspended while the child process executes. If a user sends a signal to the 
 child process, delaying its execution, the parent process (which is 
 privileged) is also blocked. This means that an unprivileged process can 
 cause a privileged process to halt, which is a privilege inversion resulting 
 in a denial of service.


Isn't the important part the previous paragraph, which said that some
older versions of Linux had the problem?  The entire thing reads that
the issue comes from an idiom of vfork(), setuid(), then exec, which
is both undefined and would be specific to only some *uses* of
vfork(), not the implementation.

The whole thing isn't worded terribly usefully; e.g. it doesn't
explain if it was only Linux that had an issue, which version of Linux
is fixed, whether normal code that went straight from vfork() to
exec() was fine, etc.

Cheers,
matthew
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: Change vfork() to posix_spawn()?

2012-09-14 Thread Jilles Tjoelker
On Fri, Sep 14, 2012 at 01:45:49PM +0200, Erik Cederstrand wrote:
 Den 14/09/2012 kl. 13.03 skrev Ivan Voras ivo...@freebsd.org:
  On 14/09/2012 09:49, Erik Cederstrand wrote:

  I'm looking through the Clang Analyzer scans on
  http://scan.freebsd.your.org/freebsd-head looking for false
  positives to report back to LLVM. There are quite a list of reports
  suggesting to change vfork() calls to posix_spawn(). Example from
  /bin/rpc:
  http://scan.freebsd.your.org/freebsd-head/bin.rcp/2012-09-12-amd64/report-nsOV80.html#EndPath

  I know nothing about this but I can see fork and posix_spawn have
  been discussed on this list previously. Is this a legitimate
  warning (in this case and in general in FreeBSD base)?

  Currently (on 9-stable at least), posix_spawn() is implemented as a
  wrapper around vfork(), so I doubt replacing one with the other
  would do much.

vfork() returns twice, possibly reanimating variables from the dead.
Calling posix_spawn() limits this issue to the posix_spawn()
implementation only.

For example, in case of unwilling compiler developers, the optimization
level for that file might be lowered or more volatile keywords might be
added. I think it makes more sense to disable various optimizations in
the compiler automatically in functions that call vfork(), longjmp() and
similar functions, but I do not decide what compiler developers do.

 The analyzer added this warning in January. The release notes link to
 this explanation:
 https://www.securecoding.cert.org/confluence/display/seccode/POS33-C.+Do+not+use+vfork()

 I guess this is the important part:

 Because of the implementation of the vfork() function, the parent
 process is suspended while the child process executes. If a user sends
 a signal to the child process, delaying its execution, the parent
 process (which is privileged) is also blocked. This means that an
 unprivileged process can cause a privileged process to halt, which is
 a privilege inversion resulting in a denial of service.

This problem only occurs if privileges are dropped between vfork and
exec, which is uncommon. If no privileges are dropped, the user can
affect the parent directly.

Furthermore, this exact problem does not happen in FreeBSD because child
processes between vfork and exec/exit are not affected by stop signals
(this is stronger than the vfork(2) man page documents).

However, related issues are still present. If there is a signal handler
that blocks for a long time (many functions which do this are
async-signal-safe) for a signal permitted by
security.bsd.conservative_signals, an unprivileged user will be able to
trigger it and delay the thread calling vfork(). A function may also be
async-signal-safe but not suitable for a vforked child (for example,
libthr makes many functions async-signal-safe by postponing signal
handlers which is not good enough if a vforked child is SIGKILL'ed).

An unprivileged user may also trigger priority inversion by lowering the
priority of the child process and consuming CPU time at a higher
priority.

Obviously, the child process should not lower its priority voluntarily
either.

These problems can be fixed in various ways. The direct priority
inversion problem can be fixed by using fork() instead in that case or
by adding a priority inheritance scheme in the kernel for vforked
children (but only for the static priority; the parent's dynamic
priority will increase because it is sleeping).

The privilege manipulation available via POSIX_SPAWN_RESETIDS seems safe
enough. Since it only affects the effective UID/GID, it does not affect
the ability to modify scheduling parameters (real UID) or to send
signals (real or saved UID). Since the seteuid() call itself will set
the issetugid flag to true if it changed anything, it does not affect
the ability to debug before exec.

More general privilege dropping frequently involves frameworks such as
PAM which are not async-signal-safe and certainly not vfork-safe.

-- 
Jilles Tjoelker
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org