Messages in this thread Patch in this message | | | Date | Fri, 04 Aug 2006 10:35:34 -0500 | From | Eric Sandeen <> | Subject | Re: [patch 16/23] ext3: avoid triggering ext3_error on bad NFS file handle |
| |
Christoph Hellwig wrote: > On Fri, Aug 04, 2006 at 09:45:52AM -0500, Eric Sandeen wrote: >> Greg KH wrote: >>> -stable review patch. If anyone has any objections, please let us know. >>> >>> ------------------ >>> From: Neil Brown <neilb@suse.de> >>> >>> The inode number out of an NFS file handle gets passed eventually to >>> ext3_get_inode_block() without any checking. If ext3_get_inode_block() >>> allows it to trigger an error, then bad filehandles can have unpleasant >>> effect - ext3_error() will usually cause a forced read-only remount, or a >>> panic if `errors=panic' was used. >>> >>> So remove the call to ext3_error there and put a matching check in >>> ext3/namei.c where inode numbers are read off storage. >> This patch and the ext2 patch (23/23) are accomplishing the same thing in 2 >> different ways, I think, and introducing unnecessary differences between >> ext2 and ext3. I'd personally prefer to see both ext2 and ext3 handled >> with the get_dentry op addition, and I'd be happy to quickly whip up the >> ext3 patch to do this if there's agreement on this path. > > I completly agree with Eric here. Also pushing out only the fix for one > (and today probably the lesser used) filesystems to -stable seems wrong.
so how's this? (compile tested)
Thanks, -Eric Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
(tho blatantly ripped off from Neil Brown's ext2 patch)
Index: linux-2.6.17/fs/ext3/super.c =================================================================== --- linux-2.6.17.orig/fs/ext3/super.c +++ linux-2.6.17/fs/ext3/super.c @@ -620,8 +620,48 @@ static struct super_operations ext3_sops #endif }; +static struct dentry *ext3_get_dentry(struct super_block *sb, void *vobjp) +{ + __u32 *objp = vobjp; + unsigned long ino = objp[0]; + __u32 generation = objp[1]; + struct inode *inode; + struct dentry *result; + + if (ino != EXT3_ROOT_INO && ino < EXT3_FIRST_INO(sb)) + return ERR_PTR(-ESTALE); + if (ino > le32_to_cpu(EXT3_SB(sb)->s_es->s_inodes_count)) + return ERR_PTR(-ESTALE); + + /* iget isn't really right if the inode is currently unallocated!! + * ext3_read_inode currently does appropriate checks, but + * it might be "neater" to call ext3_get_inode first and check + * if the inode is valid..... + */ + inode = iget(sb, ino); + if (inode == NULL) + return ERR_PTR(-ENOMEM); + if (is_bad_inode(inode) + || (generation && inode->i_generation != generation) + ) { + /* we didn't find the right inode.. */ + iput(inode); + return ERR_PTR(-ESTALE); + } + /* now to find a dentry. + * If possible, get a well-connected one + */ + result = d_alloc_anon(inode); + if (!result) { + iput(inode); + return ERR_PTR(-ENOMEM); + } + return result; +} + static struct export_operations ext3_export_ops = { .get_parent = ext3_get_parent, + .get_dentry = ext3_get_dentry, }; enum { | |