genetlink: clean up family attributes allocations

genl_family_rcv_msg_attrs_parse() and genl_family_rcv_msg_attrs_free()
take a boolean parameter to determine whether allocate/free the family
attrs. This is unnecessary as we can just check family->parallel_ops.
More importantly, callers would not need to worry about pairing these
parameters correctly after this patch.

And this fixes a memory leak, as after commit c36f05559104
("genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()")
we call genl_family_rcv_msg_attrs_parse() for both parallel and
non-parallel cases.

Fixes: c36f05559104 ("genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()")
Reported-by: Ido Schimmel <idosch@idosch.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Tested-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by Cong Wang and committed by David S. Miller b65ce380 18dbd4cd

+12 -16
+12 -16
net/netlink/genetlink.c
··· 474 struct netlink_ext_ack *extack, 475 const struct genl_ops *ops, 476 int hdrlen, 477 - enum genl_validate_flags no_strict_flag, 478 - bool parallel) 479 { 480 enum netlink_validation validate = ops->validate & no_strict_flag ? 481 NL_VALIDATE_LIBERAL : ··· 485 if (!family->maxattr) 486 return NULL; 487 488 - if (parallel) { 489 attrbuf = kmalloc_array(family->maxattr + 1, 490 sizeof(struct nlattr *), GFP_KERNEL); 491 if (!attrbuf) ··· 497 err = __nlmsg_parse(nlh, hdrlen, attrbuf, family->maxattr, 498 family->policy, validate, extack); 499 if (err) { 500 - if (parallel) 501 kfree(attrbuf); 502 return ERR_PTR(err); 503 } ··· 505 } 506 507 static void genl_family_rcv_msg_attrs_free(const struct genl_family *family, 508 - struct nlattr **attrbuf, 509 - bool parallel) 510 { 511 - if (parallel) 512 kfree(attrbuf); 513 } 514 ··· 535 536 attrs = genl_family_rcv_msg_attrs_parse(ctx->family, ctx->nlh, ctx->extack, 537 ops, ctx->hdrlen, 538 - GENL_DONT_VALIDATE_DUMP_STRICT, 539 - true); 540 if (IS_ERR(attrs)) 541 return PTR_ERR(attrs); 542 543 no_attrs: 544 info = genl_dumpit_info_alloc(); 545 if (!info) { 546 - kfree(attrs); 547 return -ENOMEM; 548 } 549 info->family = ctx->family; ··· 559 } 560 561 if (rc) { 562 - kfree(attrs); 563 genl_dumpit_info_free(info); 564 cb->data = NULL; 565 } ··· 588 rc = ops->done(cb); 589 genl_unlock(); 590 } 591 - genl_family_rcv_msg_attrs_free(info->family, info->attrs, false); 592 genl_dumpit_info_free(info); 593 return rc; 594 } ··· 601 602 if (ops->done) 603 rc = ops->done(cb); 604 - genl_family_rcv_msg_attrs_free(info->family, info->attrs, true); 605 genl_dumpit_info_free(info); 606 return rc; 607 } ··· 668 669 attrbuf = genl_family_rcv_msg_attrs_parse(family, nlh, extack, 670 ops, hdrlen, 671 - GENL_DONT_VALIDATE_STRICT, 672 - family->parallel_ops); 673 if (IS_ERR(attrbuf)) 674 return PTR_ERR(attrbuf); 675 ··· 694 family->post_doit(ops, skb, &info); 695 696 out: 697 - genl_family_rcv_msg_attrs_free(family, attrbuf, family->parallel_ops); 698 699 return err; 700 }
··· 474 struct netlink_ext_ack *extack, 475 const struct genl_ops *ops, 476 int hdrlen, 477 + enum genl_validate_flags no_strict_flag) 478 { 479 enum netlink_validation validate = ops->validate & no_strict_flag ? 480 NL_VALIDATE_LIBERAL : ··· 486 if (!family->maxattr) 487 return NULL; 488 489 + if (family->parallel_ops) { 490 attrbuf = kmalloc_array(family->maxattr + 1, 491 sizeof(struct nlattr *), GFP_KERNEL); 492 if (!attrbuf) ··· 498 err = __nlmsg_parse(nlh, hdrlen, attrbuf, family->maxattr, 499 family->policy, validate, extack); 500 if (err) { 501 + if (family->parallel_ops) 502 kfree(attrbuf); 503 return ERR_PTR(err); 504 } ··· 506 } 507 508 static void genl_family_rcv_msg_attrs_free(const struct genl_family *family, 509 + struct nlattr **attrbuf) 510 { 511 + if (family->parallel_ops) 512 kfree(attrbuf); 513 } 514 ··· 537 538 attrs = genl_family_rcv_msg_attrs_parse(ctx->family, ctx->nlh, ctx->extack, 539 ops, ctx->hdrlen, 540 + GENL_DONT_VALIDATE_DUMP_STRICT); 541 if (IS_ERR(attrs)) 542 return PTR_ERR(attrs); 543 544 no_attrs: 545 info = genl_dumpit_info_alloc(); 546 if (!info) { 547 + genl_family_rcv_msg_attrs_free(ctx->family, attrs); 548 return -ENOMEM; 549 } 550 info->family = ctx->family; ··· 562 } 563 564 if (rc) { 565 + genl_family_rcv_msg_attrs_free(info->family, info->attrs); 566 genl_dumpit_info_free(info); 567 cb->data = NULL; 568 } ··· 591 rc = ops->done(cb); 592 genl_unlock(); 593 } 594 + genl_family_rcv_msg_attrs_free(info->family, info->attrs); 595 genl_dumpit_info_free(info); 596 return rc; 597 } ··· 604 605 if (ops->done) 606 rc = ops->done(cb); 607 + genl_family_rcv_msg_attrs_free(info->family, info->attrs); 608 genl_dumpit_info_free(info); 609 return rc; 610 } ··· 671 672 attrbuf = genl_family_rcv_msg_attrs_parse(family, nlh, extack, 673 ops, hdrlen, 674 + GENL_DONT_VALIDATE_STRICT); 675 if (IS_ERR(attrbuf)) 676 return PTR_ERR(attrbuf); 677 ··· 698 family->post_doit(ops, skb, &info); 699 700 out: 701 + genl_family_rcv_msg_attrs_free(family, attrbuf); 702 703 return err; 704 }