Bug 505110 - e2fsprogs code review findings
e2fsprogs code review findings
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: e2fsprogs (Show other bugs)
All Linux
medium Severity medium
: rc
: ---
Assigned To: Eric Sandeen
Depends On:
  Show dependency treegraph
Reported: 2009-06-10 12:56 EDT by Steve Grubb
Modified: 2009-09-02 05:46 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2009-09-02 05:46:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Steve Grubb 2009-06-10 12:56:58 EDT
Did a code review of e2fsprogs and found a couple things:

* In check_mntent_file' at ismounted.c:150: open with O_CREAT in second argument needs 3 arguments.

* In e2fsck/pass3.c at line 322, the pointer 'p' is used immediat after making sure it was NULL.

* In debugfs/logdump.c at line 262, there is a call to fclose without checking to see if NULL is being passed. This will segfault if it does.

Version-Release number of selected component (if applicable):
Comment 1 Steve Grubb 2009-06-10 14:40:37 EDT
There is another O_CREAT bug in debugfs/dump.c.
Comment 3 Steve Grubb 2009-06-15 12:16:13 EDT
In lib/ext2fs/ext2fs.h, the code for ext2fs_get_mem just looks wrong. It mallocs a buffer, copies its address to a pointer and frees the memory. Seems like the caller should do the freeing and not the function that malloc'ed it. :)

ext2fs_resize_mem also appears to have problems. The realloc call uses p for both returned value and passed value. If it used 2 variables, then no leak will occur if realloc failed and overwrites p with NULL. Also, that free at the end just looks wrong, too.
Comment 4 Steve Grubb 2009-06-15 13:24:12 EDT
I was looking at modified code. Disregard comment #3.

But I did find something else. In misc/e2image.c at line 489, the variable inode is being assigned to something that holds its value long after its gone out of scope.
Comment 6 Eric Sandeen 2009-06-26 14:06:46 EDT
Ok, at least the O_CREAT problem is trivial to fix and could be a security issue, I'll do the work for 5.4 if others agree.

Comment 8 Eric Sandeen 2009-06-27 10:40:57 EDT
I have patches for all 3, just awaiting a decision on the exception.  Item 2 took a tiny bit of thought.  :)
Comment 12 Eric Sandeen 2009-06-30 16:49:33 EDT
committed & built in e2fsprogs-1.39-23
Comment 19 errata-xmlrpc 2009-09-02 05:46:59 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.


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