* [Cluster-devel] [GFS2 Patch] GFS2: Add kobject release method [not found] <5da8935b-4362-4b87-b2ef-2753ce1edb0b@zmail12.collab.prod.int.phx2.redhat.com> @ 2012-06-13 14:27 ` Bob Peterson 2012-06-13 15:52 ` Steven Whitehouse 0 siblings, 1 reply; 2+ messages in thread From: Bob Peterson @ 2012-06-13 14:27 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, This patch adds a kobject release function that properly maintains the kobject use count, so that accesses to the sysfs files do not cause an access to freed kernel memory after an unmount. Regards, Bob Peterson Red Hat File Systems Signed-off-by: Bob Peterson <rpeterso@redhat.com> --- diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index c5871ae..378c90f 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -1123,20 +1123,33 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent } error = init_names(sdp, silent); - if (error) - goto fail; + if (error) { + /* In this case, we haven't initialized sysfs, so we have to + manually free the sdp. */ + free_percpu(sdp->sd_lkstats); + kfree(sdp); + sb->s_fs_info = NULL; + return error; + } snprintf(sdp->sd_fsname, GFS2_FSNAME_LEN, "%s", sdp->sd_table_name); - gfs2_create_debugfs_file(sdp); - error = gfs2_sys_fs_add(sdp); + /* + * If we hit an error here, gfs2_sys_fs_add will have called function + * kobject_put which causes the sysfs usage count to go to zero, which + * causes sysfs to call function gfs2_sbd_release, which frees sdp. + * Subsequent error paths here will call gfs2_sys_fs_del, which also + * kobject_put to free sdp. + */ if (error) - goto fail; + return error; + + gfs2_create_debugfs_file(sdp); error = gfs2_lm_mount(sdp, silent); if (error) - goto fail_sys; + goto fail_debug; error = init_locking(sdp, &mount_gh, DO); if (error) @@ -1220,12 +1233,12 @@ fail_locking: fail_lm: gfs2_gl_hash_clear(sdp); gfs2_lm_unmount(sdp); -fail_sys: - gfs2_sys_fs_del(sdp); -fail: +fail_debug: gfs2_delete_debugfs_file(sdp); free_percpu(sdp->sd_lkstats); - kfree(sdp); + /* gfs2_sys_fs_del must be the last thing we do, since it causes + * sysfs to call function gfs2_sbd_release, which frees sdp. */ + gfs2_sys_fs_del(sdp); sb->s_fs_info = NULL; return error; } @@ -1395,10 +1408,9 @@ static void gfs2_kill_sb(struct super_block *sb) sdp->sd_root_dir = NULL; sdp->sd_master_dir = NULL; shrink_dcache_sb(sb); - kill_block_super(sb); gfs2_delete_debugfs_file(sdp); free_percpu(sdp->sd_lkstats); - kfree(sdp); + kill_block_super(sb); } struct file_system_type gfs2_fs_type = { diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c index d33172c..1a0fad9 100644 --- a/fs/gfs2/sys.c +++ b/fs/gfs2/sys.c @@ -276,7 +276,15 @@ static struct attribute *gfs2_attrs[] = { NULL, }; +static void gfs2_sbd_release(struct kobject *kobj) +{ + struct gfs2_sbd *sdp = container_of(kobj, struct gfs2_sbd, sd_kobj); + + kfree(sdp); +} + static struct kobj_type gfs2_ktype = { + .release = gfs2_sbd_release, .default_attrs = gfs2_attrs, .sysfs_ops = &gfs2_attr_ops, }; @@ -581,6 +589,7 @@ int gfs2_sys_fs_add(struct gfs2_sbd *sdp) char ro[20]; char spectator[20]; char *envp[] = { ro, spectator, NULL }; + int sysfs_frees_sdp = 0; sprintf(ro, "RDONLY=%d", (sb->s_flags & MS_RDONLY) ? 1 : 0); sprintf(spectator, "SPECTATOR=%d", sdp->sd_args.ar_spectator ? 1 : 0); @@ -589,8 +598,10 @@ int gfs2_sys_fs_add(struct gfs2_sbd *sdp) error = kobject_init_and_add(&sdp->sd_kobj, &gfs2_ktype, NULL, "%s", sdp->sd_table_name); if (error) - goto fail; + goto fail_reg; + sysfs_frees_sdp = 1; /* Freeing sdp is now done by sysfs calling + function gfs2_sbd_release. */ error = sysfs_create_group(&sdp->sd_kobj, &tune_group); if (error) goto fail_reg; @@ -613,9 +624,13 @@ fail_lock_module: fail_tune: sysfs_remove_group(&sdp->sd_kobj, &tune_group); fail_reg: - kobject_put(&sdp->sd_kobj); -fail: + free_percpu(sdp->sd_lkstats); fs_err(sdp, "error %d adding sysfs files", error); + if (sysfs_frees_sdp) + kobject_put(&sdp->sd_kobj); + else + kfree(sdp); + sb->s_fs_info = NULL; return error; } ^ permalink raw reply related [flat|nested] 2+ messages in thread
* [Cluster-devel] [GFS2 Patch] GFS2: Add kobject release method 2012-06-13 14:27 ` [Cluster-devel] [GFS2 Patch] GFS2: Add kobject release method Bob Peterson @ 2012-06-13 15:52 ` Steven Whitehouse 0 siblings, 0 replies; 2+ messages in thread From: Steven Whitehouse @ 2012-06-13 15:52 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, Now in the -nmw git tree. Thanks, Steve. On Wed, 2012-06-13 at 10:27 -0400, Bob Peterson wrote: > Hi, > > This patch adds a kobject release function that properly maintains > the kobject use count, so that accesses to the sysfs files do not > cause an access to freed kernel memory after an unmount. > > Regards, > > Bob Peterson > Red Hat File Systems > > Signed-off-by: Bob Peterson <rpeterso@redhat.com> > --- > diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c > index c5871ae..378c90f 100644 > --- a/fs/gfs2/ops_fstype.c > +++ b/fs/gfs2/ops_fstype.c > @@ -1123,20 +1123,33 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent > } > > error = init_names(sdp, silent); > - if (error) > - goto fail; > + if (error) { > + /* In this case, we haven't initialized sysfs, so we have to > + manually free the sdp. */ > + free_percpu(sdp->sd_lkstats); > + kfree(sdp); > + sb->s_fs_info = NULL; > + return error; > + } > > snprintf(sdp->sd_fsname, GFS2_FSNAME_LEN, "%s", sdp->sd_table_name); > > - gfs2_create_debugfs_file(sdp); > - > error = gfs2_sys_fs_add(sdp); > + /* > + * If we hit an error here, gfs2_sys_fs_add will have called function > + * kobject_put which causes the sysfs usage count to go to zero, which > + * causes sysfs to call function gfs2_sbd_release, which frees sdp. > + * Subsequent error paths here will call gfs2_sys_fs_del, which also > + * kobject_put to free sdp. > + */ > if (error) > - goto fail; > + return error; > + > + gfs2_create_debugfs_file(sdp); > > error = gfs2_lm_mount(sdp, silent); > if (error) > - goto fail_sys; > + goto fail_debug; > > error = init_locking(sdp, &mount_gh, DO); > if (error) > @@ -1220,12 +1233,12 @@ fail_locking: > fail_lm: > gfs2_gl_hash_clear(sdp); > gfs2_lm_unmount(sdp); > -fail_sys: > - gfs2_sys_fs_del(sdp); > -fail: > +fail_debug: > gfs2_delete_debugfs_file(sdp); > free_percpu(sdp->sd_lkstats); > - kfree(sdp); > + /* gfs2_sys_fs_del must be the last thing we do, since it causes > + * sysfs to call function gfs2_sbd_release, which frees sdp. */ > + gfs2_sys_fs_del(sdp); > sb->s_fs_info = NULL; > return error; > } > @@ -1395,10 +1408,9 @@ static void gfs2_kill_sb(struct super_block *sb) > sdp->sd_root_dir = NULL; > sdp->sd_master_dir = NULL; > shrink_dcache_sb(sb); > - kill_block_super(sb); > gfs2_delete_debugfs_file(sdp); > free_percpu(sdp->sd_lkstats); > - kfree(sdp); > + kill_block_super(sb); > } > > struct file_system_type gfs2_fs_type = { > diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c > index d33172c..1a0fad9 100644 > --- a/fs/gfs2/sys.c > +++ b/fs/gfs2/sys.c > @@ -276,7 +276,15 @@ static struct attribute *gfs2_attrs[] = { > NULL, > }; > > +static void gfs2_sbd_release(struct kobject *kobj) > +{ > + struct gfs2_sbd *sdp = container_of(kobj, struct gfs2_sbd, sd_kobj); > + > + kfree(sdp); > +} > + > static struct kobj_type gfs2_ktype = { > + .release = gfs2_sbd_release, > .default_attrs = gfs2_attrs, > .sysfs_ops = &gfs2_attr_ops, > }; > @@ -581,6 +589,7 @@ int gfs2_sys_fs_add(struct gfs2_sbd *sdp) > char ro[20]; > char spectator[20]; > char *envp[] = { ro, spectator, NULL }; > + int sysfs_frees_sdp = 0; > > sprintf(ro, "RDONLY=%d", (sb->s_flags & MS_RDONLY) ? 1 : 0); > sprintf(spectator, "SPECTATOR=%d", sdp->sd_args.ar_spectator ? 1 : 0); > @@ -589,8 +598,10 @@ int gfs2_sys_fs_add(struct gfs2_sbd *sdp) > error = kobject_init_and_add(&sdp->sd_kobj, &gfs2_ktype, NULL, > "%s", sdp->sd_table_name); > if (error) > - goto fail; > + goto fail_reg; > > + sysfs_frees_sdp = 1; /* Freeing sdp is now done by sysfs calling > + function gfs2_sbd_release. */ > error = sysfs_create_group(&sdp->sd_kobj, &tune_group); > if (error) > goto fail_reg; > @@ -613,9 +624,13 @@ fail_lock_module: > fail_tune: > sysfs_remove_group(&sdp->sd_kobj, &tune_group); > fail_reg: > - kobject_put(&sdp->sd_kobj); > -fail: > + free_percpu(sdp->sd_lkstats); > fs_err(sdp, "error %d adding sysfs files", error); > + if (sysfs_frees_sdp) > + kobject_put(&sdp->sd_kobj); > + else > + kfree(sdp); > + sb->s_fs_info = NULL; > return error; > } > > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-06-13 15:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5da8935b-4362-4b87-b2ef-2753ce1edb0b@zmail12.collab.prod.int.phx2.redhat.com>
2012-06-13 14:27 ` [Cluster-devel] [GFS2 Patch] GFS2: Add kobject release method Bob Peterson
2012-06-13 15:52 ` Steven Whitehouse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).