general protection fault in fib_dump_info (2)

general protection fault in fib_dump_info (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    da2968ff Merge tag 'pci-v5.9-fixes-1' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=137316ca900000
kernel config:  https://syzkaller.appspot.com/x/.config?x=a0437fdd630bee11
dashboard link: https://syzkaller.appspot.com/bug?extid=a61aa19b0c14c8770bd9
compiler:       gcc (GCC) 10.1.0-syz 20200507
userspace arch: i386
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12707051900000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1150a046900000

The issue was bisected to:

commit 0b5e2e39739e861fa5fc84ab27a35dbe62a15330
Author: David Ahern <dsahern@gmail.com>
Date:   Tue May 26 18:56:16 2020 +0000

    nexthop: Expand nexthop_is_multipath in a few places

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=139cec66900000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=105cec66900000
console output: https://syzkaller.appspot.com/x/log.txt?x=179cec66900000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+a61aa19b0c14c8770bd9@syzkaller.appspotmail.com
Fixes: 0b5e2e39739e ("nexthop: Expand nexthop_is_multipath in a few places")

general protection fault, probably for non-canonical address 0xdffffc0000000010: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
CPU: 0 PID: 6830 Comm: syz-executor644 Not tainted 5.9.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:nexthop_is_blackhole include/net/nexthop.h:240 [inline]
RIP: 0010:fib_dump_info+0x893/0x1f00 net/ipv4/fib_semantics.c:1781
Code: 3c 02 00 0f 85 83 15 00 00 4d 8b 6d 10 e8 85 f1 ab fa 49 8d bd 80 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 62 15 00 00 4d 8b ad 80 00 00 00 e8 37 e2 2a 01
RSP: 0018:ffffc90001d37248 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff888093b02000 RCX: ffffffff86c84d53
RDX: 0000000000000010 RSI: ffffffff86c84d8b RDI: 0000000000000080
RBP: ffff88809f95a017 R08: 0000000000000001 R09: ffff88809f95a02b
R10: 0000000000000001 R11: 0000000000000000 R12: ffff88809f95a000
R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880a7164a40
FS:  0000000000000000(0000) GS:ffff8880ae600000(0063) knlGS:0000000009af2840
CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 0000000020000300 CR3: 0000000099698000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 rtmsg_fib+0x318/0xdf0 net/ipv4/fib_semantics.c:524
 fib_table_insert+0x1383/0x1af0 net/ipv4/fib_trie.c:1284
 inet_rtm_newroute+0x109/0x1e0 net/ipv4/fib_frontend.c:883
 rtnetlink_rcv_msg+0x44e/0xad0 net/core/rtnetlink.c:5563
 netlink_rcv_skb+0x15a/0x430 net/netlink/af_netlink.c:2470
 netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline]
 netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1330
 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1919
 sock_sendmsg_nosec net/socket.c:651 [inline]
 sock_sendmsg+0xcf/0x120 net/socket.c:671
 ____sys_sendmsg+0x6e8/0x810 net/socket.c:2353
 ___sys_sendmsg+0xf3/0x170 net/socket.c:2407
 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2440
 do_syscall_32_irqs_on arch/x86/entry/common.c:84 [inline]
 __do_fast_syscall_32+0x57/0x80 arch/x86/entry/common.c:126
 do_fast_syscall_32+0x2f/0x70 arch/x86/entry/common.c:149
 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c
RIP: 0023:0xf7f66549
Code: b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000ffba6bdc EFLAGS: 00000282 ORIG_RAX: 0000000000000172
RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 0000000020000580
RDX: 0000000000000000 RSI: 00000000080ea00c RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
---[ end trace ca6d5d9f281019b7 ]---
RIP: 0010:nexthop_is_blackhole include/net/nexthop.h:240 [inline]
RIP: 0010:fib_dump_info+0x893/0x1f00 net/ipv4/fib_semantics.c:1781
Code: 3c 02 00 0f 85 83 15 00 00 4d 8b 6d 10 e8 85 f1 ab fa 49 8d bd 80 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 62 15 00 00 4d 8b ad 80 00 00 00 e8 37 e2 2a 01
RSP: 0018:ffffc90001d37248 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff888093b02000 RCX: ffffffff86c84d53
RDX: 0000000000000010 RSI: ffffffff86c84d8b RDI: 0000000000000080
RBP: ffff88809f95a017 R08: 0000000000000001 R09: ffff88809f95a02b
R10: 0000000000000001 R11: 0000000000000000 R12: ffff88809f95a000
R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880a7164a40
FS:  0000000000000000(0000) GS:ffff8880ae600000(0063) knlGS:0000000009af2840
CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 00007f3f0e891000 CR3: 0000000099698000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

Re: general protection fault in fib_dump_info (2)

[Nikolay Aleksandrov]

