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 <[email protected]>
---
changes since v2:
- free all remaining skbs. (Xiaomeng Tong)
changes since v1:
- remove the unneeded assignmnets. (Xiaomeng Tong)
v2:https://lore.kernel.org/lkml/[email protected]/
v1:https://lore.kernel.org/lkml/[email protected]/
---
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);
}
goto drop;
}
--
2.17.1
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 <[email protected]>
> ---
>
> changes since v2:
> - free all remaining skbs. (Xiaomeng Tong)
>
> changes since v1:
> - remove the unneeded assignmnets. (Xiaomeng Tong)
>
> v2:https://lore.kernel.org/lkml/[email protected]/
> v1:https://lore.kernel.org/lkml/[email protected]/
>
> ---
> 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;
> }
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 <[email protected]>
> ---
>
> changes since v2:
> - free all remaining skbs. (Xiaomeng Tong)
>
> changes since v1:
> - remove the unneeded assignmnets. (Xiaomeng Tong)
>
> v2:https://lore.kernel.org/lkml/[email protected]/
> v1:https://lore.kernel.org/lkml/[email protected]/
>
> ---
> 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;
> }
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:
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 <[email protected]>
> ---
>
> 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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
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.