iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'tipc-locking-fixes'

Browse Source 
Ying Xue says:

====================
tipc: Fix missing RTNL lock protection during setting link properties

At present it's unsafe to configure link properties through netlink
as the entire setting process is not under RTNL lock protection. Now
TIPC supports two different sets of netlink APIs at the same time, and
they share the same set of backend functions to configure bearer,
media and net properties. In order to solve the missing RTNL issue,
we have to make the whole __tipc_nl_compat_doit() protected by RTNL,
which means any function called within it cannot take RTNL any more.
So in the series we first introduce the following new functions which
doesn't hold RTNl lock:

 - __tipc_nl_bearer_disable()
 - __tipc_nl_bearer_enable()
 - __tipc_nl_bearer_set()
 - __tipc_nl_media_set()
 - __tipc_nl_net_set()

Meanwhile, __tipc_nl_compat_doit() has been reconstructed to minimize
the time of holding RTNL lock.

Changes in v4:
 - Per suggestion of Kirill Tkhai, divided original big one patch into
   seven small ones so that they can be easily reviewed.

Changes in v3:
 - Optimized return method of __tipc_nl_bearer_enable() regarding
   the comments from David M and Kirill Tkhai
 - Moved the allocations of memory in __tipc_nl_compat_doit() out
   of RTNL lock to minimize the time of holding RTNL lock according
   to the suggestion of Kirill Tkhai.

Changes in v2:
 - The whole operation of setting bearer/media properties has been
   protected under RTNL, as per feedback from David M.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
David S. Miller 2018-02-14 14:46:33 -05:00
commit 080fe7aa18
5 changed files with 91 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
net/tipc/bearer.c
View File

