Bug 464689 - libcom_err leaks a descriptor when the application calls add_error_table()
libcom_err leaks a descriptor when the application calls add_error_table()
Product: Fedora
Classification: Fedora
Component: e2fsprogs (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Eric Sandeen
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-09-29 17:38 EDT by Nalin Dahyabhai
Modified: 2008-10-02 15:33 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-10-02 15:33:51 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

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

  None (edit)
Description Nalin Dahyabhai 2008-09-29 17:38:15 EDT
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):
Comment 1 Eric Sandeen 2008-09-30 15:48:13 EDT
Thanks for the heads up - I take it you've seen this "in real life" with Kerberos?

Comment 2 Nalin Dahyabhai 2008-09-30 16:52:44 EDT
(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 17:32:19 EDT
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-01 21:04:30 EDT
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-01 21:16:38 EDT
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-01 21:45:22 EDT
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 :)

Comment 7 Theodore Tso 2008-10-01 22:53:24 EDT
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 15:33:51 EDT
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.