On Mon, 19 Dec 2011 15:13:05 -0500 Eric Paris <eparis@xxxxxxxxxx> wrote: > On Mon, 2011-12-19 at 14:50 -0500, Jeff Layton wrote: > > Setting the security context of a NFSv4 mount via the context= mount > > option is currently broken. The NFSv4 codepath allocates a parsed > > options struct, and then parses the mount options to fill it. It > > eventually calls nfs4_remote_mount which calls security_init_mnt_opts. > > That clobbers the lsm_opts struct that was populated earlier. This bug > > also looks like it causes a small memory leak on each v4 mount where > > context= is used. > > > > Fix this by moving the initialization of the lsm_opts into > > nfs_alloc_parsed_mount_data, and the freeing of the same into the > > functions that allocate the nfs_parsed_mount_data. > > I think this is a good lifetime, but I don't think we have it quite > right. > > > @@ -2222,8 +2223,6 @@ static struct dentry *nfs_fs_mount(struct file_system_type *fs_type, > > if (data == NULL || mntfh == NULL) > > goto out_free_fh; > > Lets assume we allocated data, but failed on mntfh. We are going to > have called security_init_mnt_opts() but never have called the > corresponding destructor. True, it'll be fine today with selinux, but I > make no promises what the future holds... > > I'm pretty sure the v4 code has the same issue. Maybe you should write > an explicit nfs_free_parsed_mount_data() function to handle all of the > error paths in v3 and v4? Just a suggestion.... > > That's a reasonable suggestion. I'll respin this with a destructor for parsed_mount_data structs. Thanks, -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
