On Sun, Jul 10, 2016 at 09:11:55PM +0300, Julian Anastasov wrote:
> Vegard Nossum is reporting for a crash in fib_dump_info
> when nh_dev = NULL and fib_nhs == 1:
>
> Pid: 50, comm: netlink.exe Not tainted 4.7.0-rc5+
> RIP: 0033:[<00000000602b3d18>]
> RSP: 0000000062623890 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 000000006261b800 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000024 RDI: 000000006245ba00
> RBP: 00000000626238f0 R08: 000000000000029c R09: 0000000000000000
> R10: 0000000062468038 R11: 000000006245ba00 R12: 000000006245ba00
> R13: 00000000625f96c0 R14: 00000000601e16f0 R15: 0000000000000000
> Kernel panic - not syncing: Kernel mode fault at addr 0x2e0, ip 0x602b3d18
> CPU: 0 PID: 50 Comm: netlink.exe Not tainted 4.7.0-rc5+ #581
> Stack:
> 626238f0 960226a02 00000400 000000fe
> 62623910 600afca7 62623970 62623a48
> 62468038 00000018 00000000 00000000
> Call Trace:
> [<602b3e93>] rtmsg_fib+0xd3/0x190
> [<602b6680>] fib_table_insert+0x260/0x500
> [<602b0e5d>] inet_rtm_newroute+0x4d/0x60
> [<60250def>] rtnetlink_rcv_msg+0x8f/0x270
> [<60267079>] netlink_rcv_skb+0xc9/0xe0
> [<60250d4b>] rtnetlink_rcv+0x3b/0x50
> [<60265400>] netlink_unicast+0x1a0/0x2c0
> [<60265e47>] netlink_sendmsg+0x3f7/0x470
> [<6021dc9a>] sock_sendmsg+0x3a/0x90
> [<6021e0d0>] ___sys_sendmsg+0x300/0x360
> [<6021fa64>] __sys_sendmsg+0x54/0xa0
> [<6021fac0>] SyS_sendmsg+0x10/0x20
> [<6001ea68>] handle_syscall+0x88/0x90
> [<600295fd>] userspace+0x3fd/0x500
> [<6001ac55>] fork_handler+0x85/0x90
>
> $ addr2line -e vmlinux -i 0x602b3d18
> include/linux/inetdevice.h:222
> net/ipv4/fib_semantics.c:1264
>
> Problem happens when RTNH_F_LINKDOWN is provided from user space
> when creating routes that do not use the flag, catched with
> netlink fuzzer.
>
> Currently, the kernel allows user space to set both flags
> to nh_flags and fib_flags but this is not intentional, the
> assumption was that they are not set. Fix this by rejecting
> both flags with EINVAL.
>
> Reported-by: Vegard Nossum <vegard.nos...@oracle.com>
> Fixes: 0eeb075fad73 ("net: ipv4 sysctl option to ignore routes when nexthop
> link is down")
> Signed-off-by: Julian Anastasov <j...@ssi.bg>
> Cc: Andy Gospodarek <go...@cumulusnetworks.com>
> Cc: Dinesh Dutt <dd...@cumulusnetworks.com>
> Cc: Scott Feldman <sfel...@gmail.com>
It looks like this catches the single and multipath next-hop cases, so
looks good to me.
Reviewed-by: Andy Gospodarek <go...@cumulusnetworks.com>
> ---
> net/ipv4/fib_semantics.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> Note: works for all kernels: net, net-next, 4.4.14, 4.5.7, 4.6.3
>
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index d09173b..539fa26 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -479,6 +479,9 @@ static int fib_get_nhs(struct fib_info *fi, struct
> rtnexthop *rtnh,
> if (!rtnh_ok(rtnh, remaining))
> return -EINVAL;
>
> + if (rtnh->rtnh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN))
> + return -EINVAL;
> +
> nexthop_nh->nh_flags =
> (cfg->fc_flags & ~0xFF) | rtnh->rtnh_flags;
> nexthop_nh->nh_oif = rtnh->rtnh_ifindex;
> @@ -1003,6 +1006,9 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
> if (fib_props[cfg->fc_type].scope > cfg->fc_scope)
> goto err_inval;
>
> + if (cfg->fc_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN))
> + goto err_inval;
> +
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> if (cfg->fc_mp) {
> nhs = fib_count_nexthops(cfg->fc_mp, cfg->fc_mp_len);
> --
> 1.9.3
>
