Commit 4ae649e8 authored by David S. Miller's avatar David S. Miller

Merge branch 'hsr-fix-several-bugs-in-generic-netlink-callback'

Taehee Yoo says:

====================
hsr: fix several bugs in generic netlink callback

This patchset is to fix several bugs they are related in
generic netlink callback in hsr module.

1. The first patch is to add missing rcu_read_lock() in
hsr_get_node_{list/status}().
The hsr_get_node_{list/status}() are not protected by RTNL because
they are callback functions of generic netlink.
But it calls __dev_get_by_index() without acquiring RTNL.
So, it would use unsafe data.

2. The second patch is to avoid failure of hsr_get_node_list().
hsr_get_node_list() is a callback of generic netlink and
it is used to get node information in userspace.
But, if there are so many nodes, it fails because of buffer size.
So, in this patch, restart routine is added.

3. The third patch is to set .netnsok flag to true.
If .netnsok flag is false, non-init_net namespace is not allowed to
operate generic netlink operations.
So, currently, non-init_net namespace has no way to get node information
because .netnsok is false in the current hsr code.

Change log:
v1->v2:
 - Preserve reverse christmas tree variable ordering in the second patch.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents dcadaec2 09e91dbe
...@@ -482,12 +482,9 @@ int hsr_get_node_data(struct hsr_priv *hsr, ...@@ -482,12 +482,9 @@ int hsr_get_node_data(struct hsr_priv *hsr,
struct hsr_port *port; struct hsr_port *port;
unsigned long tdiff; unsigned long tdiff;
rcu_read_lock();
node = find_node_by_addr_A(&hsr->node_db, addr); node = find_node_by_addr_A(&hsr->node_db, addr);
if (!node) { if (!node)
rcu_read_unlock(); return -ENOENT;
return -ENOENT; /* No such entry */
}
ether_addr_copy(addr_b, node->macaddress_B); ether_addr_copy(addr_b, node->macaddress_B);
...@@ -522,7 +519,5 @@ int hsr_get_node_data(struct hsr_priv *hsr, ...@@ -522,7 +519,5 @@ int hsr_get_node_data(struct hsr_priv *hsr,
*addr_b_ifindex = -1; *addr_b_ifindex = -1;
} }
rcu_read_unlock();
return 0; return 0;
} }
...@@ -251,15 +251,16 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) ...@@ -251,15 +251,16 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
if (!na) if (!na)
goto invalid; goto invalid;
hsr_dev = __dev_get_by_index(genl_info_net(info), rcu_read_lock();
hsr_dev = dev_get_by_index_rcu(genl_info_net(info),
nla_get_u32(info->attrs[HSR_A_IFINDEX])); nla_get_u32(info->attrs[HSR_A_IFINDEX]));
if (!hsr_dev) if (!hsr_dev)
goto invalid; goto rcu_unlock;
if (!is_hsr_master(hsr_dev)) if (!is_hsr_master(hsr_dev))
goto invalid; goto rcu_unlock;
/* Send reply */ /* Send reply */
skb_out = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL); skb_out = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
if (!skb_out) { if (!skb_out) {
res = -ENOMEM; res = -ENOMEM;
goto fail; goto fail;
...@@ -313,12 +314,10 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) ...@@ -313,12 +314,10 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
res = nla_put_u16(skb_out, HSR_A_IF1_SEQ, hsr_node_if1_seq); res = nla_put_u16(skb_out, HSR_A_IF1_SEQ, hsr_node_if1_seq);
if (res < 0) if (res < 0)
goto nla_put_failure; goto nla_put_failure;
rcu_read_lock();
port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_A); port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_A);
if (port) if (port)
res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX, res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX,
port->dev->ifindex); port->dev->ifindex);
rcu_read_unlock();
if (res < 0) if (res < 0)
goto nla_put_failure; goto nla_put_failure;
...@@ -328,20 +327,22 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) ...@@ -328,20 +327,22 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
res = nla_put_u16(skb_out, HSR_A_IF2_SEQ, hsr_node_if2_seq); res = nla_put_u16(skb_out, HSR_A_IF2_SEQ, hsr_node_if2_seq);
if (res < 0) if (res < 0)
goto nla_put_failure; goto nla_put_failure;
rcu_read_lock();
port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_B); port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_B);
if (port) if (port)
res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX, res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX,
port->dev->ifindex); port->dev->ifindex);
rcu_read_unlock();
if (res < 0) if (res < 0)
goto nla_put_failure; goto nla_put_failure;
rcu_read_unlock();
genlmsg_end(skb_out, msg_head); genlmsg_end(skb_out, msg_head);
genlmsg_unicast(genl_info_net(info), skb_out, info->snd_portid); genlmsg_unicast(genl_info_net(info), skb_out, info->snd_portid);
return 0; return 0;
rcu_unlock:
rcu_read_unlock();
invalid: invalid:
netlink_ack(skb_in, nlmsg_hdr(skb_in), -EINVAL, NULL); netlink_ack(skb_in, nlmsg_hdr(skb_in), -EINVAL, NULL);
return 0; return 0;
...@@ -351,6 +352,7 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) ...@@ -351,6 +352,7 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
/* Fall through */ /* Fall through */
fail: fail:
rcu_read_unlock();
return res; return res;
} }
...@@ -358,16 +360,14 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) ...@@ -358,16 +360,14 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
*/ */
static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
{ {
/* For receiving */ unsigned char addr[ETH_ALEN];
struct nlattr *na;
struct net_device *hsr_dev; struct net_device *hsr_dev;
/* For sending */
struct sk_buff *skb_out; struct sk_buff *skb_out;
void *msg_head;
struct hsr_priv *hsr; struct hsr_priv *hsr;
void *pos; bool restart = false;
unsigned char addr[ETH_ALEN]; struct nlattr *na;
void *pos = NULL;
void *msg_head;
int res; int res;
if (!info) if (!info)
...@@ -377,15 +377,17 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) ...@@ -377,15 +377,17 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
if (!na) if (!na)
goto invalid; goto invalid;
hsr_dev = __dev_get_by_index(genl_info_net(info), rcu_read_lock();
hsr_dev = dev_get_by_index_rcu(genl_info_net(info),
nla_get_u32(info->attrs[HSR_A_IFINDEX])); nla_get_u32(info->attrs[HSR_A_IFINDEX]));
if (!hsr_dev) if (!hsr_dev)
goto invalid; goto rcu_unlock;
if (!is_hsr_master(hsr_dev)) if (!is_hsr_master(hsr_dev))
goto invalid; goto rcu_unlock;
restart:
/* Send reply */ /* Send reply */
skb_out = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL); skb_out = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC);
if (!skb_out) { if (!skb_out) {
res = -ENOMEM; res = -ENOMEM;
goto fail; goto fail;
...@@ -399,18 +401,26 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) ...@@ -399,18 +401,26 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
goto nla_put_failure; goto nla_put_failure;
} }
if (!restart) {
res = nla_put_u32(skb_out, HSR_A_IFINDEX, hsr_dev->ifindex); res = nla_put_u32(skb_out, HSR_A_IFINDEX, hsr_dev->ifindex);
if (res < 0) if (res < 0)
goto nla_put_failure; goto nla_put_failure;
}
hsr = netdev_priv(hsr_dev); hsr = netdev_priv(hsr_dev);
rcu_read_lock(); if (!pos)
pos = hsr_get_next_node(hsr, NULL, addr); pos = hsr_get_next_node(hsr, NULL, addr);
while (pos) { while (pos) {
res = nla_put(skb_out, HSR_A_NODE_ADDR, ETH_ALEN, addr); res = nla_put(skb_out, HSR_A_NODE_ADDR, ETH_ALEN, addr);
if (res < 0) { if (res < 0) {
rcu_read_unlock(); if (res == -EMSGSIZE) {
genlmsg_end(skb_out, msg_head);
genlmsg_unicast(genl_info_net(info), skb_out,
info->snd_portid);
restart = true;
goto restart;
}
goto nla_put_failure; goto nla_put_failure;
} }
pos = hsr_get_next_node(hsr, pos, addr); pos = hsr_get_next_node(hsr, pos, addr);
...@@ -422,15 +432,18 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) ...@@ -422,15 +432,18 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
return 0; return 0;
rcu_unlock:
rcu_read_unlock();
invalid: invalid:
netlink_ack(skb_in, nlmsg_hdr(skb_in), -EINVAL, NULL); netlink_ack(skb_in, nlmsg_hdr(skb_in), -EINVAL, NULL);
return 0; return 0;
nla_put_failure: nla_put_failure:
kfree_skb(skb_out); nlmsg_free(skb_out);
/* Fall through */ /* Fall through */
fail: fail:
rcu_read_unlock();
return res; return res;
} }
...@@ -457,6 +470,7 @@ static struct genl_family hsr_genl_family __ro_after_init = { ...@@ -457,6 +470,7 @@ static struct genl_family hsr_genl_family __ro_after_init = {
.version = 1, .version = 1,
.maxattr = HSR_A_MAX, .maxattr = HSR_A_MAX,
.policy = hsr_genl_policy, .policy = hsr_genl_policy,
.netnsok = true,
.module = THIS_MODULE, .module = THIS_MODULE,
.ops = hsr_ops, .ops = hsr_ops,
.n_ops = ARRAY_SIZE(hsr_ops), .n_ops = ARRAY_SIZE(hsr_ops),
......
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