lkml.org 
[lkml]   [2004]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [Ext2-devel] Re: [PATCH] use-before-uninitialized value in ext3(2)_find_ goal
On Wed, May 19, 2004 at 12:53:28PM -0700, Chris Wright wrote:
> I know it's a slightly bigger patch, but would it make sense to just enforce
> this as part of api? Just a thought...(patch untested)

No, that doesn't work. Look:

reread:
...

if (ext2_find_goal(inode, iblock, chain, partial, &goal) < 0)
goto changed;

changed:
while (partial > chain) {
brelse(partial->bh);
partial--;
}
goto reread;

So it's spaghetti code that can modify goal. Yuck.

5 labels in one function? 3 backwards jumps? Disgusting.

> --- linux-2.6.6-mm3/fs/ext2/inode.c~goal 2004-05-09 19:32:00.000000000 -0700
> +++ linux-2.6.6-mm3/fs/ext2/inode.c 2004-05-19 12:27:11.968054560 -0700
> @@ -366,6 +366,7 @@ static inline int ext2_find_goal(struct
> unsigned long *goal)
> {
> struct ext2_inode_info *ei = EXT2_I(inode);
> + unsigned long _goal = 0;
> write_lock(&ei->i_meta_lock);
> if (block == ei->i_next_alloc_block + 1) {
> ei->i_next_alloc_block++;
> @@ -377,10 +378,11 @@ static inline int ext2_find_goal(struct
> * failing that at least try to get decent locality.
> */
> if (block == ei->i_next_alloc_block)
> - *goal = ei->i_next_alloc_goal;
> - if (!*goal)
> - *goal = ext2_find_near(inode, partial);
> + _goal = ei->i_next_alloc_goal;
> + if (!_goal)
> + _goal = ext2_find_near(inode, partial);
> write_unlock(&ei->i_meta_lock);
> + *goal = _goal;
> return 0;
> }
> write_unlock(&ei->i_meta_lock);
> --- linux-2.6.6-mm3/fs/ext3/inode.c~goal 2004-05-13 11:19:42.000000000 -0700
> +++ linux-2.6.6-mm3/fs/ext3/inode.c 2004-05-19 12:25:48.441752488 -0700
> @@ -461,6 +461,7 @@ static int ext3_find_goal(struct inode *
> Indirect *partial, unsigned long *goal)
> {
> struct ext3_inode_info *ei = EXT3_I(inode);
> + unsigned long _goal = 0;
> /* Writer: ->i_next_alloc* */
> if (block == ei->i_next_alloc_block + 1) {
> ei->i_next_alloc_block++;
> @@ -474,9 +475,10 @@ static int ext3_find_goal(struct inode *
> * failing that at least try to get decent locality.
> */
> if (block == ei->i_next_alloc_block)
> - *goal = ei->i_next_alloc_goal;
> - if (!*goal)
> - *goal = ext3_find_near(inode, partial);
> + _goal = ei->i_next_alloc_goal;
> + if (!_goal)
> + _goal = ext3_find_near(inode, partial);
> + *goal = _goal;
> return 0;
> }
> /* Reader: end */
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by: SourceForge.net Broadband
> Sign-up now for SourceForge Broadband and get the fastest
> 6.0/768 connection for only $19.95/mo for the first 3 months!
> http://ads.osdn.com/?ad_id=2562&alloc_id=6184&op=click
> _______________________________________________
> Ext2-devel mailing list
> Ext2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ext2-devel

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:03    [W:0.057 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site