Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751382AbdFEHpW (ORCPT ); Mon, 5 Jun 2017 03:45:22 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:26003 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751321AbdFEHpV (ORCPT ); Mon, 5 Jun 2017 03:45:21 -0400 Date: Mon, 5 Jun 2017 10:44:56 +0300 From: Yuval Shaia To: Jia-Ju Bai Cc: monis@mellanox.com, sean.hefty@intel.com, dledford@redhat.com, hal.rosenstock@gmail.com, leon@kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH V3] rxe: Fix a sleep-in-atomic bug in post_one_send Message-ID: <20170605074455.GF2572@yuvallap> References: <1496648342-906-1-git-send-email-baijiaju1990@163.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1496648342-906-1-git-send-email-baijiaju1990@163.com> User-Agent: Mutt/1.8.0 (2017-02-23) X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2750 Lines: 78 On Mon, Jun 05, 2017 at 03:39:02PM +0800, Jia-Ju Bai wrote: > The driver may sleep under a spin lock, and the function call path is: > post_one_send (acquire the lock by spin_lock_irqsave) > init_send_wqe > copy_from_user --> may sleep > > To fix it, the lock is released before copy_from_user, and the lock is > acquired again after this function. The parameter "flags" is used to > restore and save the irq status. > > > Signed-off-by: Jia-Ju Bai > --- > V3: > * It corrects the mistakes of remaining legacy code in V2. > (Thank Ram for pointing it out) > > V2: > * The parameter "flags" is added to restore and save the irq status. > Thank Leon for good advice. > My mistake for not mentioning it specifically but in addition to moving such version history after the "---" you should also add (manually) a terminating "---" after the block. > drivers/infiniband/sw/rxe/rxe_verbs.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c > index 83d709e..5293d15 100644 > --- a/drivers/infiniband/sw/rxe/rxe_verbs.c > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c > @@ -721,11 +721,11 @@ static void init_send_wr(struct rxe_qp *qp, struct rxe_send_wr *wr, > > static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, > unsigned int mask, unsigned int length, > - struct rxe_send_wqe *wqe) > + struct rxe_send_wqe *wqe, unsigned long *flags) > { > int num_sge = ibwr->num_sge; > struct ib_sge *sge; > - int i; > + int i, err; > u8 *p; > > init_send_wr(qp, &wqe->wr, ibwr); > @@ -740,8 +740,11 @@ static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, > > sge = ibwr->sg_list; > for (i = 0; i < num_sge; i++, sge++) { > - if (qp->is_user && copy_from_user(p, (__user void *) > - (uintptr_t)sge->addr, sge->length)) > + spin_unlock_irqrestore(&qp->sq.sq_lock, *flags); > + err = copy_from_user(p, (__user void *) > + (uintptr_t)sge->addr, sge->length); > + spin_lock_irqsave(&qp->sq.sq_lock, *flags); > + if (qp->is_user && err) > return -EFAULT; > > else if (!qp->is_user) > @@ -794,7 +797,7 @@ static int post_one_send(struct rxe_qp *qp, struct ib_send_wr *ibwr, > > send_wqe = producer_addr(sq->queue); > > - err = init_send_wqe(qp, ibwr, mask, length, send_wqe); > + err = init_send_wqe(qp, ibwr, mask, length, send_wqe, &flags); > if (unlikely(err)) > goto err1; > > -- > 1.7.9.5 > > > -- > 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