Commit 9fcf986c authored by Eric Dumazet's avatar Eric Dumazet Committed by Jakub Kicinski

ipv4: fix data races in fib_alias_hw_flags_set

fib_alias_hw_flags_set() can be used by concurrent threads,
and is only RCU protected.

We need to annotate accesses to following fields of struct fib_alias:

    offload, trap, offload_failed

Because of READ_ONCE()WRITE_ONCE() limitations, make these
field u8.

BUG: KCSAN: data-race in fib_alias_hw_flags_set / fib_alias_hw_flags_set

read to 0xffff888134224a6a of 1 bytes by task 2013 on cpu 1:
 fib_alias_hw_flags_set+0x28a/0x470 net/ipv4/fib_trie.c:1050
 nsim_fib4_rt_hw_flags_set drivers/net/netdevsim/fib.c:350 [inline]
 nsim_fib4_rt_add drivers/net/netdevsim/fib.c:367 [inline]
 nsim_fib4_rt_insert drivers/net/netdevsim/fib.c:429 [inline]
 nsim_fib4_event drivers/net/netdevsim/fib.c:461 [inline]
 nsim_fib_event drivers/net/netdevsim/fib.c:881 [inline]
 nsim_fib_event_work+0x1852/0x2cf0 drivers/net/netdevsim/fib.c:1477
 process_one_work+0x3f6/0x960 kernel/workqueue.c:2307
 process_scheduled_works kernel/workqueue.c:2370 [inline]
 worker_thread+0x7df/0xa70 kernel/workqueue.c:2456
 kthread+0x1bf/0x1e0 kernel/kthread.c:377
 ret_from_fork+0x1f/0x30

write to 0xffff888134224a6a of 1 bytes by task 4872 on cpu 0:
 fib_alias_hw_flags_set+0x2d5/0x470 net/ipv4/fib_trie.c:1054
 nsim_fib4_rt_hw_flags_set drivers/net/netdevsim/fib.c:350 [inline]
 nsim_fib4_rt_add drivers/net/netdevsim/fib.c:367 [inline]
 nsim_fib4_rt_insert drivers/net/netdevsim/fib.c:429 [inline]
 nsim_fib4_event drivers/net/netdevsim/fib.c:461 [inline]
 nsim_fib_event drivers/net/netdevsim/fib.c:881 [inline]
 nsim_fib_event_work+0x1852/0x2cf0 drivers/net/netdevsim/fib.c:1477
 process_one_work+0x3f6/0x960 kernel/workqueue.c:2307
 process_scheduled_works kernel/workqueue.c:2370 [inline]
 worker_thread+0x7df/0xa70 kernel/workqueue.c:2456
 kthread+0x1bf/0x1e0 kernel/kthread.c:377
 ret_from_fork+0x1f/0x30

value changed: 0x00 -> 0x02

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 4872 Comm: kworker/0:0 Not tainted 5.17.0-rc3-syzkaller-00188-g1d41d2e8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events nsim_fib_event_work

