Skip to content

Commit 4ae649e

Browse files
committed
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: David S. Miller <[email protected]>
2 parents dcadaec + 09e91db commit 4ae649e

File tree

2 files changed

+44
-35
lines changed

2 files changed

+44
-35
lines changed

net/hsr/hsr_framereg.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -482,12 +482,9 @@ int hsr_get_node_data(struct hsr_priv *hsr,
482482
struct hsr_port *port;
483483
unsigned long tdiff;
484484

485-
rcu_read_lock();
486485
node = find_node_by_addr_A(&hsr->node_db, addr);
487-
if (!node) {
488-
rcu_read_unlock();
489-
return -ENOENT; /* No such entry */
490-
}
486+
if (!node)
487+
return -ENOENT;
491488

492489
ether_addr_copy(addr_b, node->macaddress_B);
493490

@@ -522,7 +519,5 @@ int hsr_get_node_data(struct hsr_priv *hsr,
522519
*addr_b_ifindex = -1;
523520
}
524521

525-
rcu_read_unlock();
526-
527522
return 0;
528523
}

net/hsr/hsr_netlink.c

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -251,15 +251,16 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
251251
if (!na)
252252
goto invalid;
253253

254-
hsr_dev = __dev_get_by_index(genl_info_net(info),
255-
nla_get_u32(info->attrs[HSR_A_IFINDEX]));
254+
rcu_read_lock();
255+
hsr_dev = dev_get_by_index_rcu(genl_info_net(info),
256+
nla_get_u32(info->attrs[HSR_A_IFINDEX]));
256257
if (!hsr_dev)
257-
goto invalid;
258+
goto rcu_unlock;
258259
if (!is_hsr_master(hsr_dev))
259-
goto invalid;
260+
goto rcu_unlock;
260261

261262
/* Send reply */
262-
skb_out = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
263+
skb_out = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
263264
if (!skb_out) {
264265
res = -ENOMEM;
265266
goto fail;
@@ -313,12 +314,10 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
313314
res = nla_put_u16(skb_out, HSR_A_IF1_SEQ, hsr_node_if1_seq);
314315
if (res < 0)
315316
goto nla_put_failure;
316-
rcu_read_lock();
317317
port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_A);
318318
if (port)
319319
res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX,
320320
port->dev->ifindex);
321-
rcu_read_unlock();
322321
if (res < 0)
323322
goto nla_put_failure;
324323

@@ -328,20 +327,22 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
328327
res = nla_put_u16(skb_out, HSR_A_IF2_SEQ, hsr_node_if2_seq);
329328
if (res < 0)
330329
goto nla_put_failure;
331-
rcu_read_lock();
332330
port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_B);
333331
if (port)
334332
res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX,
335333
port->dev->ifindex);
336-
rcu_read_unlock();
337334
if (res < 0)
338335
goto nla_put_failure;
339336

337+
rcu_read_unlock();
338+
340339
genlmsg_end(skb_out, msg_head);
341340
genlmsg_unicast(genl_info_net(info), skb_out, info->snd_portid);
342341

343342
return 0;
344343

344+
rcu_unlock:
345+
rcu_read_unlock();
345346
invalid:
346347
netlink_ack(skb_in, nlmsg_hdr(skb_in), -EINVAL, NULL);
347348
return 0;
@@ -351,23 +352,22 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info)
351352
/* Fall through */
352353

353354
fail:
355+
rcu_read_unlock();
354356
return res;
355357
}
356358

357359
/* Get a list of MacAddressA of all nodes known to this node (including self).
358360
*/
359361
static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
360362
{
361-
/* For receiving */
362-
struct nlattr *na;
363+
unsigned char addr[ETH_ALEN];
363364
struct net_device *hsr_dev;
364-
365-
/* For sending */
366365
struct sk_buff *skb_out;
367-
void *msg_head;
368366
struct hsr_priv *hsr;
369-
void *pos;
370-
unsigned char addr[ETH_ALEN];
367+
bool restart = false;
368+
struct nlattr *na;
369+
void *pos = NULL;
370+
void *msg_head;
371371
int res;
372372

373373
if (!info)
@@ -377,15 +377,17 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
377377
if (!na)
378378
goto invalid;
379379

380-
hsr_dev = __dev_get_by_index(genl_info_net(info),
381-
nla_get_u32(info->attrs[HSR_A_IFINDEX]));
380+
rcu_read_lock();
381+
hsr_dev = dev_get_by_index_rcu(genl_info_net(info),
382+
nla_get_u32(info->attrs[HSR_A_IFINDEX]));
382383
if (!hsr_dev)
383-
goto invalid;
384+
goto rcu_unlock;
384385
if (!is_hsr_master(hsr_dev))
385-
goto invalid;
386+
goto rcu_unlock;
386387

388+
restart:
387389
/* Send reply */
388-
skb_out = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
390+
skb_out = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC);
389391
if (!skb_out) {
390392
res = -ENOMEM;
391393
goto fail;
@@ -399,18 +401,26 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info)
399401
goto nla_put_failure;
400402
}
401403

402-
res = nla_put_u32(skb_out, HSR_A_IFINDEX, hsr_dev->ifindex);
403-
if (res < 0)
404-
goto nla_put_failure;
404+
if (!restart) {
405+
res = nla_put_u32(skb_out, HSR_A_IFINDEX, hsr_dev->ifindex);
406+
if (res < 0)
407+
goto nla_put_failure;
408+
}
405409

406410
hsr = netdev_priv(hsr_dev);
407411

408-
rcu_read_lock();
409-
pos = hsr_get_next_node(hsr, NULL, addr);
412+
if (!pos)
413+
pos = hsr_get_next_node(hsr, NULL, addr);
410414
while (pos) {
411415
res = nla_put(skb_out, HSR_A_NODE_ADDR, ETH_ALEN, addr);
412416
if (res < 0) {
413-
rcu_read_unlock();
417+
if (res == -EMSGSIZE) {
418+
genlmsg_end(skb_out, msg_head);
419+
genlmsg_unicast(genl_info_net(info), skb_out,
420+
info->snd_portid);
421+
restart = true;
422+
goto restart;
423+
}
414424
goto nla_put_failure;
415425
}
416426
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)
422432

423433
return 0;
424434

435+
rcu_unlock:
436+
rcu_read_unlock();
425437
invalid:
426438
netlink_ack(skb_in, nlmsg_hdr(skb_in), -EINVAL, NULL);
427439
return 0;
428440

429441
nla_put_failure:
430-
kfree_skb(skb_out);
442+
nlmsg_free(skb_out);
431443
/* Fall through */
432444

433445
fail:
446+
rcu_read_unlock();
434447
return res;
435448
}
436449

@@ -457,6 +470,7 @@ static struct genl_family hsr_genl_family __ro_after_init = {
457470
.version = 1,
458471
.maxattr = HSR_A_MAX,
459472
.policy = hsr_genl_policy,
473+
.netnsok = true,
460474
.module = THIS_MODULE,
461475
.ops = hsr_ops,
462476
.n_ops = ARRAY_SIZE(hsr_ops),

0 commit comments

Comments
 (0)