On 8/21/20 6:27 PM, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    da2968ff Merge tag 'pci-v5.9-fixes-1' of git://git.kernel...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=137316ca900000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=a0437fdd630bee11
> dashboard link: https://syzkaller.appspot.com/bug?extid=a61aa19b0c14c8770bd9
> compiler:       gcc (GCC) 10.1.0-syz 20200507
> userspace arch: i386
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12707051900000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1150a046900000
> 
> The issue was bisected to:
> 
> commit 0b5e2e39739e861fa5fc84ab27a35dbe62a15330
> Author: David Ahern <dsahern@gmail.com>
> Date:   Tue May 26 18:56:16 2020 +0000
> 
>      nexthop: Expand nexthop_is_multipath in a few places
> 

This seems like a much older bug to me, the code allows to pass 0 groups and
thus we end up without any nh_grp_entry pointers. I reproduced it with a
modified iproute2 that sends an empty NHA_GROUP and then just uses the new
nexthop in any way (e.g. add a route with it). This is the same bug as the
earlier report for: "general protection fault in fib_check_nexthop"

I have a patch but I'll be able to send it tomorrow.

Cheers,
  Nik

Re: general protection fault in fib_dump_info (2)

[David Ahern]

On 8/21/20 10:00 AM, Nikolay Aleksandrov wrote:
> 
> This seems like a much older bug to me, the code allows to pass 0 groups
> and
> thus we end up without any nh_grp_entry pointers. I reproduced it with a
> modified iproute2 that sends an empty NHA_GROUP and then just uses the new
> nexthop in any way (e.g. add a route with it). This is the same bug as the
> earlier report for: "general protection fault in fib_check_nexthop"

hmmm.... empty NHA_GROUP should not be allowed.

> 
> I have a patch but I'll be able to send it tomorrow.
> 

thanks.

[PATCH net] net: nexthop: don't allow empty NHA_GROUP

[Nikolay Aleksandrov]

Currently the nexthop code will use an empty NHA_GROUP attribute, but it
requires at least 1 entry in order to function properly. Otherwise we
end up derefencing NULL pointers all over the place due to not having
any nh_grp_entry members allocated, nexthop code relies on having at least
the first member present. Empty NHA_GROUP doesn't make any sense so just
disallow it.
Also add a WARN_ON for any future users of nexthop_create_group().

