lkml.org 
[lkml]   [2008]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] vfs: introduce helper function to safely NUL-terminate symlinks
On Dec 16, 2008  15:51 +0000, Duane Griffin wrote:
> A number of filesystems were potentially triggering kernel bugs due to
> corrupted symlink names on disk. This helper helps safely terminate the
> names.
> +static inline void nd_terminate_link(void *name,unsigned len,unsigned maxlen)
> +{
> + ((char *) name)[min(len, maxlen)] = '\0';
> +}

> @@ -4200,9 +4201,11 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
> } else if (S_ISLNK(inode->i_mode)) {
> + if (ext4_inode_is_fast_symlink(inode)) {
> inode->i_op = &ext4_fast_symlink_inode_operations;
> + nd_terminate_link(ei->i_data, inode->i_size,
> + sizeof(ei->i_data));
> + } else {
> inode->i_op = &ext4_symlink_inode_operations;
> ext4_set_aops(inode);
> }

With sizeof(ei->i_data) = 15 * 4 = 60 bytes, this will set ei->i_data[60]
as NUL, which is writing 1 byte beyond the end of the array.

Note that in ext[234]_symlink() the check for fast symlinks is:

l = strlen(symname)+1;
if (l > sizeof (EXT3_I(inode)->i_data)) {
inode->i_op = &ext3_symlink_inode_operations;
} else {
inode->i_op = &ext3_fast_symlink_inode_operations;
inode->i_size = l-1;
}
so in fact the fast symlinks should always have space for a trailing NUL
character, and "sizeof(ei->i_data) - 1" is the right maxlen to use for
ext[234].

That might not be true for other filesystems, in which case you would
need to add a "padding" field after the symlink name in memory to hold
the trailing NUL.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.



\
 
 \ /
  Last update: 2008-12-17 00:49    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site