diff mbox series

ip6mr: Fix skb_under_panic in ip6mr_cache_report()

Message ID	20230728100035.32092-1-yuehaibing@huawei.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Yue Haibing <yuehaibing@huawei.com> To: <davem@davemloft.net>, <dsahern@kernel.org>, <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>, <yoshfuji@linux-ipv6.org> CC: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Yue Haibing <yuehaibing@huawei.com> Subject: [PATCH] ip6mr: Fix skb_under_panic in ip6mr_cache_report() Date: Fri, 28 Jul 2023 18:00:35 +0800 Message-ID: <20230728100035.32092-1-yuehaibing@huawei.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk
Series	ip6mr: Fix skb_under_panic in ip6mr_cache_report() \| ip6mr: Fix skb_under_panic in ip6mr_cache_report()

Commit Message

Yue Haibing July 28, 2023, 10 a.m. UTC

  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 declar as this:
	void *skb_push(struct sk_buff *skb, unsigned int len);
		skb->data -= len;
		//0xffff888f5f86a84c - 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>
---
 net/ipv6/ip6mr.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Simon Horman July 28, 2023, 10:27 a.m. UTC | #1

On Fri, Jul 28, 2023 at 06:00:35PM +0800, Yue Haibing 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
> 
> 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 declar as this:

nit: declar as this -> declared as

> 	void *skb_push(struct sk_buff *skb, unsigned int len);
> 		skb->data -= len;
> 		//0xffff888f5f86a84c - 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>

...

Eric Dumazet July 28, 2023, 10:50 a.m. UTC | #2

On Fri, Jul 28, 2023 at 12:01 PM 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
>  ------------[ cut here ]------------
>

> 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 declar as this:
>         void *skb_push(struct sk_buff *skb, unsigned int len);
>                 skb->data -= len;
>                 //0xffff888f5f86a84c - 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>
> ---
>  net/ipv6/ip6mr.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index cc3d5ad17257..ee9c2ff8b0e4 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -1051,9 +1051,9 @@ static int ip6mr_cache_report(const struct mr_table *mrt, struct sk_buff *pkt,
>         int ret;
>
>  #ifdef CONFIG_IPV6_PIMSM_V2
> +       int nhoff = skb_network_offset(pkt);
>         if (assert == MRT6MSG_WHOLEPKT || assert == MRT6MSG_WRMIFWHOLE)
> -               skb = skb_realloc_headroom(pkt, -skb_network_offset(pkt)
> -                                               +sizeof(*msg));
> +               skb = skb_realloc_headroom(pkt, -nhoff + sizeof(*msg));
>         else
>  #endif
>                 skb = alloc_skb(sizeof(struct ipv6hdr) + sizeof(*msg), GFP_ATOMIC);
> @@ -1073,7 +1073,8 @@ 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->data += nhoff;
> +               skb->len  -= nhoff;

__skb_pull(skb, nhoff);

>
>                 skb_push(skb, sizeof(*msg));
>                 skb_reset_transport_header(skb);
> --
> 2.34.1
>

Yue Haibing July 28, 2023, noon UTC | #3

On 2023/7/28 18:50, Eric Dumazet wrote:
> On Fri, Jul 28, 2023 at 12:01 PM 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
>>  ------------[ cut here ]------------
>>
> 
>> 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 declar as this:
>>         void *skb_push(struct sk_buff *skb, unsigned int len);
>>                 skb->data -= len;
>>                 //0xffff888f5f86a84c - 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>
>> ---
>>  net/ipv6/ip6mr.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
>> index cc3d5ad17257..ee9c2ff8b0e4 100644
>> --- a/net/ipv6/ip6mr.c
>> +++ b/net/ipv6/ip6mr.c
>> @@ -1051,9 +1051,9 @@ static int ip6mr_cache_report(const struct mr_table *mrt, struct sk_buff *pkt,
>>         int ret;
>>
>>  #ifdef CONFIG_IPV6_PIMSM_V2
>> +       int nhoff = skb_network_offset(pkt);
>>         if (assert == MRT6MSG_WHOLEPKT || assert == MRT6MSG_WRMIFWHOLE)
>> -               skb = skb_realloc_headroom(pkt, -skb_network_offset(pkt)
>> -                                               +sizeof(*msg));
>> +               skb = skb_realloc_headroom(pkt, -nhoff + sizeof(*msg));
>>         else
>>  #endif
>>                 skb = alloc_skb(sizeof(struct ipv6hdr) + sizeof(*msg), GFP_ATOMIC);
>> @@ -1073,7 +1073,8 @@ 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->data += nhoff;
>> +               skb->len  -= nhoff;
> 
> __skb_pull(skb, nhoff);

Thanks， will do this in v2.
> 
>>
>>                 skb_push(skb, sizeof(*msg));
>>                 skb_reset_transport_header(skb);
>> --
>> 2.34.1
>>
> .
>

diff mbox series

Patch

diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index cc3d5ad17257..ee9c2ff8b0e4 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1051,9 +1051,9 @@  static int ip6mr_cache_report(const struct mr_table *mrt, struct sk_buff *pkt,
 	int ret;
 
 #ifdef CONFIG_IPV6_PIMSM_V2
+	int nhoff = skb_network_offset(pkt);
 	if (assert == MRT6MSG_WHOLEPKT || assert == MRT6MSG_WRMIFWHOLE)
-		skb = skb_realloc_headroom(pkt, -skb_network_offset(pkt)
-						+sizeof(*msg));
+		skb = skb_realloc_headroom(pkt, -nhoff + sizeof(*msg));
 	else
 #endif
 		skb = alloc_skb(sizeof(struct ipv6hdr) + sizeof(*msg), GFP_ATOMIC);
@@ -1073,7 +1073,8 @@  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->data += nhoff;
+		skb->len  -= nhoff;
 
 		skb_push(skb, sizeof(*msg));
 		skb_reset_transport_header(skb);