Red Hat Bugzilla – Bug 505110
e2fsprogs code review findings
Last modified: 2009-09-02 05:46:59 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):
There is another O_CREAT bug in debugfs/dump.c.
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.
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.
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.
I have patches for all 3, just awaiting a decision on the exception. Item 2 took a tiny bit of thought. :)
committed & built in e2fsprogs-1.39-23
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.