Skip to content

Commit 12a7894

Browse files
iveceraivecera
committed
nexthop: Fix out-of-bounds access during attribute validation
JIRA: https://issues.redhat.com/browse/RHEL-59118 commit d8a2107 Author: Ido Schimmel <idosch@nvidia.com> Date: Mon Mar 11 18:23:06 2024 +0200 nexthop: Fix out-of-bounds access during attribute validation Passing a maximum attribute type to nlmsg_parse() that is larger than the size of the passed policy will result in an out-of-bounds access [1] when the attribute type is used as an index into the policy array. Fix by setting the maximum attribute type according to the policy size, as is already done for RTM_NEWNEXTHOP messages. Add a test case that triggers the bug. No regressions in fib nexthops tests: # ./fib_nexthops.sh [...] Tests passed: 236 Tests failed: 0 [1] BUG: KASAN: global-out-of-bounds in __nla_validate_parse+0x1e53/0x2940 Read of size 1 at addr ffffffff99ab4d20 by task ip/610 CPU: 3 PID: 610 Comm: ip Not tainted 6.8.0-rc7-custom-gd435d6e3e161 #9 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc38 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x8f/0xe0 print_report+0xcf/0x670 kasan_report+0xd8/0x110 __nla_validate_parse+0x1e53/0x2940 __nla_parse+0x40/0x50 rtm_del_nexthop+0x1bd/0x400 rtnetlink_rcv_msg+0x3cc/0xf20 netlink_rcv_skb+0x170/0x440 netlink_unicast+0x540/0x820 netlink_sendmsg+0x8d3/0xdb0 ____sys_sendmsg+0x31f/0xa60 ___sys_sendmsg+0x13a/0x1e0 __sys_sendmsg+0x11c/0x1f0 do_syscall_64+0xc5/0x1d0 entry_SYSCALL_64_after_hwframe+0x63/0x6b [...] The buggy address belongs to the variable: rtm_nh_policy_del+0x20/0x40 Fixes: 2118f93 ("net: nexthop: Adjust netlink policy parsing for a new attribute") Reported-by: Eric Dumazet <edumazet@google.com> Closes: https://lore.kernel.org/netdev/CANn89i+UNcG0PJMW5X7gOMunF38ryMh=L1aeZUKH3kL4UdUqag@mail.gmail.com/ Reported-by: syzbot+65bb09a7208ce3d4a633@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/00000000000088981b06133bc07b@google.com/ Signed-off-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: David Ahern <dsahern@kernel.org> Link: https://lore.kernel.org/r/20240311162307.545385-4-idosch@nvidia.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
1 parent 610f585 commit 12a7894

File tree

2 files changed

+23
-12
lines changed

2 files changed

+23
-12
lines changed

‎net/ipv4/nexthop.c‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3242,8 +3242,8 @@ static int nh_valid_get_del_req(const struct nlmsghdr *nlh,
32423242
static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
32433243
struct netlink_ext_ack *extack)
32443244
{
3245+
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_del)];
32453246
struct net *net = sock_net(skb->sk);
3246-
struct nlattr *tb[NHA_MAX + 1];
32473247
struct nl_info nlinfo = {
32483248
.nlh = nlh,
32493249
.nl_net = net,
@@ -3253,8 +3253,9 @@ static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
32533253
int err;
32543254
u32 id;
32553255

3256-
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
3257-
rtm_nh_policy_del, extack);
3256+
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
3257+
ARRAY_SIZE(rtm_nh_policy_del) - 1, rtm_nh_policy_del,
3258+
extack);
32583259
if (err < 0)
32593260
return err;
32603261

@@ -3275,16 +3276,17 @@ static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
32753276
static int rtm_get_nexthop(struct sk_buff *in_skb, struct nlmsghdr *nlh,
32763277
struct netlink_ext_ack *extack)
32773278
{
3279+
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_get)];
32783280
struct net *net = sock_net(in_skb->sk);
3279-
struct nlattr *tb[NHA_MAX + 1];
32803281
struct sk_buff *skb = NULL;
32813282
struct nexthop *nh;
32823283
u32 op_flags;
32833284
int err;
32843285
u32 id;
32853286

3286-
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
3287-
rtm_nh_policy_get, extack);
3287+
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
3288+
ARRAY_SIZE(rtm_nh_policy_get) - 1, rtm_nh_policy_get,
3289+
extack);
32883290
if (err < 0)
32893291
return err;
32903292

@@ -3403,10 +3405,11 @@ static int nh_valid_dump_req(const struct nlmsghdr *nlh,
34033405
struct nh_dump_filter *filter,
34043406
struct netlink_callback *cb)
34053407
{
3406-
struct nlattr *tb[NHA_MAX + 1];
3408+
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_dump)];
34073409
int err;
34083410

3409-
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
3411+
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
3412+
ARRAY_SIZE(rtm_nh_policy_dump) - 1,
34103413
rtm_nh_policy_dump, cb->extack);
34113414
if (err < 0)
34123415
return err;
@@ -3551,10 +3554,11 @@ static int nh_valid_dump_bucket_req(const struct nlmsghdr *nlh,
35513554
struct netlink_callback *cb)
35523555
{
35533556
struct nlattr *res_tb[ARRAY_SIZE(rtm_nh_res_bucket_policy_dump)];
3554-
struct nlattr *tb[NHA_MAX + 1];
3557+
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_dump_bucket)];
35553558
int err;
35563559

3557-
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
3560+
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
3561+
ARRAY_SIZE(rtm_nh_policy_dump_bucket) - 1,
35583562
rtm_nh_policy_dump_bucket, NULL);
35593563
if (err < 0)
35603564
return err;
@@ -3729,10 +3733,11 @@ static int nh_valid_get_bucket_req(const struct nlmsghdr *nlh,
37293733
u32 *id, u16 *bucket_index,
37303734
struct netlink_ext_ack *extack)
37313735
{
3732-
struct nlattr *tb[NHA_MAX + 1];
3736+
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_get_bucket)];
37333737
int err;
37343738

3735-
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
3739+
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
3740+
ARRAY_SIZE(rtm_nh_policy_get_bucket) - 1,
37363741
rtm_nh_policy_get_bucket, extack);
37373742
if (err < 0)
37383743
return err;

‎tools/testing/selftests/net/fib_nexthops.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,6 +2069,12 @@ basic()
20692069
run_cmd "$IP nexthop get id 1"
20702070
log_test $? 2 "Nexthop get on non-existent id"
20712071

2072+
run_cmd "$IP nexthop del id 1"
2073+
log_test $? 2 "Nexthop del with non-existent id"
2074+
2075+
run_cmd "$IP nexthop del id 1 group 1/2/3/4/5/6/7/8"
2076+
log_test $? 2 "Nexthop del with non-existent id and extra attributes"
2077+
20722078
# attempt to create nh without a device or gw - fails
20732079
run_cmd "$IP nexthop add id 1"
20742080
log_test $? 2 "Nexthop with no device or gateway"

0 commit comments

Comments
 (0)