Skip to content

Commit

Permalink
tipc: fix missing RTNL lock protection during setting link properties
Browse files Browse the repository at this point in the history 
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
=============================
WARNING: suspicious RCU usage
4.15.0-rc5+ torvalds#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+ torvalds#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 <[email protected]>
Reported-by: syzbot <[email protected]>
  • Loading branch information
@ying-xue
ying-xue authored and 0day robot committed Feb 14, 2018
1 parent a6b8881 commit 6ee21ac
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 39 deletions.
84 changes: 54 additions & 30 deletions net/tipc/bearer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ int tipc_nl_bearer_get(struct sk_buff *skb, struct genl_info *info)
return err;
}

int tipc_nl_bearer_disable(struct sk_buff *skb, struct genl_info *info)
int __tipc_nl_bearer_disable(struct sk_buff *skb, struct genl_info *info)
{
int err;
char *name;
Expand All @@ -835,20 +835,27 @@ int tipc_nl_bearer_disable(struct sk_buff *skb, struct genl_info *info)

name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);

rtnl_lock();
bearer = tipc_bearer_find(net, name);
if (!bearer) {
rtnl_unlock();
if (!bearer)
return -EINVAL;
}

bearer_disable(net, bearer);
rtnl_unlock();

return 0;
}

int tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info)
int tipc_nl_bearer_disable(struct sk_buff *skb, struct genl_info *info)
{
int err;

rtnl_lock();
err = __tipc_nl_bearer_disable(skb, info);
rtnl_unlock();

return err;
}

int __tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info)
{
int err;
char *bearer;
Expand Down Expand Up @@ -890,17 +897,24 @@ int tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info)
prio = nla_get_u32(props[TIPC_NLA_PROP_PRIO]);
}

rtnl_lock();
err = tipc_enable_bearer(net, bearer, domain, prio, attrs);
if (err) {
rtnl_unlock();
if (err)
return err;
}
rtnl_unlock();

return 0;
}

int tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info)
{
int err;

rtnl_lock();
err = __tipc_nl_bearer_enable(skb, info);
rtnl_unlock();

return err;
}

int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
{
int err;
Expand Down Expand Up @@ -944,7 +958,7 @@ int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
return 0;
}

int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
int __tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
{
int err;
char *name;
Expand All @@ -965,22 +979,17 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);

rtnl_lock();
b = tipc_bearer_find(net, name);
if (!b) {
rtnl_unlock();
if (!b)
return -EINVAL;
}

if (attrs[TIPC_NLA_BEARER_PROP]) {
struct nlattr *props[TIPC_NLA_PROP_MAX + 1];

err = tipc_nl_parse_link_prop(attrs[TIPC_NLA_BEARER_PROP],
props);
if (err) {
rtnl_unlock();
if (err)
return err;
}

if (props[TIPC_NLA_PROP_TOL])
b->tolerance = nla_get_u32(props[TIPC_NLA_PROP_TOL]);
Expand All @@ -989,11 +998,21 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
if (props[TIPC_NLA_PROP_WIN])
b->window = nla_get_u32(props[TIPC_NLA_PROP_WIN]);
}
rtnl_unlock();

return 0;
}

int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
{
int err;

rtnl_lock();
err = __tipc_nl_bearer_set(skb, info);
rtnl_unlock();

return err;
}

static int __tipc_nl_add_media(struct tipc_nl_msg *msg,
struct tipc_media *media, int nlflags)
{
Expand Down Expand Up @@ -1115,7 +1134,7 @@ int tipc_nl_media_get(struct sk_buff *skb, struct genl_info *info)
return err;
}

int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
int __tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
{
int err;
char *name;
Expand All @@ -1133,22 +1152,17 @@ int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
name = nla_data(attrs[TIPC_NLA_MEDIA_NAME]);

rtnl_lock();
m = tipc_media_find(name);
if (!m) {
rtnl_unlock();
if (!m)
return -EINVAL;
}

if (attrs[TIPC_NLA_MEDIA_PROP]) {
struct nlattr *props[TIPC_NLA_PROP_MAX + 1];

err = tipc_nl_parse_link_prop(attrs[TIPC_NLA_MEDIA_PROP],
props);
if (err) {
rtnl_unlock();
if (err)
return err;
}

if (props[TIPC_NLA_PROP_TOL])
m->tolerance = nla_get_u32(props[TIPC_NLA_PROP_TOL]);
Expand All @@ -1157,7 +1171,17 @@ int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
if (props[TIPC_NLA_PROP_WIN])
m->window = nla_get_u32(props[TIPC_NLA_PROP_WIN]);
}
rtnl_unlock();

return 0;
}