Fixes: 90b93f1b ("ipv4: Add "offload" and "trap" indications to routes")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Reviewed-by: default avatarIdo Schimmel <idosch@nvidia.com>
Link: https://lore.kernel.org/r/20220216173217.3792411-1-eric.dumazet@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 430065e2
...@@ -16,10 +16,9 @@ struct fib_alias { ...@@ -16,10 +16,9 @@ struct fib_alias {
u8 fa_slen; u8 fa_slen;
u32 tb_id; u32 tb_id;
s16 fa_default; s16 fa_default;
u8 offload:1, u8 offload;
trap:1, u8 trap;
offload_failed:1, u8 offload_failed;
unused:5;
struct rcu_head rcu; struct rcu_head rcu;
}; };
......
...@@ -525,9 +525,9 @@ void rtmsg_fib(int event, __be32 key, struct fib_alias *fa, ...@@ -525,9 +525,9 @@ void rtmsg_fib(int event, __be32 key, struct fib_alias *fa,
fri.dst_len = dst_len; fri.dst_len = dst_len;
fri.tos = fa->fa_tos; fri.tos = fa->fa_tos;
fri.type = fa->fa_type; fri.type = fa->fa_type;
fri.offload = fa->offload; fri.offload = READ_ONCE(fa->offload);
fri.trap = fa->trap; fri.trap = READ_ONCE(fa->trap);
fri.offload_failed = fa->offload_failed; fri.offload_failed = READ_ONCE(fa->offload_failed);
err = fib_dump_info(skb, info->portid, seq, event, &fri, nlm_flags); err = fib_dump_info(skb, info->portid, seq, event, &fri, nlm_flags);
if (err < 0) { if (err < 0) {
/* -EMSGSIZE implies BUG in fib_nlmsg_size() */ /* -EMSGSIZE implies BUG in fib_nlmsg_size() */
......
...@@ -1047,19 +1047,23 @@ void fib_alias_hw_flags_set(struct net *net, const struct fib_rt_info *fri) ...@@ -1047,19 +1047,23 @@ void fib_alias_hw_flags_set(struct net *net, const struct fib_rt_info *fri)
if (!fa_match) if (!fa_match)
goto out; goto out;
if (fa_match->offload == fri->offload && fa_match->trap == fri->trap && /* These are paired with the WRITE_ONCE() happening in this function.
fa_match->offload_failed == fri->offload_failed) * The reason is that we are only protected by RCU at this point.
*/
if (READ_ONCE(fa_match->offload) == fri->offload &&
READ_ONCE(fa_match->trap) == fri->trap &&
READ_ONCE(fa_match->offload_failed) == fri->offload_failed)
goto out; goto out;
fa_match->offload = fri->offload; WRITE_ONCE(fa_match->offload, fri->offload);
fa_match->trap = fri->trap; WRITE_ONCE(fa_match->trap, fri->trap);
/* 2 means send notifications only if offload_failed was changed. */ /* 2 means send notifications only if offload_failed was changed. */
if (net->ipv4.sysctl_fib_notify_on_flag_change == 2 && if (net->ipv4.sysctl_fib_notify_on_flag_change == 2 &&
fa_match->offload_failed == fri->offload_failed) READ_ONCE(fa_match->offload_failed) == fri->offload_failed)
goto out; goto out;
fa_match->offload_failed = fri->offload_failed; WRITE_ONCE(fa_match->offload_failed, fri->offload_failed);
if (!net->ipv4.sysctl_fib_notify_on_flag_change) if (!net->ipv4.sysctl_fib_notify_on_flag_change)
goto out; goto out;
...@@ -2297,9 +2301,9 @@ static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb, ...@@ -2297,9 +2301,9 @@ static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb,
fri.dst_len = KEYLENGTH - fa->fa_slen; fri.dst_len = KEYLENGTH - fa->fa_slen;
fri.tos = fa->fa_tos; fri.tos = fa->fa_tos;
fri.type = fa->fa_type; fri.type = fa->fa_type;
fri.offload = fa->offload; fri.offload = READ_ONCE(fa->offload);
fri.trap = fa->trap; fri.trap = READ_ONCE(fa->trap);
fri.offload_failed = fa->offload_failed; fri.offload_failed = READ_ONCE(fa->offload_failed);
err = fib_dump_info(skb, err = fib_dump_info(skb,
NETLINK_CB(cb->skb).portid, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, cb->nlh->nlmsg_seq,
......
...@@ -3395,8 +3395,8 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh, ...@@ -3395,8 +3395,8 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
fa->fa_tos == fri.tos && fa->fa_tos == fri.tos &&
fa->fa_info == res.fi && fa->fa_info == res.fi &&
fa->fa_type == fri.type) { fa->fa_type == fri.type) {
fri.offload = fa->offload; fri.offload = READ_ONCE(fa->offload);
fri.trap = fa->trap; fri.trap = READ_ONCE(fa->trap);
break; break;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment