Bug 505110 - e2fsprogs code review findings
e2fsprogs code review findings
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: e2fsprogs (Show other bugs)
5.4
All Linux
medium Severity medium
: rc
: ---
Assigned To: Eric Sandeen
BaseOS QE
:
Depends On:
Blocks:
  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:
Environment:
Last Closed: 2009-09-02 05:46:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
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):
1.39-22.el5
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.

Thanks,
-Eric
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.

http://rhn.redhat.com/errata/RHBA-2009-1291.html

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