Messages in this thread | | | Date | Tue, 24 Mar 2015 10:21:21 +0000 | From | Steven Whitehouse <> | Subject | Re: [PATCH 1/1] gfs2: incorrect check for debugfs returns |
| |
Hi,
On 24/03/15 02:36, Chengyu Song wrote: > debugfs_create_dir and debugfs_create_file may return -ENODEV when debugfs > is not configured, so the return value should be checked against ERROR_VALUE > as well, otherwise the later dereference of the dentry pointer would crash > the kernel. Looks reasonable to me:
Acked-by: Steven Whitehouse <swhiteho@redhat.com>
Steve.
> Signed-off-by: Chengyu Song <csong84@gatech.edu> > --- > fs/gfs2/glock.c | 47 ++++++++++++++++++++++++++++------------------- > 1 file changed, 28 insertions(+), 19 deletions(-) > > diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c > index f42dffb..0fa8062 100644 > --- a/fs/gfs2/glock.c > +++ b/fs/gfs2/glock.c > @@ -2047,34 +2047,41 @@ static const struct file_operations gfs2_sbstats_fops = { > > int gfs2_create_debugfs_file(struct gfs2_sbd *sdp) > { > - sdp->debugfs_dir = debugfs_create_dir(sdp->sd_table_name, gfs2_root); > - if (!sdp->debugfs_dir) > - return -ENOMEM; > - sdp->debugfs_dentry_glocks = debugfs_create_file("glocks", > - S_IFREG | S_IRUGO, > - sdp->debugfs_dir, sdp, > - &gfs2_glocks_fops); > - if (!sdp->debugfs_dentry_glocks) > + struct dentry *dent; > + > + dent = debugfs_create_dir(sdp->sd_table_name, gfs2_root); > + if (IS_ERR_OR_NULL(dent)) > + goto fail; > + sdp->debugfs_dir = dent; > + > + dent = debugfs_create_file("glocks", > + S_IFREG | S_IRUGO, > + sdp->debugfs_dir, sdp, > + &gfs2_glocks_fops); > + if (IS_ERR_OR_NULL(dent)) > goto fail; > + sdp->debugfs_dentry_glocks = dent; > > - sdp->debugfs_dentry_glstats = debugfs_create_file("glstats", > - S_IFREG | S_IRUGO, > - sdp->debugfs_dir, sdp, > - &gfs2_glstats_fops); > - if (!sdp->debugfs_dentry_glstats) > + dent = debugfs_create_file("glstats", > + S_IFREG | S_IRUGO, > + sdp->debugfs_dir, sdp, > + &gfs2_glstats_fops); > + if (IS_ERR_OR_NULL(dent)) > goto fail; > + sdp->debugfs_dentry_glstats = dent; > > - sdp->debugfs_dentry_sbstats = debugfs_create_file("sbstats", > - S_IFREG | S_IRUGO, > - sdp->debugfs_dir, sdp, > - &gfs2_sbstats_fops); > - if (!sdp->debugfs_dentry_sbstats) > + dent = debugfs_create_file("sbstats", > + S_IFREG | S_IRUGO, > + sdp->debugfs_dir, sdp, > + &gfs2_sbstats_fops); > + if (IS_ERR_OR_NULL(dent)) > goto fail; > + sdp->debugfs_dentry_sbstats = dent; > > return 0; > fail: > gfs2_delete_debugfs_file(sdp); > - return -ENOMEM; > + return dent ? PTR_ERR(dent) : -ENOMEM; > } > > void gfs2_delete_debugfs_file(struct gfs2_sbd *sdp) > @@ -2100,6 +2107,8 @@ void gfs2_delete_debugfs_file(struct gfs2_sbd *sdp) > int gfs2_register_debugfs(void) > { > gfs2_root = debugfs_create_dir("gfs2", NULL); > + if (IS_ERR(gfs2_root)) > + return PTR_ERR(gfs2_root); > return gfs2_root ? 0 : -ENOMEM; > } >
| |