* Re: [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso
2022-04-06 3:55 [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso Xiaomeng Tong
@ 2022-04-06 11:36 ` Denis Kirjanov
2022-04-06 12:33 ` Denis Kirjanov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Denis Kirjanov @ 2022-04-06 11:36 UTC (permalink / raw)
To: Xiaomeng Tong, christopher.lee, davem, kuba, pabeni; +Cc: netdev, linux-kernel
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;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso
2022-04-06 3:55 [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso Xiaomeng Tong
2022-04-06 11:36 ` Denis Kirjanov
@ 2022-04-06 12:33 ` Denis Kirjanov
2022-04-06 14:30 ` patchwork-bot+netdevbpf
2022-04-06 17:50 ` Jakub Kicinski
3 siblings, 0 replies; 5+ messages in thread
From: Denis Kirjanov @ 2022-04-06 12:33 UTC (permalink / raw)
To: Xiaomeng Tong, christopher.lee, davem, kuba, pabeni; +Cc: netdev, linux-kernel
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;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso
2022-04-06 3:55 [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso Xiaomeng Tong
2022-04-06 11:36 ` Denis Kirjanov
2022-04-06 12:33 ` Denis Kirjanov
@ 2022-04-06 14:30 ` patchwork-bot+netdevbpf
2022-04-06 17:50 ` Jakub Kicinski
3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-04-06 14:30 UTC (permalink / raw)
To: Xiaomeng Tong; +Cc: christopher.lee, davem, kuba, pabeni, netdev, linux-kernel
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso
2022-04-06 3:55 [PATCH v3] myri10ge: fix an incorrect free for skb in myri10ge_sw_tso Xiaomeng Tong
` (2 preceding siblings ...)
2022-04-06 14:30 ` patchwork-bot+netdevbpf
@ 2022-04-06 17:50 ` Jakub Kicinski
3 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2022-04-06 17:50 UTC (permalink / raw)
To: Xiaomeng Tong; +Cc: christopher.lee, davem, pabeni, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 5+ messages in thread