Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On Thu, Mar 28, 2019 at 07:50:23PM +0530, Potnuri Bharat Teja wrote: > On Thursday, March 03/28/19, 2019 at 18:10:37 +0530, Jason Gunthorpe wrote: > > On Wed, Mar 27, 2019 at 07:08:54PM +0530, Potnuri Bharat Teja wrote: > > > On Saturday, March 03/23/19, 2019 at 08:07:46 +0530, Kangjie Lu wrote: > > > > > > > > > > > > > On Mar 8, 2019, at 11:19 PM, Kangjie Lu wrote: > > > > > > > > > > get_skb may fail and return NULL. The fix returns "ENOMEM" > > > > > when it fails to avoid NULL dereference. > > > > > > > > > > Signed-off-by: Kangjie Lu > > > > > drivers/infiniband/hw/cxgb4/cm.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c > > > > > b/drivers/infiniband/hw/cxgb4/cm.c > > > > > index 8221813219e5..502a54d57e2c 100644 > > > > > +++ b/drivers/infiniband/hw/cxgb4/cm.c > > > > > @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep > > > > > *ep, unsigned int atid) > > > > > int win; > > > > > > > > > > skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); > > > > > + if (!skb) > > > > > + return -ENOMEM; > > > > > + > > > > > > > > Can someone review this patch? Thanks. > > > > > > Sorry for the late response. > > > I recommend an error print before the return. > > > if (!skb) { > > > pr_err("%s - failed to alloc skb\n", __func__); > > > return -ENOMEM; > > > } > > > > no error prints on memory allocation failure, the kernel already > > prints enough on this > Ok. > > Acked-by: Potnuri Bharat Teja It needs to be resent with Bart's comment addressed, and all the tags collected. Jason
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On Thursday, March 03/28/19, 2019 at 18:10:37 +0530, Jason Gunthorpe wrote: > On Wed, Mar 27, 2019 at 07:08:54PM +0530, Potnuri Bharat Teja wrote: > > On Saturday, March 03/23/19, 2019 at 08:07:46 +0530, Kangjie Lu wrote: > > > > > > > > > > On Mar 8, 2019, at 11:19 PM, Kangjie Lu wrote: > > > > > > > > get_skb may fail and return NULL. The fix returns "ENOMEM" > > > > when it fails to avoid NULL dereference. > > > > > > > > Signed-off-by: Kangjie Lu > > > > drivers/infiniband/hw/cxgb4/cm.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c > > > > b/drivers/infiniband/hw/cxgb4/cm.c > > > > index 8221813219e5..502a54d57e2c 100644 > > > > +++ b/drivers/infiniband/hw/cxgb4/cm.c > > > > @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep > > > > *ep, unsigned int atid) > > > > int win; > > > > > > > > skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); > > > > + if (!skb) > > > > + return -ENOMEM; > > > > + > > > > > > Can someone review this patch? Thanks. > > > > Sorry for the late response. > > I recommend an error print before the return. > > if (!skb) { > > pr_err("%s - failed to alloc skb\n", __func__); > > return -ENOMEM; > > } > > no error prints on memory allocation failure, the kernel already > prints enough on this Ok. Acked-by: Potnuri Bharat Teja
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On Wed, Mar 27, 2019 at 07:08:54PM +0530, Potnuri Bharat Teja wrote: > On Saturday, March 03/23/19, 2019 at 08:07:46 +0530, Kangjie Lu wrote: > > > > > > > On Mar 8, 2019, at 11:19 PM, Kangjie Lu wrote: > > > > > > get_skb may fail and return NULL. The fix returns "ENOMEM" > > > when it fails to avoid NULL dereference. > > > > > > Signed-off-by: Kangjie Lu > > > drivers/infiniband/hw/cxgb4/cm.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c > > > b/drivers/infiniband/hw/cxgb4/cm.c > > > index 8221813219e5..502a54d57e2c 100644 > > > +++ b/drivers/infiniband/hw/cxgb4/cm.c > > > @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, > > > unsigned int atid) > > > int win; > > > > > > skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); > > > + if (!skb) > > > + return -ENOMEM; > > > + > > > > Can someone review this patch? Thanks. > > Sorry for the late response. > I recommend an error print before the return. > if (!skb) { > pr_err("%s - failed to alloc skb\n", __func__); > return -ENOMEM; > } no error prints on memory allocation failure, the kernel already prints enough on this Jason
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On Saturday, March 03/23/19, 2019 at 08:07:46 +0530, Kangjie Lu wrote: > > > > On Mar 8, 2019, at 11:19 PM, Kangjie Lu wrote: > > > > get_skb may fail and return NULL. The fix returns "ENOMEM" > > when it fails to avoid NULL dereference. > > > > Signed-off-by: Kangjie Lu > > --- > > drivers/infiniband/hw/cxgb4/cm.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c > > b/drivers/infiniband/hw/cxgb4/cm.c > > index 8221813219e5..502a54d57e2c 100644 > > --- a/drivers/infiniband/hw/cxgb4/cm.c > > +++ b/drivers/infiniband/hw/cxgb4/cm.c > > @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, > > unsigned int atid) > > int win; > > > > skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); > > + if (!skb) > > + return -ENOMEM; > > + > > Can someone review this patch? Thanks. Sorry for the late response. I recommend an error print before the return. --- if (!skb) { pr_err("%s - failed to alloc skb\n", __func__); return -ENOMEM; } --- Thanks for the patch! - Bharat. > > > req = __skb_put_zero(skb, sizeof(*req)); > > req->op_compl = htonl(WR_OP_V(FW_OFLD_CONNECTION_WR)); > > req->len16_pkd = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16))); > > -- > > 2.17.1 > > >
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On 3/9/2019 10:49 AM, Kangjie Lu wrote: get_skb may fail and return NULL. The fix returns "ENOMEM" when it fails to avoid NULL dereference. Signed-off-by: Kangjie Lu Reviewed-by: Mukesh OJha --- drivers/infiniband/hw/cxgb4/cm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c index 8221813219e5..502a54d57e2c 100644 --- a/drivers/infiniband/hw/cxgb4/cm.c +++ b/drivers/infiniband/hw/cxgb4/cm.c @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, unsigned int atid) int win; skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); + if (!skb) + return -ENOMEM; + req = __skb_put_zero(skb, sizeof(*req)); req->op_compl = htonl(WR_OP_V(FW_OFLD_CONNECTION_WR)); req->len16_pkd = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16)));
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On 3/9/2019 10:49 AM, Kangjie Lu wrote: get_skb may fail and return NULL. The fix returns "ENOMEM" when it fails to avoid NULL dereference. Signed-off-by: Kangjie Lu Reviewed-by: Mukesh Ojha --- drivers/infiniband/hw/cxgb4/cm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c index 8221813219e5..502a54d57e2c 100644 --- a/drivers/infiniband/hw/cxgb4/cm.c +++ b/drivers/infiniband/hw/cxgb4/cm.c @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, unsigned int atid) int win; skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); + if (!skb) + return -ENOMEM; + req = __skb_put_zero(skb, sizeof(*req)); req->op_compl = htonl(WR_OP_V(FW_OFLD_CONNECTION_WR)); req->len16_pkd = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16)));
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
On 3/22/19 7:37 PM, Kangjie Lu wrote: On Mar 8, 2019, at 11:19 PM, Kangjie Lu wrote: get_skb may fail and return NULL. The fix returns "ENOMEM" when it fails to avoid NULL dereference. Signed-off-by: Kangjie Lu --- drivers/infiniband/hw/cxgb4/cm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c index 8221813219e5..502a54d57e2c 100644 --- a/drivers/infiniband/hw/cxgb4/cm.c +++ b/drivers/infiniband/hw/cxgb4/cm.c @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, unsigned int atid) int win; skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); + if (!skb) + return -ENOMEM; + Can someone review this patch? Thanks. Hi Kangjie, Please change the patch description to the imperative mood (see also https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/SubmittingPatches). A good way to invite feedback is to add a relevant Cc-list to a patch. The output of scripts/get_maintainer.pl can be a good start. Bart.
Re: [PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
> On Mar 8, 2019, at 11:19 PM, Kangjie Lu wrote: > > get_skb may fail and return NULL. The fix returns "ENOMEM" > when it fails to avoid NULL dereference. > > Signed-off-by: Kangjie Lu > --- > drivers/infiniband/hw/cxgb4/cm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c > b/drivers/infiniband/hw/cxgb4/cm.c > index 8221813219e5..502a54d57e2c 100644 > --- a/drivers/infiniband/hw/cxgb4/cm.c > +++ b/drivers/infiniband/hw/cxgb4/cm.c > @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, > unsigned int atid) > int win; > > skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); > + if (!skb) > + return -ENOMEM; > + Can someone review this patch? Thanks. > req = __skb_put_zero(skb, sizeof(*req)); > req->op_compl = htonl(WR_OP_V(FW_OFLD_CONNECTION_WR)); > req->len16_pkd = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16))); > -- > 2.17.1 >
[PATCH] infiniband: cxgb4: fix a potential NULL pointer dereference
get_skb may fail and return NULL. The fix returns "ENOMEM" when it fails to avoid NULL dereference. Signed-off-by: Kangjie Lu --- drivers/infiniband/hw/cxgb4/cm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c index 8221813219e5..502a54d57e2c 100644 --- a/drivers/infiniband/hw/cxgb4/cm.c +++ b/drivers/infiniband/hw/cxgb4/cm.c @@ -1919,6 +1919,9 @@ static int send_fw_act_open_req(struct c4iw_ep *ep, unsigned int atid) int win; skb = get_skb(NULL, sizeof(*req), GFP_KERNEL); + if (!skb) + return -ENOMEM; + req = __skb_put_zero(skb, sizeof(*req)); req->op_compl = htonl(WR_OP_V(FW_OFLD_CONNECTION_WR)); req->len16_pkd = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16))); -- 2.17.1