Bug 464689

Summary:

libcom_err leaks a descriptor when the application calls add_error_table()

Product:

[Fedora] Fedora

Reporter:

Nalin Dahyabhai <nalin>

Component:

e2fsprogs

Assignee:

Eric Sandeen <esandeen>

Status:

CLOSED RAWHIDE

QA Contact:

Fedora Extras Quality Assurance <extras-qa>

Severity:

medium

Docs Contact:

Priority:

medium

Version:

rawhide

CC:

esandeen, kzak, oliver, tytso

Target Milestone:

---

Target Release:

---

Hardware:

All

OS:

Linux

Whiteboard:

Fixed In Version:

Doc Type:

Bug Fix

Doc Text:

Story Points:

---

Clone Of:

Environment:

Last Closed:

2008-10-02 19:33:51 UTC

Type:

---

Regression:

---

Mount Type:

---

Documentation:

---

CRM:

Verified Versions:

Category:

---

oVirt Team:

---

RHEL 7.3 requirements from Atomic Host:

Cloudforms Team:

---

Target Upstream Version:

Embargoed:

Attachments:

Description	Flags
Patch to fix this bug	none
Updated patch to fix this bug	none

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!