mirror of
https://mirrors.bfsu.edu.cn/git/linux.git
synced 2024-12-18 08:35:08 +08:00
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: 2118f9390d
("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>
This commit is contained in:
parent
262a68aa46
commit
d8a21070b6
@ -3244,8 +3244,8 @@ static int nh_valid_get_del_req(const struct nlmsghdr *nlh,
|
||||
static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
|
||||
struct netlink_ext_ack *extack)
|
||||
{
|
||||
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_del)];
|
||||
struct net *net = sock_net(skb->sk);
|
||||
struct nlattr *tb[NHA_MAX + 1];
|
||||
struct nl_info nlinfo = {
|
||||
.nlh = nlh,
|
||||
.nl_net = net,
|
||||
@ -3255,8 +3255,9 @@ static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
|
||||
int err;
|
||||
u32 id;
|
||||
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
|
||||
rtm_nh_policy_del, extack);
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
|
||||
ARRAY_SIZE(rtm_nh_policy_del) - 1, rtm_nh_policy_del,
|
||||
extack);
|
||||
if (err < 0)
|
||||
return err;
|
||||
|
||||
@ -3277,16 +3278,17 @@ static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
|
||||
static int rtm_get_nexthop(struct sk_buff *in_skb, struct nlmsghdr *nlh,
|
||||
struct netlink_ext_ack *extack)
|
||||
{
|
||||
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_get)];
|
||||
struct net *net = sock_net(in_skb->sk);
|
||||
struct nlattr *tb[NHA_MAX + 1];
|
||||
struct sk_buff *skb = NULL;
|
||||
struct nexthop *nh;
|
||||
u32 op_flags;
|
||||
int err;
|
||||
u32 id;
|
||||
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
|
||||
rtm_nh_policy_get, extack);
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
|
||||
ARRAY_SIZE(rtm_nh_policy_get) - 1, rtm_nh_policy_get,
|
||||
extack);
|
||||
if (err < 0)
|
||||
return err;
|
||||
|
||||
@ -3405,10 +3407,11 @@ static int nh_valid_dump_req(const struct nlmsghdr *nlh,
|
||||
struct nh_dump_filter *filter,
|
||||
struct netlink_callback *cb)
|
||||
{
|
||||
struct nlattr *tb[NHA_MAX + 1];
|
||||
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_dump)];
|
||||
int err;
|
||||
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
|
||||
ARRAY_SIZE(rtm_nh_policy_dump) - 1,
|
||||
rtm_nh_policy_dump, cb->extack);
|
||||
if (err < 0)
|
||||
return err;
|
||||
@ -3548,10 +3551,11 @@ static int nh_valid_dump_bucket_req(const struct nlmsghdr *nlh,
|
||||
struct netlink_callback *cb)
|
||||
{
|
||||
struct nlattr *res_tb[ARRAY_SIZE(rtm_nh_res_bucket_policy_dump)];
|
||||
struct nlattr *tb[NHA_MAX + 1];
|
||||
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_dump_bucket)];
|
||||
int err;
|
||||
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
|
||||
ARRAY_SIZE(rtm_nh_policy_dump_bucket) - 1,
|
||||
rtm_nh_policy_dump_bucket, NULL);
|
||||
if (err < 0)
|
||||
return err;
|
||||
@ -3716,10 +3720,11 @@ static int nh_valid_get_bucket_req(const struct nlmsghdr *nlh,
|
||||
u32 *id, u16 *bucket_index,
|
||||
struct netlink_ext_ack *extack)
|
||||
{
|
||||
struct nlattr *tb[NHA_MAX + 1];
|
||||
struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_get_bucket)];
|
||||
int err;
|
||||
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb, NHA_MAX,
|
||||
err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
|
||||
ARRAY_SIZE(rtm_nh_policy_get_bucket) - 1,
|
||||
rtm_nh_policy_get_bucket, extack);
|
||||
if (err < 0)
|
||||
return err;
|
||||
|
@ -2066,6 +2066,12 @@ basic()
|
||||
run_cmd "$IP nexthop get id 1"
|
||||
log_test $? 2 "Nexthop get on non-existent id"
|
||||
|
||||
run_cmd "$IP nexthop del id 1"
|
||||
log_test $? 2 "Nexthop del with non-existent id"
|
||||
|
||||
run_cmd "$IP nexthop del id 1 group 1/2/3/4/5/6/7/8"
|
||||
log_test $? 2 "Nexthop del with non-existent id and extra attributes"
|
||||
|
||||
# attempt to create nh without a device or gw - fails
|
||||
run_cmd "$IP nexthop add id 1"
|
||||
log_test $? 2 "Nexthop with no device or gateway"
|
||||
|
Loading…
Reference in New Issue
Block a user