| Message ID | 20210408113132.87250-1-wangwensheng4@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] RDMA/srpt: Fix error return code in srpt_cm_req_recv() | expand |
On 4/8/21 4:31 AM, Wang Wensheng wrote: > Fix to return a negative error code from the error handling > case instead of 0, as done elsewhere in this function. > > Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com> > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c > index 98a393d..ea44780 100644 > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", > dev_name(&sdev->device->dev), port_num); > mutex_unlock(&sport->mutex); > + ret = -EINVAL; > goto reject; > } Please fix the Hulk Robot. The following code occurs three lines above the modified code: ret = -EINVAL; Thanks, Bart.
On Thu, Apr 08, 2021 at 09:50:30AM -0700, Bart Van Assche wrote: > On 4/8/21 4:31 AM, Wang Wensheng wrote: > > Fix to return a negative error code from the error handling > > case instead of 0, as done elsewhere in this function. > > > > Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") > > Reported-by: Hulk Robot <hulkci@huawei.com> > > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com> > > drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c > > index 98a393d..ea44780 100644 > > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > > @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, > > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", > > dev_name(&sdev->device->dev), port_num); > > mutex_unlock(&sport->mutex); > > + ret = -EINVAL; > > goto reject; > > } > > Please fix the Hulk Robot. The following code occurs three lines above > the modified code: > > ret = -EINVAL; That is a different if. The patch looks correct to me, please check again: ret = srpt_create_ch_ib(ch); if (ret) { // Ret is proven to be 0 [..] if (!sport->enabled) { rej->reason = cpu_to_be32( SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES); pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", dev_name(&sdev->device->dev), port_num); goto reject; // Ret is 0 If there is an assignment to ret between those two blocks it is hidden.. Jason
On 4/12/21 11:09 AM, Jason Gunthorpe wrote: > On Thu, Apr 08, 2021 at 09:50:30AM -0700, Bart Van Assche wrote: >> On 4/8/21 4:31 AM, Wang Wensheng wrote: >>> Fix to return a negative error code from the error handling >>> case instead of 0, as done elsewhere in this function. >>> >>> Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") >>> Reported-by: Hulk Robot <hulkci@huawei.com> >>> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com> >>> drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c >>> index 98a393d..ea44780 100644 >>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c >>> @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, >>> pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", >>> dev_name(&sdev->device->dev), port_num); >>> mutex_unlock(&sport->mutex); >>> + ret = -EINVAL; >>> goto reject; >>> } >> >> Please fix the Hulk Robot. The following code occurs three lines above >> the modified code: >> >> ret = -EINVAL; > > That is a different if. > > The patch looks correct to me, please check again: > > ret = srpt_create_ch_ib(ch); > if (ret) { > // Ret is proven to be 0 > > [..] > > if (!sport->enabled) { > rej->reason = cpu_to_be32( > SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES); > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", > dev_name(&sdev->device->dev), port_num); > goto reject; // Ret is 0 > > If there is an assignment to ret between those two blocks it is hidden.. Right, I was looking at another if-statement in the same function. Bart.
On 4/8/21 4:31 AM, Wang Wensheng wrote: > Fix to return a negative error code from the error handling > case instead of 0, as done elsewhere in this function. > > Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com> > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c > index 98a393d..ea44780 100644 > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", > dev_name(&sdev->device->dev), port_num); > mutex_unlock(&sport->mutex); > + ret = -EINVAL; > goto reject; > } Reviewed-by: Bart Van Assche <bvanassche@acm.org>
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index 98a393d..ea44780 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", dev_name(&sdev->device->dev), port_num); mutex_unlock(&sport->mutex); + ret = -EINVAL; goto reject; }
Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com> --- drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + 1 file changed, 1 insertion(+)