| Message ID | 20220406035556.730-1-xiam0nd.tong@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b423e54ba965b4469b48e46fd16941f1e1701697 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso | expand |
4/6/22 06:55, Xiaomeng Tong пишет: > All remaining skbs should be released when myri10ge_xmit fails to > transmit a packet. Fix it within another skb_list_walk_safe. > > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> > --- > > changes since v2: > - free all remaining skbs. (Xiaomeng Tong) > > changes since v1: > - remove the unneeded assignmnets. (Xiaomeng Tong) > > v2:https://lore.kernel.org/lkml/20220405000553.21856-1-xiam0nd.tong@gmail.com/ > v1:https://lore.kernel.org/lkml/20220319052350.26535-1-xiam0nd.tong@gmail.com/ > > --- > drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > index 50ac3ee2577a..21d2645885ce 100644 > --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > @@ -2903,11 +2903,9 @@ static netdev_tx_t myri10ge_sw_tso(struct sk_buff *skb, > status = myri10ge_xmit(curr, dev); > if (status != 0) { a transmit function returns NETDEV_TX_OK on success > dev_kfree_skb_any(curr); > - if (segs != NULL) { > - curr = segs; > - segs = next; > + skb_list_walk_safe(next, curr, next) { > curr->next = NULL; > - dev_kfree_skb_any(segs); > + dev_kfree_skb_any(curr); > } > goto drop; > }
4/6/22 06:55, Xiaomeng Tong пишет: > All remaining skbs should be released when myri10ge_xmit fails to > transmit a packet. Fix it within another skb_list_walk_safe. > > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> > --- > > changes since v2: > - free all remaining skbs. (Xiaomeng Tong) > > changes since v1: > - remove the unneeded assignmnets. (Xiaomeng Tong) > > v2:https://lore.kernel.org/lkml/20220405000553.21856-1-xiam0nd.tong@gmail.com/ > v1:https://lore.kernel.org/lkml/20220319052350.26535-1-xiam0nd.tong@gmail.com/ > > --- > drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > index 50ac3ee2577a..21d2645885ce 100644 > --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > @@ -2903,11 +2903,9 @@ static netdev_tx_t myri10ge_sw_tso(struct sk_buff *skb, > status = myri10ge_xmit(curr, dev); > if (status != 0) { > dev_kfree_skb_any(curr); > - if (segs != NULL) { > - curr = segs; > - segs = next; > + skb_list_walk_safe(next, curr, next) { > curr->next = NULL; > - dev_kfree_skb_any(segs); > + dev_kfree_skb_any(curr); why can't we just do the following? skb_list_walk_safe(segs, skb, nskb) { status = myri10ge_xmit(curr, dev); if (err) break; } /* Free all of the segments. */ skb_list_walk_safe(segs, skb, nskb) { if (err) kfree_skb(skb); else consume_skb(skb); } return err; > } > goto drop; > }
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Wed, 6 Apr 2022 11:55:56 +0800 you wrote: > All remaining skbs should be released when myri10ge_xmit fails to > transmit a packet. Fix it within another skb_list_walk_safe. > > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> > --- > > changes since v2: > - free all remaining skbs. (Xiaomeng Tong) > > [...] Here is the summary with links: - [v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso https://git.kernel.org/netdev/net/c/b423e54ba965 You are awesome, thank you!
On Wed, 6 Apr 2022 11:55:56 +0800 Xiaomeng Tong wrote: > All remaining skbs should be released when myri10ge_xmit fails to > transmit a packet. Fix it within another skb_list_walk_safe. I think it was also a UAF. > diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > index 50ac3ee2577a..21d2645885ce 100644 > --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c > @@ -2903,11 +2903,9 @@ static netdev_tx_t myri10ge_sw_tso(struct sk_buff *skb, > status = myri10ge_xmit(curr, dev); > if (status != 0) { > dev_kfree_skb_any(curr); > - if (segs != NULL) { > - curr = segs; > - segs = next; > + skb_list_walk_safe(next, curr, next) { > curr->next = NULL; > - dev_kfree_skb_any(segs); > + dev_kfree_skb_any(curr); > } > goto drop; > } Much better, thanks. kfree_skb_list() exists but the patch was already applied, so whatever.
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c index 50ac3ee2577a..21d2645885ce 100644 --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c @@ -2903,11 +2903,9 @@ static netdev_tx_t myri10ge_sw_tso(struct sk_buff *skb, status = myri10ge_xmit(curr, dev); if (status != 0) { dev_kfree_skb_any(curr); - if (segs != NULL) { - curr = segs; - segs = next; + skb_list_walk_safe(next, curr, next) { curr->next = NULL; - dev_kfree_skb_any(segs); + dev_kfree_skb_any(curr); } goto drop; }
All remaining skbs should be released when myri10ge_xmit fails to transmit a packet. Fix it within another skb_list_walk_safe. Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> --- changes since v2: - free all remaining skbs. (Xiaomeng Tong) changes since v1: - remove the unneeded assignmnets. (Xiaomeng Tong) v2:https://lore.kernel.org/lkml/20220405000553.21856-1-xiam0nd.tong@gmail.com/ v1:https://lore.kernel.org/lkml/20220319052350.26535-1-xiam0nd.tong@gmail.com/ --- drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)