On Tue, Apr 14, 2015 at 01:00:00PM -0400, Theodore Ts'o wrote: > > Look, either nd_get_link() points inside that page (in which case that > > kfree() is obviously invalid), or it points at kmalloc'ed buffer. In > > which case kfree() is correct, but WTF do you need anything _else_? > > Such as mapped pages, etc. > > Yes, it's either one or the other. > > 1) In the case of an unencrypted symlink which is too big to fit in > the inode, we map in the first (only) block of the symlink, and set > the link to it. ... and that kfree() will bugger us. > 2) In the case of an encrypted symlink, we allocate memory and decrypt > from the first block (or the i_block[] array in the inode), and then > release the page if necessary. ... and that should've dropped that page in ->follow_link(). > I suppose we could have gone from two struct inode_operations (for > fast and "slow" symlinks), to four struct inodes_operations (for > [fast, unencrypted symlinks], [fast, encrypted symlinks], [slow, > unencrypted symlinks], and [slow, encrypted symlinks]), but it was > simpler to use a single follow_link() and put_link() function to > handle multiple cases. Except that you do not handle the slow unencrypted case - you end up with kfree() on the freshly kunmaped address. -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html