CC: David Ahern <dsahern@gmail.com>
Reported-by: syzbot+a61aa19b0c14c8770bd9@syzkaller.appspotmail.com
Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
Tested on 5.3 and latest -net by adding a nexthop with an empty NHA_GROUP
(purposefully broken iproute2) and then adding a route which uses it.

 net/ipv4/nexthop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index cc8049b100b2..134e92382275 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -446,7 +446,7 @@ static int nh_check_attr_group(struct net *net, struct nlattr *tb[],
 	unsigned int i, j;
 	u8 nhg_fdb = 0;
 
-	if (len & (sizeof(struct nexthop_grp) - 1)) {
+	if (!len || len & (sizeof(struct nexthop_grp) - 1)) {
 		NL_SET_ERR_MSG(extack,
 			       "Invalid length for nexthop group attribute");
 		return -EINVAL;
@@ -1187,6 +1187,9 @@ static struct nexthop *nexthop_create_group(struct net *net,
 	struct nexthop *nh;
 	int i;
 
+	if (WARN_ON(!num_nh))
+		return ERR_PTR(-EINVAL);
+
 	nh = nexthop_alloc();
 	if (!nh)
 		return ERR_PTR(-ENOMEM);
-- 
2.26.2

[PATCH net v2] net: nexthop: don't allow empty NHA_GROUP

[Nikolay Aleksandrov]

Currently the nexthop code will use an empty NHA_GROUP attribute, but it
requires at least 1 entry in order to function properly. Otherwise we
end up derefencing null or random pointers all over the place due to not
having any nh_grp_entry members allocated, nexthop code relies on having at
least the first member present. Empty NHA_GROUP doesn't make any sense so
just disallow it.
Also add a WARN_ON for any future users of nexthop_create_group().

 BUG: kernel NULL pointer dereference, address: 0000000000000080
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0 
 Oops: 0000 [#1] SMP
 CPU: 0 PID: 558 Comm: ip Not tainted 5.9.0-rc1+ #93
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-2.fc32 04/01/2014
 RIP: 0010:fib_check_nexthop+0x4a/0xaa
 Code: 0f 84 83 00 00 00 48 c7 02 80 03 f7 81 c3 40 80 fe fe 75 12 b8 ea ff ff ff 48 85 d2 74 6b 48 c7 02 40 03 f7 81 c3 48 8b 40 10 <48> 8b 80 80 00 00 00 eb 36 80 78 1a 00 74 12 b8 ea ff ff ff 48 85
 RSP: 0018:ffff88807983ba00 EFLAGS: 00010213
 RAX: 0000000000000000 RBX: ffff88807983bc00 RCX: 0000000000000000
 RDX: ffff88807983bc00 RSI: 0000000000000000 RDI: ffff88807bdd0a80
 RBP: ffff88807983baf8 R08: 0000000000000dc0 R09: 000000000000040a
 R10: 0000000000000000 R11: ffff88807bdd0ae8 R12: 0000000000000000
 R13: 0000000000000000 R14: ffff88807bea3100 R15: 0000000000000001
 FS:  00007f10db393700(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000080 CR3: 000000007bd0f004 CR4: 00000000003706f0
 Call Trace:
  fib_create_info+0x64d/0xaf7
  fib_table_insert+0xf6/0x581
  ? __vma_adjust+0x3b6/0x4d4
  inet_rtm_newroute+0x56/0x70
  rtnetlink_rcv_msg+0x1e3/0x20d
  ? rtnl_calcit.isra.0+0xb8/0xb8
  netlink_rcv_skb+0x5b/0xac
  netlink_unicast+0xfa/0x17b
  netlink_sendmsg+0x334/0x353
  sock_sendmsg_nosec+0xf/0x3f
  ____sys_sendmsg+0x1a0/0x1fc
  ? copy_msghdr_from_user+0x4c/0x61
  ___sys_sendmsg+0x63/0x84
  ? handle_mm_fault+0xa39/0x11b5
  ? sockfd_lookup_light+0x72/0x9a
  __sys_sendmsg+0x50/0x6e
  do_syscall_64+0x54/0xbe
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
 RIP: 0033:0x7f10dacc0bb7
 Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb cd 66 0f 1f 44 00 00 8b 05 9a 4b 2b 00 85 c0 75 2e 48 63 ff 48 63 d2 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 b1 f2 2a 00 f7 d8 64 89 02 48
 RSP: 002b:00007ffcbe628bf8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
 RAX: ffffffffffffffda RBX: 00007ffcbe628f80 RCX: 00007f10dacc0bb7
 RDX: 0000000000000000 RSI: 00007ffcbe628c60 RDI: 0000000000000003
 RBP: 000000005f41099c R08: 0000000000000001 R09: 0000000000000008
 R10: 00000000000005e9 R11: 0000000000000246 R12: 0000000000000000
 R13: 0000000000000000 R14: 00007ffcbe628d70 R15: 0000563a86c6e440
 Modules linked in:
 CR2: 0000000000000080

CC: David Ahern <dsahern@gmail.com>
Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
Reported-by: syzbot+a61aa19b0c14c8770bd9@syzkaller.appspotmail.com
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
Tested on 5.3 and latest -net by adding a nexthop with an empty NHA_GROUP
(purposefully broken iproute2) and then adding a route which uses it.

v2: no changes, include stack trace in commit message

 net/ipv4/nexthop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index cc8049b100b2..134e92382275 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -446,7 +446,7 @@ static int nh_check_attr_group(struct net *net, struct nlattr *tb[],
 	unsigned int i, j;
 	u8 nhg_fdb = 0;
 
-	if (len & (sizeof(struct nexthop_grp) - 1)) {
+	if (!len || len & (sizeof(struct nexthop_grp) - 1)) {
 		NL_SET_ERR_MSG(extack,
 			       "Invalid length for nexthop group attribute");
 		return -EINVAL;
@@ -1187,6 +1187,9 @@ static struct nexthop *nexthop_create_group(struct net *net,
 	struct nexthop *nh;
 	int i;
 
+	if (WARN_ON(!num_nh))
+		return ERR_PTR(-EINVAL);
+
 	nh = nexthop_alloc();
 	if (!nh)
 		return ERR_PTR(-ENOMEM);
-- 
2.26.2

Re: [PATCH net v2] net: nexthop: don't allow empty NHA_GROUP

[David Ahern]

On 8/22/20 6:06 AM, Nikolay Aleksandrov wrote:
> Currently the nexthop code will use an empty NHA_GROUP attribute, but it
> requires at least 1 entry in order to function properly. Otherwise we
> end up derefencing null or random pointers all over the place due to not
> having any nh_grp_entry members allocated, nexthop code relies on having at
> least the first member present. Empty NHA_GROUP doesn't make any sense so
> just disallow it.
> Also add a WARN_ON for any future users of nexthop_create_group().
> 

...

> 
> CC: David Ahern <dsahern@gmail.com>
> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
> Reported-by: syzbot+a61aa19b0c14c8770bd9@syzkaller.appspotmail.com
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> Tested on 5.3 and latest -net by adding a nexthop with an empty NHA_GROUP
> (purposefully broken iproute2) and then adding a route which uses it.
> 
> v2: no changes, include stack trace in commit message
> 
>  net/ipv4/nexthop.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

Thanks, Nik

Re: [PATCH net v2] net: nexthop: don't allow empty NHA_GROUP

[David Miller]

From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Sat, 22 Aug 2020 15:06:36 +0300

> Currently the nexthop code will use an empty NHA_GROUP attribute, but it
> requires at least 1 entry in order to function properly. Otherwise we
> end up derefencing null or random pointers all over the place due to not
> having any nh_grp_entry members allocated, nexthop code relies on having at
> least the first member present. Empty NHA_GROUP doesn't make any sense so
> just disallow it.
> Also add a WARN_ON for any future users of nexthop_create_group().
 ...
> CC: David Ahern <dsahern@gmail.com>
> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
> Reported-by: syzbot+a61aa19b0c14c8770bd9@syzkaller.appspotmail.com
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

Applied and queued up for -stable, thanks.