On Mon, 06 Sep 2010 21:10:10 +0200 Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Mon, 06 Sep 2010, NeilBrown wrote: > > It is not sufficient to depend on the the "filesystem is readonly" > > tests in dentry_permission as it does not check if the vfsmnt is > > readonly. > > All call sites already call mnt_want_write or __mnt_is_readonly which > > includes the test on MS_RDONLY. > > Last time I checked I found some holes (in nfsd IIRC). That was a > long time ago and things may have changed. nfsd looks OK to me. I didn't do an exhaustive audit but couldn't find things that would not still work correctly. > > That check could be replaced with a > > if (IS_RDONLY(inode) && > (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode))) > BUG(); That wouldn't quite work currently. sys_faccessat checks __mnt_is_readonly *after* the call to dentry_permission, so the above would cause it to BUG. Possibly the __mnt_is_readonly could be checked before dentry_permission. However nfsd_permission is a bit more awkward to fix as sometimes it deliberately wants to ignore read-only-filesystem issues ... but it might still be possible to work around.. > > which would catch these cases but only if the superblock was marked > r/o. Otherwise it's basically impossible to make sure the callers of > the VFS play by the rules. That was one reason I advocated a > path_... interface for the VFS instead of the current dentry based > one, but Al didn't like it. > I guess there comes a point were we just have to document the rules and if someone doesn't play by them - that is a bug... NeilBrown -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
