From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Wed, 19 Dec 2012 16:23:10 +0000 Subject: [Cluster-devel] [GFS2 PATCH] [bz878476] - Fix race in gfs2_rs_alloc In-Reply-To: <1725701894.66056191.1355932081651.JavaMail.root@redhat.com> References: <1725701894.66056191.1355932081651.JavaMail.root@redhat.com> Message-ID: <1355934190.2740.33.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, I've added that to my tree of pending patches. That will be in the -nmw tree just as soon as -rc1 is out. Thanks, Steve. On Wed, 2012-12-19 at 10:48 -0500, Abhijith Das wrote: > QE aio tests uncovered a race condition in gfs2_rs_alloc where it's possible to come out of the function with a valid ip->i_res allocation but it gets freed before use resulting in a NULL ptr dereference. > > This patch envelopes the initial short-circuit check for non-NULL ip->i_res into the mutex lock. With this patch, I was able to successfully run the reproducer test multiple times. > > Resolves: rhbz#878476 > Signed-off-by: Abhi Das > > diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c > index 37ee061..738b388 100644 > --- a/fs/gfs2/rgrp.c > +++ b/fs/gfs2/rgrp.c > @@ -557,22 +557,20 @@ void gfs2_free_clones(struct gfs2_rgrpd *rgd) > */ > int gfs2_rs_alloc(struct gfs2_inode *ip) > { > - struct gfs2_blkreserv *res; > + int error = 0; > > + down_write(&ip->i_rw_mutex); > if (ip->i_res) > - return 0; > - > - res = kmem_cache_zalloc(gfs2_rsrv_cachep, GFP_NOFS); > - if (!res) > - return -ENOMEM; > + goto out; > > - RB_CLEAR_NODE(&res->rs_node); > + ip->i_res = kmem_cache_zalloc(gfs2_rsrv_cachep, GFP_NOFS); > + if (!ip->i_res) { > + error = -ENOMEM; > + goto out; > + } > > - down_write(&ip->i_rw_mutex); > - if (ip->i_res) > - kmem_cache_free(gfs2_rsrv_cachep, res); > - else > - ip->i_res = res; > + RB_CLEAR_NODE(&ip->i_res->rs_node); > +out: > up_write(&ip->i_rw_mutex); > return 0; > } >