netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] ipmr: Fix skb headroom in ipmr_get_route().
@ 2019-11-15 17:29 Guillaume Nault
  2019-11-16 21:09 ` David Miller
  0 siblings, 1 reply; 2+ messages in thread
From: Guillaume Nault @ 2019-11-15 17:29 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: netdev, Roopa Prabhu, Paolo Abeni

In route.c, inet_rtm_getroute_build_skb() creates an skb with no
headroom. This skb is then used by inet_rtm_getroute() which may pass
it to rt_fill_info() and, from there, to ipmr_get_route(). The later
might try to reuse this skb by cloning it and prepending an IPv4
header. But since the original skb has no headroom, skb_push() triggers
skb_under_panic():

skbuff: skb_under_panic: text:00000000ca46ad8a len:80 put:20 head:00000000cd28494e data:000000009366fd6b tail:0x3c end:0xec0 dev:veth0
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:108!
invalid opcode: 0000 [#1] SMP KASAN PTI
CPU: 6 PID: 587 Comm: ip Not tainted 5.4.0-rc6+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-2.fc30 04/01/2014
RIP: 0010:skb_panic+0xbf/0xd0
Code: 41 a2 ff 8b 4b 70 4c 8b 4d d0 48 c7 c7 20 76 f5 8b 44 8b 45 bc 48 8b 55 c0 48 8b 75 c8 41 54 41 57 41 56 41 55 e8 75 dc 7a ff <0f> 0b 0f 1f 44 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00
RSP: 0018:ffff888059ddf0b0 EFLAGS: 00010286
RAX: 0000000000000086 RBX: ffff888060a315c0 RCX: ffffffff8abe4822
RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff88806c9a79cc
RBP: ffff888059ddf118 R08: ffffed100d9361b1 R09: ffffed100d9361b0
R10: ffff88805c68aee3 R11: ffffed100d9361b1 R12: ffff88805d218000
R13: ffff88805c689fec R14: 000000000000003c R15: 0000000000000ec0
FS:  00007f6af184b700(0000) GS:ffff88806c980000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffc8204a000 CR3: 0000000057b40006 CR4: 0000000000360ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 skb_push+0x7e/0x80
 ipmr_get_route+0x459/0x6fa
 rt_fill_info+0x692/0x9f0
 inet_rtm_getroute+0xd26/0xf20
 rtnetlink_rcv_msg+0x45d/0x630
 netlink_rcv_skb+0x1a5/0x220
 rtnetlink_rcv+0x15/0x20
 netlink_unicast+0x305/0x3a0
 netlink_sendmsg+0x575/0x730
 sock_sendmsg+0xb5/0xc0
 ___sys_sendmsg+0x497/0x4f0
 __sys_sendmsg+0xcb/0x150
 __x64_sys_sendmsg+0x48/0x50
 do_syscall_64+0xd2/0xac0
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Actually the original skb used to have enough headroom, but the
reserve_skb() call was lost with the introduction of
inet_rtm_getroute_build_skb() by commit 404eb77ea766 ("ipv4: support
sport, dport and ip_proto in RTM_GETROUTE").

We could reserve some headroom again in inet_rtm_getroute_build_skb(),
but this function shouldn't be responsible for handling the special
case of ipmr_get_route(). Let's handle that directly in
ipmr_get_route() by calling skb_realloc_headroom() instead of
skb_clone().

Fixes: 404eb77ea766 ("ipv4: support sport, dport and ip_proto in RTM_GETROUTE")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/ipv4/ipmr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 716d5472c022..58007439cffd 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2289,7 +2289,8 @@ int ipmr_get_route(struct net *net, struct sk_buff *skb,
 			rcu_read_unlock();
 			return -ENODEV;
 		}
-		skb2 = skb_clone(skb, GFP_ATOMIC);
+
+		skb2 = skb_realloc_headroom(skb, sizeof(struct iphdr));
 		if (!skb2) {
 			read_unlock(&mrt_lock);
 			rcu_read_unlock();
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] ipmr: Fix skb headroom in ipmr_get_route().
  2019-11-15 17:29 [PATCH net] ipmr: Fix skb headroom in ipmr_get_route() Guillaume Nault
@ 2019-11-16 21:09 ` David Miller
  0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2019-11-16 21:09 UTC (permalink / raw)
  To: gnault; +Cc: jakub.kicinski, netdev, roopa, pabeni

From: Guillaume Nault <gnault@redhat.com>
Date: Fri, 15 Nov 2019 18:29:52 +0100

> In route.c, inet_rtm_getroute_build_skb() creates an skb with no
> headroom. This skb is then used by inet_rtm_getroute() which may pass
> it to rt_fill_info() and, from there, to ipmr_get_route(). The later
> might try to reuse this skb by cloning it and prepending an IPv4
> header. But since the original skb has no headroom, skb_push() triggers
> skb_under_panic():
 ...
> Actually the original skb used to have enough headroom, but the
> reserve_skb() call was lost with the introduction of
> inet_rtm_getroute_build_skb() by commit 404eb77ea766 ("ipv4: support
> sport, dport and ip_proto in RTM_GETROUTE").
> 
> We could reserve some headroom again in inet_rtm_getroute_build_skb(),
> but this function shouldn't be responsible for handling the special
> case of ipmr_get_route(). Let's handle that directly in
> ipmr_get_route() by calling skb_realloc_headroom() instead of
> skb_clone().
> 
> Fixes: 404eb77ea766 ("ipv4: support sport, dport and ip_proto in RTM_GETROUTE")
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-11-16 21:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15 17:29 [PATCH net] ipmr: Fix skb headroom in ipmr_get_route() Guillaume Nault
2019-11-16 21:09 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).