| Message ID | 20180109200346.rlsrxa24xmltpz2r@mwanda (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma- > owner@vger.kernel.org] On Behalf Of Dan Carpenter > Sent: Tuesday, January 9, 2018 3:04 PM > To: Marciniszyn, Mike <mike.marciniszyn@intel.com> > Cc: Dalessandro, Dennis <dennis.dalessandro@intel.com>; Doug Ledford > <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; linux- > rdma@vger.kernel.org; kernel-janitors@vger.kernel.org > Subject: [PATCH v2] IB/hfi1: Prevent a NULL dereference > > In the original code, we set "fd->uctxt" to NULL and then dereference it > which will cause an Oops. > > Fixes: f2a3bc00a03c ("IB/hfi1: Protect context array set/clear with spinlock") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > v2: In v1, I accidentally moved the __clear_bit() out from under the > spin_lock(). Thanks for the review, Michael! > > diff --git a/drivers/infiniband/hw/hfi1/file_ops.c > b/drivers/infiniband/hw/hfi1/file_ops.c > index 82086241aac3..bd6f03cc5ee0 100644 > --- a/drivers/infiniband/hw/hfi1/file_ops.c > +++ b/drivers/infiniband/hw/hfi1/file_ops.c > @@ -763,11 +763,11 @@ static int complete_subctxt(struct hfi1_filedata *fd) > } > > if (ret) { > - hfi1_rcd_put(fd->uctxt); > - fd->uctxt = NULL; > spin_lock_irqsave(&fd->dd->uctxt_lock, flags); > __clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts); > spin_unlock_irqrestore(&fd->dd->uctxt_lock, flags); > + hfi1_rcd_put(fd->uctxt); > + fd->uctxt = NULL; > } > > return ret; > -- Cc: <stable@vger.kernel.org> # 4.14.x Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> Looks good. Adding tag for stable too. Thanks, Mike > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2018-01-09 at 20:39 +0000, Ruhl, Michael J wrote: > > -----Original Message----- > > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma- > > owner@vger.kernel.org] On Behalf Of Dan Carpenter > > Sent: Tuesday, January 9, 2018 3:04 PM > > To: Marciniszyn, Mike <mike.marciniszyn@intel.com> > > Cc: Dalessandro, Dennis <dennis.dalessandro@intel.com>; Doug Ledford > > <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; linux- > > rdma@vger.kernel.org; kernel-janitors@vger.kernel.org > > Subject: [PATCH v2] IB/hfi1: Prevent a NULL dereference > > > > In the original code, we set "fd->uctxt" to NULL and then dereference it > > which will cause an Oops. > > > > Fixes: f2a3bc00a03c ("IB/hfi1: Protect context array set/clear with spinlock") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > v2: In v1, I accidentally moved the __clear_bit() out from under the > > spin_lock(). Thanks for the review, Michael! > > > > diff --git a/drivers/infiniband/hw/hfi1/file_ops.c > > b/drivers/infiniband/hw/hfi1/file_ops.c > > index 82086241aac3..bd6f03cc5ee0 100644 > > --- a/drivers/infiniband/hw/hfi1/file_ops.c > > +++ b/drivers/infiniband/hw/hfi1/file_ops.c > > @@ -763,11 +763,11 @@ static int complete_subctxt(struct hfi1_filedata *fd) > > } > > > > if (ret) { > > - hfi1_rcd_put(fd->uctxt); > > - fd->uctxt = NULL; > > spin_lock_irqsave(&fd->dd->uctxt_lock, flags); > > __clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts); > > spin_unlock_irqrestore(&fd->dd->uctxt_lock, flags); > > + hfi1_rcd_put(fd->uctxt); > > + fd->uctxt = NULL; > > } > > > > return ret; > > -- > > Cc: <stable@vger.kernel.org> # 4.14.x Unfortunately, patchworks doesn't pick up additional Cc: tags, only reviewed-bys and acks. In any case, I hand added it. Thanks, applied to for-rc. > Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > > Looks good. Adding tag for stable too. > > Thanks, > > Mike > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index 82086241aac3..bd6f03cc5ee0 100644 --- a/drivers/infiniband/hw/hfi1/file_ops.c +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -763,11 +763,11 @@ static int complete_subctxt(struct hfi1_filedata *fd) } if (ret) { - hfi1_rcd_put(fd->uctxt); - fd->uctxt = NULL; spin_lock_irqsave(&fd->dd->uctxt_lock, flags); __clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts); spin_unlock_irqrestore(&fd->dd->uctxt_lock, flags); + hfi1_rcd_put(fd->uctxt); + fd->uctxt = NULL; } return ret;
In the original code, we set "fd->uctxt" to NULL and then dereference it which will cause an Oops. Fixes: f2a3bc00a03c ("IB/hfi1: Protect context array set/clear with spinlock") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- v2: In v1, I accidentally moved the __clear_bit() out from under the spin_lock(). Thanks for the review, Michael! -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html