Bug 464689 - libcom_err leaks a descriptor when the application calls add_error_table()
Summary: libcom_err leaks a descriptor when the application calls add_error_table()
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: e2fsprogs
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Eric Sandeen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-29 21:38 UTC by Nalin Dahyabhai
Modified: 2008-10-02 19:33 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-02 19:33:51 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Patch to fix this bug (1.72 KB, patch)
2008-10-02 01:04 UTC, Theodore Tso
no flags Details | Diff
Updated patch to fix this bug (1.86 KB, patch)
2008-10-02 01:16 UTC, Theodore Tso
no flags Details | Diff

Description Nalin Dahyabhai 2008-09-29 21:38:15 UTC
Description of problem:

When an application calls add_error_table(), libcom_err opens the controlling terminal to potentially log debug output.  It doesn't set the
close-on-exec flag on the file descriptor associated with the handle, so if/when the calling application exec()s another program (or even itself), the descriptor is leaked.

For e2fsprogs applications, this might not ever pose a problem, but it also affects binaries which pull in libcom_err as a dependency of Kerberos.

Version-Release number of selected component (if applicable):
1.41.0-2.fc10

Comment 1 Eric Sandeen 2008-09-30 19:48:13 UTC
Thanks for the heads up - I take it you've seen this "in real life" with Kerberos?

Thanks,
-Eric

Comment 2 Nalin Dahyabhai 2008-09-30 20:52:44 UTC
(In reply to comment #1)
> Thanks for the heads up - I take it you've seen this "in real life" with
> Kerberos?

That's how I ran across it, yes.  Though in my case it was more a curiosity (the test app in question was periodically exec()ing itself to restart, and it kept losing a descriptor each time) than a critical failure.  Eventually it would have broken, but it would have had to be running for days before it became a real problem.

Comment 3 Theodore Tso 2008-09-30 21:32:19 UTC
Setting the close on exec flag would be a good thing, but the right answer is to not fopen() and initialize debug_f if the COMERR_DEBUG environment variable is not set.

Comment 4 Theodore Tso 2008-10-02 01:04:30 UTC
Created attachment 319184 [details]
Patch to fix this bug

Here's a patch which should fix this bugzilla report.

Comment 5 Theodore Tso 2008-10-02 01:16:38 UTC
Created attachment 319186 [details]
Updated patch to fix this bug

Oops, the first patch didn't do the proper error checking.  (Neither did the original code, actually.)

Comment 6 Eric Sandeen 2008-10-02 01:45:22 UTC
Thanks Ted! - I didn't mean to foist the bug off on you, but I hadn't caught that the file should only be opened under COMERR_DEBUG, so I guess the cc: was a good thing :)

-Eric

Comment 7 Theodore Tso 2008-10-02 02:53:24 UTC
Well, I'm trying to get an e2fsprogs 1.41.2 release out the door, so it was faster for me to just fix it myself.

BTW, there's one fix in particular in e2fsprogs 1.41.2 that you'll want to take, even if you don't take the all of 1.41.2, and that's commit 52771ab5.  Without this, "e2fsck -b 32768 /dev/hdXX" doesn't work, and users have to explicitly specify the blocksize "e2fsck -b 32768 -B 4096 /dev/hdXX".  That's nasty, since e2fsck suggests the former, and users are used to the former.

Comment 8 Eric Sandeen 2008-10-02 19:33:51 UTC
Should be fixed in 1.41.2; it's in rawhide now.
Thanks Ted!


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