| Message ID | 20081211222605.GB18665@dastardly.home.dghda.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 11 Dec 2008 22:26:05 +0000 "Duane Griffin" <duaneg@dghda.com> wrote: > Ensure link targets are NUL-terminated, even if corrupted on-disk. > > Signed-off-by: Duane Griffin <duaneg@dghda.com> > --- > > V2: terminate when the link is read instead of every time it is > followed, as suggested by Dave Kleikamp. > > diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c > index f8424ad..c168781 100644 > --- a/fs/ext3/inode.c > +++ b/fs/ext3/inode.c > @@ -2817,9 +2817,10 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino) > inode->i_op = &ext3_dir_inode_operations; > inode->i_fop = &ext3_dir_operations; > } else if (S_ISLNK(inode->i_mode)) { > - if (ext3_inode_is_fast_symlink(inode)) > + if (ext3_inode_is_fast_symlink(inode)) { > inode->i_op = &ext3_fast_symlink_inode_operations; > - else { > + ((char *) ei->i_data)[inode->i_size] = '\0'; > + } else { > inode->i_op = &ext3_symlink_inode_operations; > ext3_set_aops(inode); > } Really? The ext2 on-disk format requires that the fast symlink be null-terminated on disk? Even though the length is already in i_size? It seems that's true. How un-ext2-like. ext2 and ext4 need the same fix, yes? -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2008/12/12 Andrew Morton <akpm@linux-foundation.org>: > Really? The ext2 on-disk format requires that the fast symlink be > null-terminated on disk? Even though the length is already in i_size? > > It seems that's true. How un-ext2-like. > > ext2 and ext4 need the same fix, yes? Yes. I've sent them out already, but thanks to a monumental cock-up with the CCs they may not have made it to the list. I'll check and resend to real addresses if necessary. Cheers, Duane.
2008/12/12 Duane Griffin <duaneg@dghda.com>: > 2008/12/12 Andrew Morton <akpm@linux-foundation.org>: >> Really? The ext2 on-disk format requires that the fast symlink be >> null-terminated on disk? Even though the length is already in i_size? >> >> It seems that's true. How un-ext2-like. >> >> ext2 and ext4 need the same fix, yes? > > Yes. I've sent them out already, but thanks to a monumental cock-up > with the CCs they may not have made it to the list. I'll check and > resend to real addresses if necessary. Seems they did make it: http://marc.info/?l=linux-kernel&m=122903437006575&w=2 http://marc.info/?l=linux-kernel&m=122903451306859&w=2 Cheers, Duane.
On Fri, 12 Dec 2008 10:19:20 +0000 "Duane Griffin" <duaneg@dghda.com> wrote: > 2008/12/12 Duane Griffin <duaneg@dghda.com>: > > 2008/12/12 Andrew Morton <akpm@linux-foundation.org>: > >> Really? The ext2 on-disk format requires that the fast symlink be > >> null-terminated on disk? Even though the length is already in i_size? > >> > >> It seems that's true. How un-ext2-like. > >> > >> ext2 and ext4 need the same fix, yes? > > > > Yes. I've sent them out already, but thanks to a monumental cock-up > > with the CCs they may not have made it to the list. I'll check and > > resend to real addresses if necessary. > > Seems they did make it: > http://marc.info/?l=linux-kernel&m=122903437006575&w=2 > http://marc.info/?l=linux-kernel&m=122903451306859&w=2 > OK, thanks, it seems I was sneakily not cc'ed ;) As Al points out, the code which you implemented is still vulnerable to on-disk corruption: bad values of i_size will cause the kernel to write a zero byte to any address within the entire CPU address range. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c index f8424ad..c168781 100644 --- a/fs/ext3/inode.c +++ b/fs/ext3/inode.c @@ -2817,9 +2817,10 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino) inode->i_op = &ext3_dir_inode_operations; inode->i_fop = &ext3_dir_operations; } else if (S_ISLNK(inode->i_mode)) { - if (ext3_inode_is_fast_symlink(inode)) + if (ext3_inode_is_fast_symlink(inode)) { inode->i_op = &ext3_fast_symlink_inode_operations; - else { + ((char *) ei->i_data)[inode->i_size] = '\0'; + } else { inode->i_op = &ext3_symlink_inode_operations; ext3_set_aops(inode); }
Ensure link targets are NUL-terminated, even if corrupted on-disk. Signed-off-by: Duane Griffin <duaneg@dghda.com> --- V2: terminate when the link is read instead of every time it is followed, as suggested by Dave Kleikamp.