@ -813,7 +813,7 @@ err_out:
return err; 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; int err;
char *name; char *name;
@ -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]); name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);
rtnl_lock();
bearer = tipc_bearer_find(net, name); bearer = tipc_bearer_find(net, name);
if (!bearer) { if (!bearer)
rtnl_unlock();
return -EINVAL; return -EINVAL;
}
bearer_disable(net, bearer); bearer_disable(net, bearer);
rtnl_unlock();
return 0; 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; int err;
char *bearer; char *bearer;
@ -890,15 +897,18 @@ int tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info)
prio = nla_get_u32(props[TIPC_NLA_PROP_PRIO]); prio = nla_get_u32(props[TIPC_NLA_PROP_PRIO]);
} }
return tipc_enable_bearer(net, bearer, domain, prio, attrs);
}
int tipc_nl_bearer_enable(struct sk_buff *skb, struct genl_info *info)
{
int err;
rtnl_lock(); rtnl_lock();
err = tipc_enable_bearer(net, bearer, domain, prio, attrs); err = __tipc_nl_bearer_enable(skb, info);
if (err) {
rtnl_unlock();
return err;
}
rtnl_unlock(); rtnl_unlock();
return 0; return err;
} }
int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info) int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
@ -944,7 +954,7 @@ int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
return 0; 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; int err;
char *name; char *name;
@ -965,22 +975,17 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
return -EINVAL; return -EINVAL;
name = nla_data(attrs[TIPC_NLA_BEARER_NAME]); name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);
rtnl_lock();
b = tipc_bearer_find(net, name); b = tipc_bearer_find(net, name);
if (!b) { if (!b)
rtnl_unlock();
return -EINVAL; return -EINVAL;
}
if (attrs[TIPC_NLA_BEARER_PROP]) { if (attrs[TIPC_NLA_BEARER_PROP]) {
struct nlattr *props[TIPC_NLA_PROP_MAX + 1]; struct nlattr *props[TIPC_NLA_PROP_MAX + 1];
err = tipc_nl_parse_link_prop(attrs[TIPC_NLA_BEARER_PROP], err = tipc_nl_parse_link_prop(attrs[TIPC_NLA_BEARER_PROP],
props); props);
if (err) { if (err)
rtnl_unlock();
return err; return err;
}
if (props[TIPC_NLA_PROP_TOL]) if (props[TIPC_NLA_PROP_TOL])
b->tolerance = nla_get_u32(props[TIPC_NLA_PROP_TOL]); b->tolerance = nla_get_u32(props[TIPC_NLA_PROP_TOL]);
@ -989,11 +994,21 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
if (props[TIPC_NLA_PROP_WIN]) if (props[TIPC_NLA_PROP_WIN])
b->window = nla_get_u32(props[TIPC_NLA_PROP_WIN]); b->window = nla_get_u32(props[TIPC_NLA_PROP_WIN]);
} }
rtnl_unlock();
return 0; 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, static int __tipc_nl_add_media(struct tipc_nl_msg *msg,
struct tipc_media *media, int nlflags) struct tipc_media *media, int nlflags)
{ {
@ -1115,7 +1130,7 @@ err_out:
return err; 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; int err;
char *name; char *name;
@ -1133,22 +1148,17 @@ int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
return -EINVAL; return -EINVAL;
name = nla_data(attrs[TIPC_NLA_MEDIA_NAME]); name = nla_data(attrs[TIPC_NLA_MEDIA_NAME]);
rtnl_lock();
m = tipc_media_find(name); m = tipc_media_find(name);
if (!m) { if (!m)
rtnl_unlock();
return -EINVAL; return -EINVAL;
}
if (attrs[TIPC_NLA_MEDIA_PROP]) { if (attrs[TIPC_NLA_MEDIA_PROP]) {
struct nlattr *props[TIPC_NLA_PROP_MAX + 1]; struct nlattr *props[TIPC_NLA_PROP_MAX + 1];
err = tipc_nl_parse_link_prop(attrs[TIPC_NLA_MEDIA_PROP], err = tipc_nl_parse_link_prop(attrs[TIPC_NLA_MEDIA_PROP],
props); props);
if (err) { if (err)
rtnl_unlock();
return err; return err;
}
if (props[TIPC_NLA_PROP_TOL]) if (props[TIPC_NLA_PROP_TOL])
m->tolerance = nla_get_u32(props[TIPC_NLA_PROP_TOL]); m->tolerance = nla_get_u32(props[TIPC_NLA_PROP_TOL]);
@ -1157,7 +1167,17 @@ int tipc_nl_media_set(struct sk_buff *skb, struct genl_info *info)
if (props[TIPC_NLA_PROP_WIN]) if (props[TIPC_NLA_PROP_WIN])
m->window = nla_get_u32(props[TIPC_NLA_PROP_WIN]); m->window = nla_get_u32(props[TIPC_NLA_PROP_WIN]);
} }
rtnl_unlock();
return 0; 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
net/tipc/bearer.h
View File

@ -188,15 +188,19 @@ extern struct tipc_media udp_media_info;
#endif #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_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_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_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_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_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_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_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_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_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_priority(const char *name, u32 new_value);
int tipc_media_set_window(const char *name, u32 new_value); int tipc_media_set_window(const char *name, u32 new_value);

15
net/tipc/net.c
View File

@ -200,7 +200,7 @@ out:
return skb->len; 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 net *net = sock_net(skb->sk);
struct tipc_net *tn = net_generic(net, tipc_net_id); struct tipc_net *tn = net_generic(net, tipc_net_id);
@ -241,10 +241,19 @@ int tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info)
if (!tipc_addr_node_valid(addr)) if (!tipc_addr_node_valid(addr))
return -EINVAL; return -EINVAL;
rtnl_lock();
tipc_net_start(net, addr); tipc_net_start(net, addr);
rtnl_unlock();
} }
return 0; 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
net/tipc/net.h
View File

@ -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_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);
int __tipc_nl_net_set(struct sk_buff *skb, struct genl_info *info);
#endif #endif

43
net/tipc/netlink_compat.c
View File

