diff options
author | Ying Xue <ying.xue@windriver.com> | 2018-02-14 00:38:04 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2018-02-14 14:46:33 -0500 |
commit | ed4ffdfec26dfe1bb02435afd1e01f61426f7212 (patch) | |
tree | 9c3fbf6aec977663deaf766bbaaf5f5a8a41a15c | |
parent | 5631f65decf390ae480d157838c0c393a991328e (diff) |
tipc: Fix missing RTNL lock protection during setting link properties
Currently when user changes link properties, TIPC first checks if
user's command message contains media name or bearer name through
tipc_media_find() or tipc_bearer_find() which is protected by RTNL
lock. But when tipc_nl_compat_link_set() conducts the checking with
the two functions, it doesn't hold RTNL lock at all, as a result,
the following complaints were reported:
audit: type=1400 audit(1514679888.244:9): avc: denied { write } for
pid=3194 comm="syzkaller021477" path="socket:[11143]" dev="sockfs"
ino=11143 scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
tcontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
tclass=netlink_generic_socket permissive=1
Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
=============================
WARNING: suspicious RCU usage
4.15.0-rc5+ #152 Not tainted
-----------------------------
net/tipc/bearer.c:177 suspicious rcu_dereference_protected() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by syzkaller021477/3194:
#0: (cb_lock){++++}, at: [<00000000d20133ea>] genl_rcv+0x19/0x40
net/netlink/genetlink.c:634
#1: (genl_mutex){+.+.}, at: [<00000000fcc5d1bc>] genl_lock
net/netlink/genetlink.c:33 [inline]
#1: (genl_mutex){+.+.}, at: [<00000000fcc5d1bc>] genl_rcv_msg+0x115/0x140
net/netlink/genetlink.c:622
stack backtrace:
CPU: 1 PID: 3194 Comm: syzkaller021477 Not tainted 4.15.0-rc5+ #152
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4585
tipc_bearer_find+0x2b4/0x3b0 net/tipc/bearer.c:177
tipc_nl_compat_link_set+0x329/0x9f0 net/tipc/netlink_compat.c:729
__tipc_nl_compat_doit net/tipc/netlink_compat.c:288 [inline]
tipc_nl_compat_doit+0x15b/0x660 net/tipc/netlink_compat.c:335
tipc_nl_compat_handle net/tipc/netlink_compat.c:1119 [inline]
tipc_nl_compat_recv+0x112f/0x18f0 net/tipc/netlink_compat.c:1201
genl_family_rcv_msg+0x7b7/0xfb0 net/netlink/genetlink.c:599
genl_rcv_msg+0xb2/0x140 net/netlink/genetlink.c:624
netlink_rcv_skb+0x21e/0x460 net/netlink/af_netlink.c:2408
genl_rcv+0x28/0x40 net/netlink/genetlink.c:635
netlink_unicast_kernel net/netlink/af_netlink.c:1275 [inline]
netlink_unicast+0x4e8/0x6f0 net/netlink/af_netlink.c:1301
netlink_sendmsg+0xa4a/0xe60 net/netlink/af_netlink.c:1864
sock_sendmsg_nosec net/socket.c:636 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:646
sock_write_iter+0x31a/0x5d0 net/socket.c:915
call_write_iter include/linux/fs.h:1772 [inline]
new_sync_write fs/read_write.c:469 [inline]
__vfs_write+0x684/0x970 fs/read_write.c:482
vfs_write+0x189/0x510 fs/read_write.c:544
SYSC_write fs/read_write.c:589 [inline]
SyS_write+0xef/0x220 fs/read_write.c:581
do_syscall_32_irqs_on arch/x86/entry/common.c:327 [inline]
do_fast_syscall_32+0x3ee/0xf9d arch/x86/entry/common.c:389
entry_SYSENTER_compat+0x54/0x63 arch/x86/entry/entry_64_compat.S:129
In order to correct the mistake, __tipc_nl_compat_doit() has been
protected by RTNL lock, which means the whole operation of setting
bearer/media properties is under RTNL protection.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reported-by: syzbot <syzbot+6345fd433db009b29413@syzkaller.appspotmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/tipc/netlink_compat.c | 14 |
1 files changed, 8 insertions, 6 deletions
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c index 974169059b9c..4492cda45566 100644 --- a/net/tipc/netlink_compat.c +++ b/net/tipc/netlink_compat.c | |||
@@ -301,6 +301,7 @@ static int __tipc_nl_compat_doit(struct tipc_nl_compat_cmd_doit *cmd, | |||
301 | memset(&info, 0, sizeof(info)); | 301 | memset(&info, 0, sizeof(info)); |
302 | info.attrs = attrbuf; | 302 | info.attrs = attrbuf; |
303 | 303 | ||
304 | rtnl_lock(); | ||
304 | err = (*cmd->transcode)(cmd, trans_buf, msg); | 305 | err = (*cmd->transcode)(cmd, trans_buf, msg); |
305 | if (err) | 306 | if (err) |
306 | goto doit_out; | 307 | goto doit_out; |
@@ -315,6 +316,7 @@ static int __tipc_nl_compat_doit(struct tipc_nl_compat_cmd_doit *cmd, | |||
315 | 316 | ||
316 | err = (*cmd->doit)(doit_buf, &info); | 317 | err = (*cmd->doit)(doit_buf, &info); |
317 | doit_out: | 318 | doit_out: |
319 | rtnl_unlock(); | ||
318 | 320 | ||
319 | kfree_skb(doit_buf); | 321 | kfree_skb(doit_buf); |
320 | attrbuf_out: | 322 | attrbuf_out: |
@@ -723,13 +725,13 @@ static int tipc_nl_compat_link_set(struct tipc_nl_compat_cmd_doit *cmd, | |||
723 | 725 | ||
724 | media = tipc_media_find(lc->name); | 726 | media = tipc_media_find(lc->name); |
725 | if (media) { | 727 | if (media) { |
726 | cmd->doit = &tipc_nl_media_set; | 728 | cmd->doit = &__tipc_nl_media_set; |
727 | return tipc_nl_compat_media_set(skb, msg); | 729 | return tipc_nl_compat_media_set(skb, msg); |
728 | } | 730 | } |
729 | 731 | ||
730 | bearer = tipc_bearer_find(msg->net, lc->name); | 732 | bearer = tipc_bearer_find(msg->net, lc->name); |
731 | if (bearer) { | 733 | if (bearer) { |
732 | cmd->doit = &tipc_nl_bearer_set; | 734 | cmd->doit = &__tipc_nl_bearer_set; |
733 | return tipc_nl_compat_bearer_set(skb, msg); | 735 | return tipc_nl_compat_bearer_set(skb, msg); |
734 | } | 736 | } |
735 | 737 | ||
@@ -1090,12 +1092,12 @@ static int tipc_nl_compat_handle(struct tipc_nl_compat_msg *msg) | |||
1090 | return tipc_nl_compat_dumpit(&dump, msg); | 1092 | return tipc_nl_compat_dumpit(&dump, msg); |
1091 | case TIPC_CMD_ENABLE_BEARER: | 1093 | case TIPC_CMD_ENABLE_BEARER: |
1092 | msg->req_type = TIPC_TLV_BEARER_CONFIG; | 1094 | msg->req_type = TIPC_TLV_BEARER_CONFIG; |
1093 | doit.doit = tipc_nl_bearer_enable; | 1095 | doit.doit = __tipc_nl_bearer_enable; |
1094 | doit.transcode = tipc_nl_compat_bearer_enable; | 1096 | doit.transcode = tipc_nl_compat_bearer_enable; |
1095 | return tipc_nl_compat_doit(&doit, msg); | 1097 | return tipc_nl_compat_doit(&doit, msg); |
1096 | case TIPC_CMD_DISABLE_BEARER: | 1098 | case TIPC_CMD_DISABLE_BEARER: |
1097 | msg->req_type = TIPC_TLV_BEARER_NAME; | 1099 | msg->req_type = TIPC_TLV_BEARER_NAME; |
1098 | doit.doit = tipc_nl_bearer_disable; | 1100 | doit.doit = __tipc_nl_bearer_disable; |
1099 | doit.transcode = tipc_nl_compat_bearer_disable; | 1101 | doit.transcode = tipc_nl_compat_bearer_disable; |
1100 | return tipc_nl_compat_doit(&doit, msg); | 1102 | return tipc_nl_compat_doit(&doit, msg); |
1101 | case TIPC_CMD_SHOW_LINK_STATS: | 1103 | case TIPC_CMD_SHOW_LINK_STATS: |
@@ -1149,12 +1151,12 @@ static int tipc_nl_compat_handle(struct tipc_nl_compat_msg *msg) | |||
1149 | return tipc_nl_compat_dumpit(&dump, msg); | 1151 | return tipc_nl_compat_dumpit(&dump, msg); |
1150 | case TIPC_CMD_SET_NODE_ADDR: | 1152 | case TIPC_CMD_SET_NODE_ADDR: |
1151 | msg->req_type = TIPC_TLV_NET_ADDR; | 1153 | msg->req_type = TIPC_TLV_NET_ADDR; |
1152 | doit.doit = tipc_nl_net_set; | 1154 | doit.doit = __tipc_nl_net_set; |
1153 | doit.transcode = tipc_nl_compat_net_set; | 1155 | doit.transcode = tipc_nl_compat_net_set; |
1154 | return tipc_nl_compat_doit(&doit, msg); | 1156 | return tipc_nl_compat_doit(&doit, msg); |
1155 | case TIPC_CMD_SET_NETID: | 1157 | case TIPC_CMD_SET_NETID: |
1156 | msg->req_type = TIPC_TLV_UNSIGNED; | 1158 | msg->req_type = TIPC_TLV_UNSIGNED; |
1157 | doit.doit = tipc_nl_net_set; | 1159 | doit.doit = __tipc_nl_net_set; |
1158 | doit.transcode = tipc_nl_compat_net_set; | 1160 | doit.transcode = tipc_nl_compat_net_set; |
1159 | return tipc_nl_compat_doit(&doit, msg); | 1161 | return tipc_nl_compat_doit(&doit, msg); |
1160 | case TIPC_CMD_GET_NETID: | 1162 | case TIPC_CMD_GET_NETID: |