Bug 450011 - should complain about shared objects that call exit() or _exit()
should complain about shared objects that call exit() or _exit()
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: rpmlint (Show other bugs)
9
All Linux
low Severity low
: ---
: ---
Assigned To: Ville Skyttä
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-04 14:44 EDT by Bill Nottingham
Modified: 2014-03-16 23:14 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-31 06:25:19 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
First draft implementation (3.48 KB, patch)
2008-06-04 16:57 EDT, Ville Skyttä
no flags Details | Diff
2nd draft (4.84 KB, patch)
2008-07-28 17:11 EDT, Ville Skyttä
no flags Details | Diff

  None (edit)
Description Bill Nottingham 2008-06-04 14:44:14 EDT
Description of problem:

Because it's unspeakably rude. glibc can probably be excepted, though.

Version-Release number of selected component (if applicable):

0.82-3.fc9
Comment 1 Ville Skyttä 2008-06-04 16:57:50 EDT
Created attachment 308398 [details]
First draft implementation

Some hand holding and sanity checking would be very much welcome:

Here's a first draft implementation; in short, search for exit and _exit from
readelf -s output for shared libs in lib/lib64 paths.  The patch is against
upstream svn, should apply to rpmlint 0.83 (in Rawhide only at the moment).

While testing this patch, I see that quite a few cases that now trigger a
warning are ones where fork() is involved, for example F-8 gamin and
esound-libs.  Should we try to detect these cases and not trigger the warning
for them?

Should we check all shared objects, not just ones in lib/lib64 paths?  For
example apps' "private" DSO modules?

Could you submit a descriptive info message for rpmlint -i/-I that Joe Average
packagers would understand and that would help them get the problem fixed? 
"Because it's unspeakably rude" doesn't quite cut it, I think :)
Comment 2 Ville Skyttä 2008-07-26 12:56:51 EDT
Bill, could you take a look at comment 1 when you find time?
Comment 3 Bill Nottingham 2008-07-28 13:21:50 EDT
(In reply to comment #1)
> Here's a first draft implementation; in short, search for exit and _exit from
> readelf -s output for shared libs in lib/lib64 paths.  The patch is against
> upstream svn, should apply to rpmlint 0.83 (in Rawhide only at the moment).

I tested in rawhide.

> While testing this patch, I see that quite a few cases that now trigger a
> warning are ones where fork() is involved, for example F-8 gamin and
> esound-libs.  Should we try to detect these cases and not trigger the warning
> for them?

The more I think about it, if we can't filter these cases, we may want to throw
the idea out, because we'll get way too many false positives.

> Should we check all shared objects, not just ones in lib/lib64 paths?  For
> example apps' "private" DSO modules?

Probably not.

> Could you submit a descriptive info message for rpmlint -i/-I that Joe Average
> packagers would understand and that would help them get the problem fixed? 
> "Because it's unspeakably rude" doesn't quite cut it, I think :)

...
This library package calls exit() or _exit(). Doing so from a library is
strongly discouraged - when a library function calls exit(), it prevents the
calling program from handling the error, reporting it to the user, closing files
properly, and cleaning up any state that the program has. It is preferred for
the library to return an actual error code and let the calling program
decide how to handle the situation.

...
Comment 4 Ville Skyttä 2008-07-28 17:11:31 EDT
Created attachment 312820 [details]
2nd draft

Ok, thanks.  Here's the 2nd draft; this one quite naively omits all exit()
warnings if fork() is also called (anywhere) by the same shared lib.  This will
most likely let more problematic situations pass silently, but could be useful
nevertheless.  At least the warnings for the few packages I tried this with did
not look obvious false positives to me.  Thoughts?
Comment 5 Bill Nottingham 2008-07-28 21:08:52 EDT
Looks good. Would be mildly curious how many packages in rawhide this affects,
but I don't have a full mirror handy at the moment; may try tomorrow.
Comment 6 Jeff Johnson 2008-07-29 09:37:08 EDT
Forbidding all usage cases for exit(2) and _exit(2) in shared objects is rather
rude & crude behavior as well. There are certainly "can't happen" conditions
in libraries that justify linking exit/_exit symbols.

E.g.
$ nm /usr/lib/librpm-5.0.so | grep exit
         U _exit@@GLIBC_2.0
         U exit@@GLIBC_2.0

But whatever, --fuzz=0 is equally risky build policy to inflict on unsuspecting package monkeys imho 
...

Comment 7 Bill Nottingham 2008-07-29 09:50:39 EDT
Funny, that's one of the rudely behaved libaries that this is intended to flag....
Comment 8 Jeff Johnson 2008-07-29 09:58:04 EDT
Ha ha ...

You have to look at the usage case, malloc returning NULL is a "can't happen"
condition where an exit call is arguably justified.

Returning an error from library to application when malloc returns NULL assumes:

1) error return paths exist (not true for many paths in rpmlib).

2) applications are prepared to do something meaningful with the error (rpm-python
script kiddies ain't the best programmers).

Comment 9 Bill Nottingham 2008-07-29 10:46:56 EDT
> Returning an error from library to application when malloc returns NULL assumes:
> 
> 1) error return paths exist (not true for many paths in rpmlib).

Other sloppiness is no justification.

> 2) applications are prepared to do something meaningful with the error (rpm-python
> script kiddies ain't the best programmers).

And by calling exit(), you blindly make the assumption for any apps that may use
the library.

In any case, this bug isn't about the sanity or lack thereof of librpm.
Comment 10 Jeff Johnson 2008-07-29 11:02:19 EDT
"Just return an error." is no solution when stable ABI's are equally important.

DV hounded every occurence of exit out of rpmlib years ago. Surely you remember
the tedium on internal IRC ...

And no, the problem is quite general, you will find many developers that have
sloppy code paths and that blindly call exit/_exit if you pursue a packaging
policy of
    No library should call exit/_exit *EVER*.
That is clearly both unattainable perfection at the same time it is rude and crude
policy *AND* development. I'm using rpmlib only as an example.

Have fun!
Comment 11 Bill Nottingham 2008-07-29 11:16:23 EDT
I'm having a fine time. You seem to have misinterpreted a clearly specified
*warning* as a unilateral packaging policy.
Comment 12 Jeff Johnson 2008-07-29 13:17:32 EDT
Warning abt issues that can't be easily fixed serves about as much purpose as
discussing same issues in bugzilla.You could just as easily detect all libraries
that call exit, vet the results, triage into what issues are fixable (or not), as insturment
the detection into rpmlint. Comment #5 indicates you know that approach quite well.

No matter what: You are clearly a consenting adult, if *warning* is your fig leaf, go fer it!
Comment 13 Ville Skyttä 2008-07-30 12:54:00 EDT
Committed upstream, http://rpmlint.zarb.org/cgi-bin/trac.cgi/changeset/1448
Comment 14 Fedora Update System 2008-10-23 16:54:11 EDT
rpmlint-0.85-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/rpmlint-0.85-1.fc9
Comment 15 Fedora Update System 2008-10-24 19:49:21 EDT
rpmlint-0.85-1.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update rpmlint'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-9125
Comment 16 Fedora Update System 2008-10-31 06:24:54 EDT
rpmlint-0.85-2.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 17 Fedora Update System 2008-11-04 12:36:58 EST
rpmlint-0.85-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/rpmlint-0.85-2.fc10
Comment 18 Fedora Update System 2008-11-22 11:55:24 EST
rpmlint-0.85-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.