* [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
@ 2023-08-01 6:43 Yue Haibing
2023-08-01 7:51 ` Eric Dumazet
2023-08-02 9:40 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 8+ messages in thread
From: Yue Haibing @ 2023-08-01 6:43 UTC (permalink / raw)
To: davem, dsahern, edumazet, kuba, pabeni, yoshfuji
Cc: netdev, linux-kernel, simon.horman, Yue Haibing
skbuff: skb_under_panic: text:ffffffff88771f69 len:56 put:-4
head:ffff88805f86a800 data:ffff887f5f86a850 tail:0x88 end:0x2c0 dev:pim6reg
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:192!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 2 PID: 22968 Comm: kworker/2:11 Not tainted 6.5.0-rc3-00044-g0a8db05b571a #236
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:skb_panic+0x152/0x1d0
Call Trace:
<TASK>
skb_push+0xc4/0xe0
ip6mr_cache_report+0xd69/0x19b0
reg_vif_xmit+0x406/0x690
dev_hard_start_xmit+0x17e/0x6e0
__dev_queue_xmit+0x2d6a/0x3d20
vlan_dev_hard_start_xmit+0x3ab/0x5c0
dev_hard_start_xmit+0x17e/0x6e0
__dev_queue_xmit+0x2d6a/0x3d20
neigh_connected_output+0x3ed/0x570
ip6_finish_output2+0x5b5/0x1950
ip6_finish_output+0x693/0x11c0
ip6_output+0x24b/0x880
NF_HOOK.constprop.0+0xfd/0x530
ndisc_send_skb+0x9db/0x1400
ndisc_send_rs+0x12a/0x6c0
addrconf_dad_completed+0x3c9/0xea0
addrconf_dad_work+0x849/0x1420
process_one_work+0xa22/0x16e0
worker_thread+0x679/0x10c0
ret_from_fork+0x28/0x60
ret_from_fork_asm+0x11/0x20
When setup a vlan device on dev pim6reg, DAD ns packet may sent on reg_vif_xmit().
reg_vif_xmit()
ip6mr_cache_report()
skb_push(skb, -skb_network_offset(pkt));//skb_network_offset(pkt) is 4
And skb_push declared as:
void *skb_push(struct sk_buff *skb, unsigned int len);
skb->data -= len;
//0xffff88805f86a84c - 0xfffffffc = 0xffff887f5f86a850
skb->data is set to 0xffff887f5f86a850, which is invalid mem addr, lead to skb_push() fails.
Fixes: 14fb64e1f449 ("[IPV6] MROUTE: Support PIM-SM (SSM).")
Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
v3: drop unnecessary nhoff change
v2: Use __skb_pull() and fix commit log.
---
net/ipv6/ip6mr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index cc3d5ad17257..67a3b8f6e72b 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1073,7 +1073,7 @@ static int ip6mr_cache_report(const struct mr_table *mrt, struct sk_buff *pkt,
And all this only to mangle msg->im6_msgtype and
to set msg->im6_mbz to "mbz" :-)
*/
- skb_push(skb, -skb_network_offset(pkt));
+ __skb_pull(skb, skb_network_offset(pkt));
skb_push(skb, sizeof(*msg));
skb_reset_transport_header(skb);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-01 6:43 [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report() Yue Haibing
@ 2023-08-01 7:51 ` Eric Dumazet
2023-08-01 20:11 ` Jakub Kicinski
2023-08-02 9:40 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2023-08-01 7:51 UTC (permalink / raw)
To: Yue Haibing
Cc: davem, dsahern, kuba, pabeni, yoshfuji, netdev, linux-kernel,
simon.horman
On Tue, Aug 1, 2023 at 8:45 AM Yue Haibing <yuehaibing@huawei.com> wrote:
>
> skbuff: skb_under_panic: text:ffffffff88771f69 len:56 put:-4
> head:ffff88805f86a800 data:ffff887f5f86a850 tail:0x88 end:0x2c0 dev:pim6reg
>
> When setup a vlan device on dev pim6reg, DAD ns packet may sent on reg_vif_xmit().
> reg_vif_xmit()
> ip6mr_cache_report()
> skb_push(skb, -skb_network_offset(pkt));//skb_network_offset(pkt) is 4
> And skb_push declared as:
> void *skb_push(struct sk_buff *skb, unsigned int len);
> skb->data -= len;
> //0xffff88805f86a84c - 0xfffffffc = 0xffff887f5f86a850
> skb->data is set to 0xffff887f5f86a850, which is invalid mem addr, lead to skb_push() fails.
>
> Fixes: 14fb64e1f449 ("[IPV6] MROUTE: Support PIM-SM (SSM).")
> Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
> ---
> v3: drop unnecessary nhoff change
> v2: Use __skb_pull() and fix commit log.
> ---
> net/ipv6/ip6mr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index cc3d5ad17257..67a3b8f6e72b 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -1073,7 +1073,7 @@ static int ip6mr_cache_report(const struct mr_table *mrt, struct sk_buff *pkt,
> And all this only to mangle msg->im6_msgtype and
> to set msg->im6_mbz to "mbz" :-)
> */
> - skb_push(skb, -skb_network_offset(pkt));
> + __skb_pull(skb, skb_network_offset(pkt));
>
> skb_push(skb, sizeof(*msg));
> skb_reset_transport_header(skb);
Presumably this code has never been tested :/
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-01 7:51 ` Eric Dumazet
@ 2023-08-01 20:11 ` Jakub Kicinski
2023-08-02 0:52 ` David Ahern
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2023-08-01 20:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: Yue Haibing, davem, dsahern, pabeni, yoshfuji, netdev,
linux-kernel, simon.horman
On Tue, 1 Aug 2023 09:51:29 +0200 Eric Dumazet wrote:
> > - skb_push(skb, -skb_network_offset(pkt));
> > + __skb_pull(skb, skb_network_offset(pkt));
> >
> > skb_push(skb, sizeof(*msg));
> > skb_reset_transport_header(skb);
>
> Presumably this code has never been tested :/
Could have been tested on 32bit, I wonder if there is more such bugs :S
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-01 20:11 ` Jakub Kicinski
@ 2023-08-02 0:52 ` David Ahern
2023-08-02 1:28 ` YueHaibing
0 siblings, 1 reply; 8+ messages in thread
From: David Ahern @ 2023-08-02 0:52 UTC (permalink / raw)
To: Jakub Kicinski, Eric Dumazet
Cc: Yue Haibing, davem, pabeni, yoshfuji, netdev, linux-kernel, simon.horman
On 8/1/23 2:11 PM, Jakub Kicinski wrote:
> On Tue, 1 Aug 2023 09:51:29 +0200 Eric Dumazet wrote:
>>> - skb_push(skb, -skb_network_offset(pkt));
>>> + __skb_pull(skb, skb_network_offset(pkt));
>>>
>>> skb_push(skb, sizeof(*msg));
>>> skb_reset_transport_header(skb);
>>
>> Presumably this code has never been tested :/
>
> Could have been tested on 32bit, I wonder if there is more such bugs :S
that pattern shows up a few times:
net/ipv4/ah4.c: skb_push(skb, -skb_network_offset(skb));
net/ipv4/esp4.c: skb_push(skb, -skb_network_offset(skb));
net/ipv4/esp4_offload.c: skb_push(skb, -skb_network_offset(skb));
net/ipv4/esp4_offload.c: skb_push(skb, -skb_network_offset(skb));
net/ipv4/xfrm4_tunnel.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/ah6.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/esp6.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/esp6_offload.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/esp6_offload.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/ip6mr.c: skb_push(skb, -skb_network_offset(pkt));
net/ipv6/mip6.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/mip6.c: skb_push(skb, -skb_network_offset(skb));
net/ipv6/xfrm6_tunnel.c: skb_push(skb, -skb_network_offset(skb));
net/xfrm/xfrm_ipcomp.c: skb_push(skb, -skb_network_offset(skb));
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-02 0:52 ` David Ahern
@ 2023-08-02 1:28 ` YueHaibing
2023-08-02 2:10 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: YueHaibing @ 2023-08-02 1:28 UTC (permalink / raw)
To: David Ahern, Jakub Kicinski, Eric Dumazet
Cc: davem, pabeni, yoshfuji, netdev, linux-kernel, simon.horman
On 2023/8/2 8:52, David Ahern wrote:
> On 8/1/23 2:11 PM, Jakub Kicinski wrote:
>> On Tue, 1 Aug 2023 09:51:29 +0200 Eric Dumazet wrote:
>>>> - skb_push(skb, -skb_network_offset(pkt));
>>>> + __skb_pull(skb, skb_network_offset(pkt));
>>>>
>>>> skb_push(skb, sizeof(*msg));
>>>> skb_reset_transport_header(skb);
>>>
>>> Presumably this code has never been tested :/
>>
>> Could have been tested on 32bit, I wonder if there is more such bugs :S
>
> that pattern shows up a few times:
Ok, I will test and fix these if any.
>
> net/ipv4/ah4.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv4/esp4.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv4/esp4_offload.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv4/esp4_offload.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv4/xfrm4_tunnel.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/ah6.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/esp6.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/esp6_offload.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/esp6_offload.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/ip6mr.c: skb_push(skb, -skb_network_offset(pkt));
> net/ipv6/mip6.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/mip6.c: skb_push(skb, -skb_network_offset(skb));
> net/ipv6/xfrm6_tunnel.c: skb_push(skb, -skb_network_offset(skb));
> net/xfrm/xfrm_ipcomp.c: skb_push(skb, -skb_network_offset(skb));
> .
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-02 1:28 ` YueHaibing
@ 2023-08-02 2:10 ` Jakub Kicinski
2023-08-02 3:18 ` David Ahern
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2023-08-02 2:10 UTC (permalink / raw)
To: YueHaibing
Cc: David Ahern, Eric Dumazet, davem, pabeni, yoshfuji, netdev,
linux-kernel, simon.horman
On Wed, 2 Aug 2023 09:28:31 +0800 YueHaibing wrote:
> > that pattern shows up a few times:
>
> Ok, I will test and fix these if any.
Thanks, we may also want to add a
DEBUG_NET_WARN_ON_ONCE(len > INT_MAX)
in skb_push() but perhaps that's an overkill.
Most of those cases are probably legit (skb_.*_offset()
can well be negative if headers were already pulled).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-02 2:10 ` Jakub Kicinski
@ 2023-08-02 3:18 ` David Ahern
0 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2023-08-02 3:18 UTC (permalink / raw)
To: Jakub Kicinski, YueHaibing
Cc: Eric Dumazet, davem, pabeni, yoshfuji, netdev, linux-kernel,
simon.horman
On 8/1/23 8:10 PM, Jakub Kicinski wrote:
> On Wed, 2 Aug 2023 09:28:31 +0800 YueHaibing wrote:
>>> that pattern shows up a few times:
>>
>> Ok, I will test and fix these if any.
>
> Thanks, we may also want to add a
> DEBUG_NET_WARN_ON_ONCE(len > INT_MAX)
> in skb_push() but perhaps that's an overkill.
> Most of those cases are probably legit (skb_.*_offset()
> can well be negative if headers were already pulled).
Yea, a lot of those could be legit, so it would be best to have a test
case that shows it can be triggered and then any patch fixes it.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
2023-08-01 6:43 [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report() Yue Haibing
2023-08-01 7:51 ` Eric Dumazet
@ 2023-08-02 9:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-08-02 9:40 UTC (permalink / raw)
To: Yue Haibing
Cc: davem, dsahern, edumazet, kuba, pabeni, yoshfuji, netdev,
linux-kernel, simon.horman
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Tue, 1 Aug 2023 14:43:18 +0800 you wrote:
> skbuff: skb_under_panic: text:ffffffff88771f69 len:56 put:-4
> head:ffff88805f86a800 data:ffff887f5f86a850 tail:0x88 end:0x2c0 dev:pim6reg
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:192!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> CPU: 2 PID: 22968 Comm: kworker/2:11 Not tainted 6.5.0-rc3-00044-g0a8db05b571a #236
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> Workqueue: ipv6_addrconf addrconf_dad_work
> RIP: 0010:skb_panic+0x152/0x1d0
> Call Trace:
> <TASK>
> skb_push+0xc4/0xe0
> ip6mr_cache_report+0xd69/0x19b0
> reg_vif_xmit+0x406/0x690
> dev_hard_start_xmit+0x17e/0x6e0
> __dev_queue_xmit+0x2d6a/0x3d20
> vlan_dev_hard_start_xmit+0x3ab/0x5c0
> dev_hard_start_xmit+0x17e/0x6e0
> __dev_queue_xmit+0x2d6a/0x3d20
> neigh_connected_output+0x3ed/0x570
> ip6_finish_output2+0x5b5/0x1950
> ip6_finish_output+0x693/0x11c0
> ip6_output+0x24b/0x880
> NF_HOOK.constprop.0+0xfd/0x530
> ndisc_send_skb+0x9db/0x1400
> ndisc_send_rs+0x12a/0x6c0
> addrconf_dad_completed+0x3c9/0xea0
> addrconf_dad_work+0x849/0x1420
> process_one_work+0xa22/0x16e0
> worker_thread+0x679/0x10c0
> ret_from_fork+0x28/0x60
> ret_from_fork_asm+0x11/0x20
>
> [...]
Here is the summary with links:
- [v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report()
https://git.kernel.org/netdev/net/c/30e0191b16e8
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] 8+ messages in thread
end of thread, other threads:[~2023-08-02 9:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-01 6:43 [PATCH v3] ip6mr: Fix skb_under_panic in ip6mr_cache_report() Yue Haibing
2023-08-01 7:51 ` Eric Dumazet
2023-08-01 20:11 ` Jakub Kicinski
2023-08-02 0:52 ` David Ahern
2023-08-02 1:28 ` YueHaibing
2023-08-02 2:10 ` Jakub Kicinski
2023-08-02 3:18 ` David Ahern
2023-08-02 9:40 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.