| Message ID | 20221114110626.526643-1-weiyongjun@huaweicloud.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | scsi: bnx2fc: Fix skb double free in bnx2fc_rcv() | expand |
What ever happened to this patch? I was reviewing old use after free static checker warnings (Smatch) and came across it. The patch looks correct to me (I wrote the exact same patch myself before seeing this one on lore). regards, dan carpenter On Mon, Nov 14, 2022 at 11:06:26AM +0000, Wei Yongjun wrote: > From: Wei Yongjun <weiyongjun1@huawei.com> > > skb_share_check() already drop the reference of skb when return > NULL, using kfree_skb() in the error handling path lead to skb > double free. > > Fix it by remve the variable tmp_skb, and return directly when > skb_share_check() return NULL. > > Fixes: 01a4cc4d0cd6 ("bnx2fc: do not add shared skbs to the fcoe_rx_list") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- > drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > index 05ddbb9bb7d8..451a58e0fd96 100644 > --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > @@ -429,7 +429,6 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev, > struct fcoe_ctlr *ctlr; > struct fcoe_rcv_info *fr; > struct fcoe_percpu_s *bg; > - struct sk_buff *tmp_skb; > > interface = container_of(ptype, struct bnx2fc_interface, > fcoe_packet_type); > @@ -441,11 +440,9 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev, > goto err; > } > > - tmp_skb = skb_share_check(skb, GFP_ATOMIC); > - if (!tmp_skb) > - goto err; > - > - skb = tmp_skb; > + skb = skb_share_check(skb, GFP_ATOMIC); > + if (!skb) > + return -1; > > if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) { > printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n"); > -- > 2.34.1 >
Dan, > What ever happened to this patch? I was reviewing old use after free > static checker warnings (Smatch) and came across it. The patch looks > correct to me (I wrote the exact same patch myself before seeing this > one on lore). Not sure what happened. Patchwork had it tagged as "New/archived" which is really peculiar. In any case I have applied the patch to 6.7/scsi-fixes, thanks!
On Mon, 14 Nov 2022 11:06:26 +0000, Wei Yongjun wrote: > skb_share_check() already drop the reference of skb when return > NULL, using kfree_skb() in the error handling path lead to skb > double free. > > Fix it by remve the variable tmp_skb, and return directly when > skb_share_check() return NULL. > > [...] Applied to 6.7/scsi-fixes, thanks! [1/1] scsi: bnx2fc: Fix skb double free in bnx2fc_rcv() https://git.kernel.org/mkp/scsi/c/08c94d80b2da
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 05ddbb9bb7d8..451a58e0fd96 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -429,7 +429,6 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev, struct fcoe_ctlr *ctlr; struct fcoe_rcv_info *fr; struct fcoe_percpu_s *bg; - struct sk_buff *tmp_skb; interface = container_of(ptype, struct bnx2fc_interface, fcoe_packet_type); @@ -441,11 +440,9 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev, goto err; } - tmp_skb = skb_share_check(skb, GFP_ATOMIC); - if (!tmp_skb) - goto err; - - skb = tmp_skb; + skb = skb_share_check(skb, GFP_ATOMIC); + if (!skb) + return -1; if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) { printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n");