On Fri, Jul 23, 2010 at 10:37:59AM +0200, Jan Kara wrote: > Checking of name_len is fine as you did it. But I wanted to point out > that if name_len is greater than 255, you then go to 'cleanup' label which > tries to do up_read(&EXT2_I(inode)->xattr_sem). But that's a bug because > after you moved the code, we don't hold xattr_sem at the moment we check > name_len. Yup, you could just return -ERANGE right there. The simpler fix though might be to just delete the check altogether. Neither ext3 nor ext4 checks for the length of the xattr name in their _xattr_get() function. Instead they'll just do the search, and then return -ENODATA. That seems legit; there can be no entries larger than 255, so saying the extended attribute doesn't exist is quite correct. There is a check in the _xattr_set() functions for both ext3 and ext4, which is quite correct and proper. Does that mean we'll end up doing a search before returning an error --- yes, but I don't think that matters. Why should we care about optimizing an error case? It's not like this is going to be in a timing sensitive part of an application.... (of course the same consideration could apply to your patch as well). - Ted -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