int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
{
int err;

rtnl_lock();
err = __tipc_nl_media_set(skb, info);
rtnl_unlock();

return err;
}
4 changes: 4 additions & 0 deletions net/tipc/bearer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,19 @@ extern struct tipc_media udp_media_info;
#endif

int tipc_nl_bearer_disable(struct sk_buff *skb, struct genl_info *info);
int __tipc_nl_bearer_disable(struct sk_buff *skb, struct genl_info *info);
int tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info);
int __tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info);
int tipc_nl_bearer_dump(struct sk_buff *skb, struct netlink_callback *cb);
int tipc_nl_bearer_get(struct sk_buff *skb, struct genl_info *info);
int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info);
int __tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info);
int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info);

int tipc_nl_media_dump(struct sk_buff *skb, struct netlink_callback *cb);
int tipc_nl_media_get(struct sk_buff *skb, struct genl_info *info);
int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info);
int __tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info);

int tipc_media_set_priority(const char *name, u32 new_value);
int tipc_media_set_window(const char *name, u32 new_value);
Expand Down
15 changes: 12 additions & 3 deletions net/tipc/net.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ int tipc_nl_net_dump(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}

int tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info)
int __tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info)
{
struct net *net = sock_net(skb->sk);
struct tipc_net *tn = net_generic(net, tipc_net_id);
Expand Down Expand Up @@ -241,10 +241,19 @@ int tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info)
if (!tipc_addr_node_valid(addr))
return -EINVAL;

rtnl_lock();
tipc_net_start(net, addr);
rtnl_unlock();
}

return 0;
}

int tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info)
{
int err;

rtnl_lock();
err = __tipc_nl_net_set(skb, info);
rtnl_unlock();

return err;
}
1 change: 1 addition & 0 deletions net/tipc/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ void tipc_net_stop(struct net *net);

int tipc_nl_net_dump(struct sk_buff *skb, struct netlink_callback *cb);
int tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info);
int __tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info);

#endif
14 changes: 8 additions & 6 deletions net/tipc/netlink_compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,9 @@ static int tipc_nl_compat_doit(struct tipc_nl_compat_cmd_doit *cmd,
if (msg->req_type && !TLV_CHECK_TYPE(msg->req, msg->req_type))
return -EINVAL;

rtnl_lock();
err = __tipc_nl_compat_doit(cmd, msg);
rtnl_unlock();
if (err)
return err;

Expand Down Expand Up @@ -722,13 +724,13 @@ static int tipc_nl_compat_link_set(struct tipc_nl_compat_cmd_doit *cmd,

media = tipc_media_find(lc->name);
if (media) {
cmd->doit = &tipc_nl_media_set;
cmd->doit = &__tipc_nl_media_set;
return tipc_nl_compat_media_set(skb, msg);
}

bearer = tipc_bearer_find(msg->net, lc->name);
if (bearer) {
cmd->doit = &tipc_nl_bearer_set;
cmd->doit = &__tipc_nl_bearer_set;
return tipc_nl_compat_bearer_set(skb, msg);
}

Expand Down Expand Up @@ -1089,12 +1091,12 @@ static int tipc_nl_compat_handle(struct tipc_nl_compat_msg *msg)
return tipc_nl_compat_dumpit(&dump, msg);
case TIPC_CMD_ENABLE_BEARER:
msg->req_type = TIPC_TLV_BEARER_CONFIG;
doit.doit = tipc_nl_bearer_enable;
doit.doit = __tipc_nl_bearer_enable;
doit.transcode = tipc_nl_compat_bearer_enable;
return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_DISABLE_BEARER:
msg->req_type = TIPC_TLV_BEARER_NAME;
doit.doit = tipc_nl_bearer_disable;
doit.doit = __tipc_nl_bearer_disable;
doit.transcode = tipc_nl_compat_bearer_disable;
return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_SHOW_LINK_STATS:
Expand Down Expand Up @@ -1148,12 +1150,12 @@ static int tipc_nl_compat_handle(struct tipc_nl_compat_msg *msg)
return tipc_nl_compat_dumpit(&dump, msg);
case TIPC_CMD_SET_NODE_ADDR:
msg->req_type = TIPC_TLV_NET_ADDR;
doit.doit = tipc_nl_net_set;
doit.doit = __tipc_nl_net_set;
doit.transcode = tipc_nl_compat_net_set;
return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_SET_NETID:
msg->req_type = TIPC_TLV_UNSIGNED;
doit.doit = tipc_nl_net_set;
doit.doit = __tipc_nl_net_set;
doit.transcode = tipc_nl_compat_net_set;
return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_GET_NETID:
Expand Down

0 comments on commit 6ee21ac

Please sign in to comment.