Re: [PATCH] ufs: ensure fast symlinks are NUL-terminated

[Duane Griffin]

On Tue, Dec 16, 2008 at 10:40:55PM +0300, Evgeniy Dushistov wrote:
> There is different types of ufs, one used 64 bit for "pointers to
> blocks", another 32 bit,
> so sizeof(UFS_I(inode)->i_u1.i_symlink))
> is not right choice every time,
> in ufs2 it should be
> sizeof(UFS_I(inode)->i_u1.u2_i_data) which 2 times bigger,
> 
> also there is hint for *BSD ufs
> 
> fs/ufs/ufs_fs.h:
> __fs32	fs_maxsymlinklen;/* max length of an internal symlink */
> 
> which may be used if ufs type ufs1 or ufs2

Hmm, I see. However it looks like ufs1_read_inode and ufs2_read_inode
both copy the same, ((UFS_NDADDR + UFS_NINDIR) * 4), amount of inline
symlink data. They also both copy it to ufs_inode_info->i_u1.i_symlink
(not that that matters, I suppose). Perhaps I'm being obtuse, but it
looks like inline ufs2 symlinks between 60 and 120 characters long are
being truncated to 60 characters, no?

There also doesn't seem to be any validation of (f)s_maxsymlinklen being
done. Unless I'm mistaken ufs_symlink could end up overwriting random
memory if it contains a large bogus value.

Does that all sound correct? If so would you like me to whip up a couple
of patches to fix it? I'll respin the NUL-termination patch on top of
those, if so.

> /Evgeniy

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/