Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 434532

Summary: sys-apps/openrc - Unsafe Signal Handling?
Product: Gentoo Hosted Projects Reporter: Sven E. <dark>
Component: OpenRCAssignee: OpenRC Team <openrc>
Status: RESOLVED UPSTREAM    
Severity: normal CC: phajdan.jr
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
See Also: https://github.com/OpenRC/openrc/issues/589
Whiteboard:
Package list:
Runtime testing required: ---

Description Sven E. 2012-09-10 03:44:14 UTC
All versions (including current git) of openrc seem have unsafe signal handling in (at least) rc.c and runscript.c.

While snprintf() can possibly be reentrant with a local buffer, it is not guaranteed in general.

free() is most certainly not reentrant.

And ioctl() is possible not reentrant either.

Reproducible: Always




The signal handlers should be modified, so that they adhere to the POSIX standards and only make use of those functions, considered reentrant by POSIX.
Comment 1 SpanKY gentoo-dev 2012-09-17 18:12:08 UTC
to be clear, the complaint is about C library functions called from signal handlers (so the handle_signal() function) and whether they appear in POSIX's async-signal-safe list (they are not called "reentrant"):
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03_03

ioctl() is merely a syscall.  it is not an issue.

snprintf() i'm not too worried about, but its usage is pointless -- it can easily be replaced with a strcpy().

i don't see free() being used anywhere.  you'll need to be more specific.
Comment 2 Sven E. 2012-09-18 21:01:49 UTC
The question is here: Can ioctl() generally be assumed to be reentrant? If we consider it to be reentrant (at least on linux) it is not an issue, agreed.

Concerning free():

in handle_singal() there is a call to: remove_pid(pid);

 366 static void
 367 remove_pid(pid_t pid)
 368 {
 369         RC_PID *p;
 370 
 371         LIST_FOREACH(p, &service_pids, entries)
 372             if (p->pid == pid) {
 373                     LIST_REMOVE(p, entries);
 374                     free(p); //reentrant ??
 375                     return;
 376             }
 377 }
 378 

Which obviously calls free() ... When we assume that glibc's version of free() is reentrant, can we generally assume free() to be reenatrant (on all supported platforms?)?

If snprintf() can be replaced, should it be replaced by a call that is considered reentrant?

Don't get me wrong here, we might be discussing on a high level, but I'd prefer signal handlers (in core components), that can be assumed safe on any platform supported (as they adhere posixes' rules) (there is gentoo/freebsd etc. ???), or am I wrong here?)

If I am completely wrong, I am sorry about the noise, but I am not sure, if posix' constraints should (or can) be ignored here ...

I prefer standards adhering versions of core components which we know are complete safe on all supported platforms.

If you say, the way we implement things is safe on all supported platforsm, so be it, never mind ... but I epect this to be documented clearly ...
Comment 3 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-01-29 03:09:11 UTC
This is valid as a general issue (not sure about these specific examples or not) we're talking about it on the github side now. Let's speak about it further at https://github.com/OpenRC/openrc/issues/589.