On Mon, Sep 10, 2012 at 11:55:48AM -0000, Carlos Maiolino wrote: > htree_dirblock_to_tree() declares a non-initialized 'err' variable, > which is passed as a reference to another functions expecting them > to set this variable with thei error codes. It's passed to > ext4_bread(), which then passes it to ext4_getblk(). If > ext4_map_blocks() returns 0 due to a lookup failure, leaving the > ext4_getblk() buffer_head uninitialized, it will make ext4_getblk() > return to ext4_bread() without initialize the 'err' variable, and > ext4_bread() will return to htree_dirblock_to_tree() with this > variable still uninitialized. Hi Carlos, Thanks for noticing this problem! In the case where there is no block mapping for a particular block, ext4_bread() can return NULL, and with your patch, *err will now be zero instead of some uninitialized value. That's an improvement, and in the case of htree_dirblock_to_tree(), when we return 0 as an "error", the caller will do the right thing. But there are other places where when ext4_bread() returns NULL with err set to 0, the function ends up returning err, i.e., in ext4_add_entry: bh = ext4_bread(handle, dir, block, 0, &retval); if(!bh) return retval; ... which will cause the caller of ext4_add_entry() to think that the function had succeeded. In the case of directories, there is never supposed to "holes" in directories, so the right thing to do is to check to see if err = 0 and in that case to call ext4_error() to mark the file system as being inconsistent, and then returning some kind of error like -EIO. So your patch is an improvement, but I'm worried that there were cases where we had been returning some uninitialized, non-zero stack garbage, we had been serendipously treating the case of a directory hole as an "error", now we we consider that situation as a "success" even though the calling function (such as ext4_add_entry) had not completed its processing. Which is a very long-winded way of saying that we need to audit all of the functions which call ext4_bread() so that they do the right thing when ext4_bread() returns NULL and err is set to zero. Regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