@ -285,10 +285,6 @@ static int __tipc_nl_compat_doit(struct tipc_nl_compat_cmd_doit *cmd,
if (!trans_buf) if (!trans_buf)
return -ENOMEM; return -ENOMEM;
err = (*cmd->transcode)(cmd, trans_buf, msg);
if (err)
goto trans_out;
attrbuf = kmalloc((tipc_genl_family.maxattr + 1) * attrbuf = kmalloc((tipc_genl_family.maxattr + 1) *
sizeof(struct nlattr *), GFP_KERNEL); sizeof(struct nlattr *), GFP_KERNEL);
if (!attrbuf) { if (!attrbuf) {
@ -296,27 +292,34 @@ static int __tipc_nl_compat_doit(struct tipc_nl_compat_cmd_doit *cmd,
goto trans_out; goto trans_out;
} }
err = nla_parse(attrbuf, tipc_genl_family.maxattr,
(const struct nlattr *)trans_buf->data,
trans_buf->len, NULL, NULL);
if (err)
goto parse_out;
doit_buf = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL); doit_buf = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!doit_buf) { if (!doit_buf) {
err = -ENOMEM; err = -ENOMEM;
goto parse_out; goto attrbuf_out;
} }
doit_buf->sk = msg->dst_sk;
memset(&info, 0, sizeof(info)); memset(&info, 0, sizeof(info));
info.attrs = attrbuf; info.attrs = attrbuf;
rtnl_lock();
err = (*cmd->transcode)(cmd, trans_buf, msg);
if (err)
goto doit_out;
err = nla_parse(attrbuf, tipc_genl_family.maxattr,
(const struct nlattr *)trans_buf->data,
trans_buf->len, NULL, NULL);
if (err)
goto doit_out;
doit_buf->sk = msg->dst_sk;
err = (*cmd->doit)(doit_buf, &info); err = (*cmd->doit)(doit_buf, &info);
doit_out:
rtnl_unlock();
kfree_skb(doit_buf); kfree_skb(doit_buf);
parse_out: attrbuf_out:
kfree(attrbuf); kfree(attrbuf);
trans_out: trans_out:
kfree_skb(trans_buf); kfree_skb(trans_buf);
@ -722,13 +725,13 @@ static int tipc_nl_compat_link_set(struct tipc_nl_compat_cmd_doit *cmd,
media = tipc_media_find(lc->name); media = tipc_media_find(lc->name);
if (media) { if (media) {
cmd->doit = &tipc_nl_media_set; cmd->doit = &__tipc_nl_media_set;
return tipc_nl_compat_media_set(skb, msg); return tipc_nl_compat_media_set(skb, msg);
} }
bearer = tipc_bearer_find(msg->net, lc->name); bearer = tipc_bearer_find(msg->net, lc->name);
if (bearer) { if (bearer) {
cmd->doit = &tipc_nl_bearer_set; cmd->doit = &__tipc_nl_bearer_set;
return tipc_nl_compat_bearer_set(skb, msg); return tipc_nl_compat_bearer_set(skb, msg);
} }
@ -1089,12 +1092,12 @@ static int tipc_nl_compat_handle(struct tipc_nl_compat_msg *msg)
return tipc_nl_compat_dumpit(&dump, msg); return tipc_nl_compat_dumpit(&dump, msg);
case TIPC_CMD_ENABLE_BEARER: case TIPC_CMD_ENABLE_BEARER:
msg->req_type = TIPC_TLV_BEARER_CONFIG; 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; doit.transcode = tipc_nl_compat_bearer_enable;
return tipc_nl_compat_doit(&doit, msg); return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_DISABLE_BEARER: case TIPC_CMD_DISABLE_BEARER:
msg->req_type = TIPC_TLV_BEARER_NAME; 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; doit.transcode = tipc_nl_compat_bearer_disable;
return tipc_nl_compat_doit(&doit, msg); return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_SHOW_LINK_STATS: case TIPC_CMD_SHOW_LINK_STATS:
@ -1148,12 +1151,12 @@ static int tipc_nl_compat_handle(struct tipc_nl_compat_msg *msg)
return tipc_nl_compat_dumpit(&dump, msg); return tipc_nl_compat_dumpit(&dump, msg);
case TIPC_CMD_SET_NODE_ADDR: case TIPC_CMD_SET_NODE_ADDR:
msg->req_type = TIPC_TLV_NET_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; doit.transcode = tipc_nl_compat_net_set;
return tipc_nl_compat_doit(&doit, msg); return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_SET_NETID: case TIPC_CMD_SET_NETID:
msg->req_type = TIPC_TLV_UNSIGNED; 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; doit.transcode = tipc_nl_compat_net_set;
return tipc_nl_compat_doit(&doit, msg); return tipc_nl_compat_doit(&doit, msg);
case TIPC_CMD_GET_NETID: case TIPC_CMD_GET_NETID: