On Thu, Mar 13, 2014 at 01:29:46AM +0200, Imre Deak wrote: > During dentry path lookups we can end up corrupting memory if the > destination path buffer is too small. This is because prepend_path() > and prepend() adjust the passed buffer length unconditionally, allowing > for the buffer length to go negative. Then a later prepend_name() call > will receive a negative length and convert this to unsigned before > comparing it against the source string length leading to a possible > memory corruption preceeding the destination buffer. > > diff --git a/fs/dcache.c b/fs/dcache.c > index 265e0ce..4015fd9 100644 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -2833,7 +2833,8 @@ static int prepend_name(char **buffer, int *buflen, > struct qstr *name) > u32 dlen = ACCESS_ONCE(name->len); > char *p; > > - if (*buflen < dlen + 1) > + /* make sure we don't convert a negative value to unsigned int */ > + if (*buflen < 0 || *buflen < dlen + 1) > return -ENAMETOOLONG; > *buflen -= dlen + 1;
It's much easier to fix, actually. Look at the callers of prepend_name(); when it returns a negative value, they either discard the value left in *buflen (__dentry_path()) or pass it back to their caller and return a negative value themselves (prepend_path()). The same is true for prepend_path() (discared in __d_path(), d_absolute_path(), sys_getcwd(); passed to caller with negative return value in path_with_deleted()) and path_with_deleted() (the only caller is d_path() and it always discards). In other words, when prepend_name() returns -ENAMETOOLONG, it is free to leave whatever it wants in *buflen. So let's do what prepend() does - subtract from *buflen, then check if the result has become negative. Generates a better code than the original, actually... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
