| Message ID | 20170314104843.1358-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
Thanks! regards, dan carpenter
On Tue, Mar 14, 2017 at 10:48:43AM +0000, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > On the error exit path, skb is always null, so the non-null check > and __kfree_skb call are redundant. Remove the redundant code and > just directly return with the appropriate error return code. > > Detected by CoverityScan, CID#114328 ("Logically Dead Code") > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/scsi/cxgbi/cxgb3i/cxgb3i.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c > index 1880eb6..3c9f8cf2 100644 > --- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c > +++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c > @@ -979,14 +979,14 @@ static int init_act_open(struct cxgbi_sock *csk) > csk->atid = cxgb3_alloc_atid(t3dev, &t3_client, csk); > if (csk->atid < 0) { > pr_err("NO atid available.\n"); > - goto rel_resource; > + return -EINVAL; > } > cxgbi_sock_set_flag(csk, CTPF_HAS_ATID); > cxgbi_sock_get(csk); > > skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_KERNEL); > if (!skb) > - goto rel_resource; > + return -ENOMEM; I don't think that's correct, not that it was before. cxgbi_sock_get(csk) does a kref_get(&csk->refcnt), so this will at lease leak a kref. It will also "leak" the atids_in_use in cxgb3_alloc_atid() as there's a call to cxgb3_free_atid() missing. Looks like the complete cleanup path is worng here. But I'd prefer having Karen or someone else at Chelsio confirm my assumptions. Thanks, Johannes
diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c index 1880eb6..3c9f8cf2 100644 --- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c +++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c @@ -979,14 +979,14 @@ static int init_act_open(struct cxgbi_sock *csk) csk->atid = cxgb3_alloc_atid(t3dev, &t3_client, csk); if (csk->atid < 0) { pr_err("NO atid available.\n"); - goto rel_resource; + return -EINVAL; } cxgbi_sock_set_flag(csk, CTPF_HAS_ATID); cxgbi_sock_get(csk); skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_KERNEL); if (!skb) - goto rel_resource; + return -ENOMEM; skb->sk = (struct sock *)csk; set_arp_failure_handler(skb, act_open_arp_failure); csk->snd_win = cxgb3i_snd_win; @@ -1007,11 +1007,6 @@ static int init_act_open(struct cxgbi_sock *csk) cxgbi_sock_set_state(csk, CTP_ACTIVE_OPEN); send_act_open_req(csk, skb, csk->l2t); return 0; - -rel_resource: - if (skb) - __kfree_skb(skb); - return -EINVAL; } cxgb3_cpl_handler_func cxgb3i_cpl_handlers[NUM_CPL_CMDS] = {