Message ID | 20210713094158.450434-1-mudongliangabcd@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | audit: fix memory leak in nf_tables_commit | expand |
On Tue, Jul 13, 2021 at 1:52 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > On Tue, Jul 13, 2021 at 7:47 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > On Tue, Jul 13, 2021 at 11:42 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > > > > > > > > Reported-by: syzbot <syzkaller@googlegroups.com> > > > > As far as I see, the more default way is to reference to syzbot by: > > > > Reported-by: syzbot+[[20-letter hex reference]]@syzkaller.appspotmail.com > > > > Hi Lukas, > > this bug is not listed on the syzbot dashboard. I found this bug by > setting up a local syzkaller instance, so I only list syzbot other > than concrete syzbot id. > I see. Thanks for your explanation. Lukas
Hi Dongliang, Thank you for the patch! Yet something to improve: [auto build test ERROR on nf/master] [also build test ERROR on nf-next/master ipvs/master v5.14-rc1 next-20210713] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Dongliang-Mu/audit-fix-memory-leak-in-nf_tables_commit/20210713-174434 base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master config: alpha-randconfig-r002-20210713 (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/2112ee88ee1fa56b43d8d4ba2554d8d94199bd37 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Dongliang-Mu/audit-fix-memory-leak-in-nf_tables_commit/20210713-174434 git checkout 2112ee88ee1fa56b43d8d4ba2554d8d94199bd37 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash net/netfilter/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): net/netfilter/nf_tables_api.c: In function 'nf_tables_commit': >> net/netfilter/nf_tables_api.c:8522:26: error: incompatible type for argument 1 of 'nf_tables_commit_free' 8522 | nf_tables_commit_free(adl); | ^~~ | | | struct list_head net/netfilter/nf_tables_api.c:8448:53: note: expected 'struct list_head *' but argument is of type 'struct list_head' 8448 | static void nf_tables_commit_free(struct list_head *adl) | ~~~~~~~~~~~~~~~~~~^~~ net/netfilter/nf_tables_api.c:8532:27: error: incompatible type for argument 1 of 'nf_tables_commit_free' 8532 | nf_tables_commit_free(adl); | ^~~ | | | struct list_head net/netfilter/nf_tables_api.c:8448:53: note: expected 'struct list_head *' but argument is of type 'struct list_head' 8448 | static void nf_tables_commit_free(struct list_head *adl) | ~~~~~~~~~~~~~~~~~~^~~ vim +/nf_tables_commit_free +8522 net/netfilter/nf_tables_api.c 8491 8492 static int nf_tables_commit(struct net *net, struct sk_buff *skb) 8493 { 8494 struct nftables_pernet *nft_net = nft_pernet(net); 8495 struct nft_trans *trans, *next; 8496 struct nft_trans_elem *te; 8497 struct nft_chain *chain; 8498 struct nft_table *table; 8499 LIST_HEAD(adl); 8500 int err; 8501 8502 if (list_empty(&nft_net->commit_list)) { 8503 mutex_unlock(&nft_net->commit_mutex); 8504 return 0; 8505 } 8506 8507 /* 0. Validate ruleset, otherwise roll back for error reporting. */ 8508 if (nf_tables_validate(net) < 0) 8509 return -EAGAIN; 8510 8511 err = nft_flow_rule_offload_commit(net); 8512 if (err < 0) 8513 return err; 8514 8515 /* 1. Allocate space for next generation rules_gen_X[] */ 8516 list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) { 8517 int ret; 8518 8519 ret = nf_tables_commit_audit_alloc(&adl, trans->ctx.table); 8520 if (ret) { 8521 nf_tables_commit_chain_prepare_cancel(net); > 8522 nf_tables_commit_free(adl); 8523 return ret; 8524 } 8525 if (trans->msg_type == NFT_MSG_NEWRULE || 8526 trans->msg_type == NFT_MSG_DELRULE) { 8527 chain = trans->ctx.chain; 8528 8529 ret = nf_tables_commit_chain_prepare(net, chain); 8530 if (ret < 0) { 8531 nf_tables_commit_chain_prepare_cancel(net); 8532 nf_tables_commit_free(adl); 8533 return ret; 8534 } 8535 } 8536 } 8537 8538 /* step 2. Make rules_gen_X visible to packet path */ 8539 list_for_each_entry(table, &nft_net->tables, list) { 8540 list_for_each_entry(chain, &table->chains, list) 8541 nf_tables_commit_chain(net, chain); 8542 } 8543 8544 /* 8545 * Bump generation counter, invalidate any dump in progress. 8546 * Cannot fail after this point. 8547 */ 8548 while (++nft_net->base_seq == 0) 8549 ; 8550 8551 /* step 3. Start new generation, rules_gen_X now in use. */ 8552 net->nft.gencursor = nft_gencursor_next(net); 8553 8554 list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) { 8555 nf_tables_commit_audit_collect(&adl, trans->ctx.table, 8556 trans->msg_type); 8557 switch (trans->msg_type) { 8558 case NFT_MSG_NEWTABLE: 8559 if (nft_trans_table_update(trans)) { 8560 if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) { 8561 nft_trans_destroy(trans); 8562 break; 8563 } 8564 if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT) 8565 nf_tables_table_disable(net, trans->ctx.table); 8566 8567 trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE; 8568 } else { 8569 nft_clear(net, trans->ctx.table); 8570 } 8571 nf_tables_table_notify(&trans->ctx, NFT_MSG_NEWTABLE); 8572 nft_trans_destroy(trans); 8573 break; 8574 case NFT_MSG_DELTABLE: 8575 list_del_rcu(&trans->ctx.table->list); 8576 nf_tables_table_notify(&trans->ctx, NFT_MSG_DELTABLE); 8577 break; 8578 case NFT_MSG_NEWCHAIN: 8579 if (nft_trans_chain_update(trans)) { 8580 nft_chain_commit_update(trans); 8581 nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN); 8582 /* trans destroyed after rcu grace period */ 8583 } else { 8584 nft_chain_commit_drop_policy(trans); 8585 nft_clear(net, trans->ctx.chain); 8586 nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN); 8587 nft_trans_destroy(trans); 8588 } 8589 break; 8590 case NFT_MSG_DELCHAIN: 8591 nft_chain_del(trans->ctx.chain); 8592 nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN); 8593 nf_tables_unregister_hook(trans->ctx.net, 8594 trans->ctx.table, 8595 trans->ctx.chain); 8596 break; 8597 case NFT_MSG_NEWRULE: 8598 nft_clear(trans->ctx.net, nft_trans_rule(trans)); 8599 nf_tables_rule_notify(&trans->ctx, 8600 nft_trans_rule(trans), 8601 NFT_MSG_NEWRULE); 8602 nft_trans_destroy(trans); 8603 break; 8604 case NFT_MSG_DELRULE: 8605 list_del_rcu(&nft_trans_rule(trans)->list); 8606 nf_tables_rule_notify(&trans->ctx, 8607 nft_trans_rule(trans), 8608 NFT_MSG_DELRULE); 8609 nft_rule_expr_deactivate(&trans->ctx, 8610 nft_trans_rule(trans), 8611 NFT_TRANS_COMMIT); 8612 break; 8613 case NFT_MSG_NEWSET: 8614 nft_clear(net, nft_trans_set(trans)); 8615 /* This avoids hitting -EBUSY when deleting the table 8616 * from the transaction. 8617 */ 8618 if (nft_set_is_anonymous(nft_trans_set(trans)) && 8619 !list_empty(&nft_trans_set(trans)->bindings)) 8620 trans->ctx.table->use--; 8621 8622 nf_tables_set_notify(&trans->ctx, nft_trans_set(trans), 8623 NFT_MSG_NEWSET, GFP_KERNEL); 8624 nft_trans_destroy(trans); 8625 break; 8626 case NFT_MSG_DELSET: 8627 list_del_rcu(&nft_trans_set(trans)->list); 8628 nf_tables_set_notify(&trans->ctx, nft_trans_set(trans), 8629 NFT_MSG_DELSET, GFP_KERNEL); 8630 break; 8631 case NFT_MSG_NEWSETELEM: 8632 te = (struct nft_trans_elem *)trans->data; 8633 8634 nft_setelem_activate(net, te->set, &te->elem); 8635 nf_tables_setelem_notify(&trans->ctx, te->set, 8636 &te->elem, 8637 NFT_MSG_NEWSETELEM, 0); 8638 nft_trans_destroy(trans); 8639 break; 8640 case NFT_MSG_DELSETELEM: 8641 te = (struct nft_trans_elem *)trans->data; 8642 8643 nf_tables_setelem_notify(&trans->ctx, te->set, 8644 &te->elem, 8645 NFT_MSG_DELSETELEM, 0); 8646 nft_setelem_remove(net, te->set, &te->elem); 8647 if (!nft_setelem_is_catchall(te->set, &te->elem)) { 8648 atomic_dec(&te->set->nelems); 8649 te->set->ndeact--; 8650 } 8651 break; 8652 case NFT_MSG_NEWOBJ: 8653 if (nft_trans_obj_update(trans)) { 8654 nft_obj_commit_update(trans); 8655 nf_tables_obj_notify(&trans->ctx, 8656 nft_trans_obj(trans), 8657 NFT_MSG_NEWOBJ); 8658 } else { 8659 nft_clear(net, nft_trans_obj(trans)); 8660 nf_tables_obj_notify(&trans->ctx, 8661 nft_trans_obj(trans), 8662 NFT_MSG_NEWOBJ); 8663 nft_trans_destroy(trans); 8664 } 8665 break; 8666 case NFT_MSG_DELOBJ: 8667 nft_obj_del(nft_trans_obj(trans)); 8668 nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans), 8669 NFT_MSG_DELOBJ); 8670 break; 8671 case NFT_MSG_NEWFLOWTABLE: 8672 if (nft_trans_flowtable_update(trans)) { 8673 nft_trans_flowtable(trans)->data.flags = 8674 nft_trans_flowtable_flags(trans); 8675 nf_tables_flowtable_notify(&trans->ctx, 8676 nft_trans_flowtable(trans), 8677 &nft_trans_flowtable_hooks(trans), 8678 NFT_MSG_NEWFLOWTABLE); 8679 list_splice(&nft_trans_flowtable_hooks(trans), 8680 &nft_trans_flowtable(trans)->hook_list); 8681 } else { 8682 nft_clear(net, nft_trans_flowtable(trans)); 8683 nf_tables_flowtable_notify(&trans->ctx, 8684 nft_trans_flowtable(trans), 8685 &nft_trans_flowtable(trans)->hook_list, 8686 NFT_MSG_NEWFLOWTABLE); 8687 } 8688 nft_trans_destroy(trans); 8689 break; 8690 case NFT_MSG_DELFLOWTABLE: 8691 if (nft_trans_flowtable_update(trans)) { 8692 nft_flowtable_hooks_del(nft_trans_flowtable(trans), 8693 &nft_trans_flowtable_hooks(trans)); 8694 nf_tables_flowtable_notify(&trans->ctx, 8695 nft_trans_flowtable(trans), 8696 &nft_trans_flowtable_hooks(trans), 8697 NFT_MSG_DELFLOWTABLE); 8698 nft_unregister_flowtable_net_hooks(net, 8699 &nft_trans_flowtable_hooks(trans)); 8700 } else { 8701 list_del_rcu(&nft_trans_flowtable(trans)->list); 8702 nf_tables_flowtable_notify(&trans->ctx, 8703 nft_trans_flowtable(trans), 8704 &nft_trans_flowtable(trans)->hook_list, 8705 NFT_MSG_DELFLOWTABLE); 8706 nft_unregister_flowtable_net_hooks(net, 8707 &nft_trans_flowtable(trans)->hook_list); 8708 } 8709 break; 8710 } 8711 } 8712 8713 nft_commit_notify(net, NETLINK_CB(skb).portid); 8714 nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN); 8715 nf_tables_commit_audit_log(&adl, nft_net->base_seq); 8716 nf_tables_commit_release(net); 8717 8718 return 0; 8719 } 8720 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Dongliang, Thank you for the patch! Yet something to improve: [auto build test ERROR on nf/master] [also build test ERROR on nf-next/master ipvs/master v5.14-rc1 next-20210713] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Dongliang-Mu/audit-fix-memory-leak-in-nf_tables_commit/20210713-174434 base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master config: x86_64-randconfig-a005-20210713 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8d69635ed9ecf36fd0ca85906bfde17949671cbe) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/2112ee88ee1fa56b43d8d4ba2554d8d94199bd37 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Dongliang-Mu/audit-fix-memory-leak-in-nf_tables_commit/20210713-174434 git checkout 2112ee88ee1fa56b43d8d4ba2554d8d94199bd37 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> net/netfilter/nf_tables_api.c:8522:26: error: passing 'struct list_head' to parameter of incompatible type 'struct list_head *'; take the address with & nf_tables_commit_free(adl); ^~~ & net/netfilter/nf_tables_api.c:8448:53: note: passing argument to parameter 'adl' here static void nf_tables_commit_free(struct list_head *adl) ^ net/netfilter/nf_tables_api.c:8532:27: error: passing 'struct list_head' to parameter of incompatible type 'struct list_head *'; take the address with & nf_tables_commit_free(adl); ^~~ & net/netfilter/nf_tables_api.c:8448:53: note: passing argument to parameter 'adl' here static void nf_tables_commit_free(struct list_head *adl) ^ 2 errors generated. vim +8522 net/netfilter/nf_tables_api.c 8491 8492 static int nf_tables_commit(struct net *net, struct sk_buff *skb) 8493 { 8494 struct nftables_pernet *nft_net = nft_pernet(net); 8495 struct nft_trans *trans, *next; 8496 struct nft_trans_elem *te; 8497 struct nft_chain *chain; 8498 struct nft_table *table; 8499 LIST_HEAD(adl); 8500 int err; 8501 8502 if (list_empty(&nft_net->commit_list)) { 8503 mutex_unlock(&nft_net->commit_mutex); 8504 return 0; 8505 } 8506 8507 /* 0. Validate ruleset, otherwise roll back for error reporting. */ 8508 if (nf_tables_validate(net) < 0) 8509 return -EAGAIN; 8510 8511 err = nft_flow_rule_offload_commit(net); 8512 if (err < 0) 8513 return err; 8514 8515 /* 1. Allocate space for next generation rules_gen_X[] */ 8516 list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) { 8517 int ret; 8518 8519 ret = nf_tables_commit_audit_alloc(&adl, trans->ctx.table); 8520 if (ret) { 8521 nf_tables_commit_chain_prepare_cancel(net); > 8522 nf_tables_commit_free(adl); 8523 return ret; 8524 } 8525 if (trans->msg_type == NFT_MSG_NEWRULE || 8526 trans->msg_type == NFT_MSG_DELRULE) { 8527 chain = trans->ctx.chain; 8528 8529 ret = nf_tables_commit_chain_prepare(net, chain); 8530 if (ret < 0) { 8531 nf_tables_commit_chain_prepare_cancel(net); 8532 nf_tables_commit_free(adl); 8533 return ret; 8534 } 8535 } 8536 } 8537 8538 /* step 2. Make rules_gen_X visible to packet path */ 8539 list_for_each_entry(table, &nft_net->tables, list) { 8540 list_for_each_entry(chain, &table->chains, list) 8541 nf_tables_commit_chain(net, chain); 8542 } 8543 8544 /* 8545 * Bump generation counter, invalidate any dump in progress. 8546 * Cannot fail after this point. 8547 */ 8548 while (++nft_net->base_seq == 0) 8549 ; 8550 8551 /* step 3. Start new generation, rules_gen_X now in use. */ 8552 net->nft.gencursor = nft_gencursor_next(net); 8553 8554 list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) { 8555 nf_tables_commit_audit_collect(&adl, trans->ctx.table, 8556 trans->msg_type); 8557 switch (trans->msg_type) { 8558 case NFT_MSG_NEWTABLE: 8559 if (nft_trans_table_update(trans)) { 8560 if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) { 8561 nft_trans_destroy(trans); 8562 break; 8563 } 8564 if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT) 8565 nf_tables_table_disable(net, trans->ctx.table); 8566 8567 trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE; 8568 } else { 8569 nft_clear(net, trans->ctx.table); 8570 } 8571 nf_tables_table_notify(&trans->ctx, NFT_MSG_NEWTABLE); 8572 nft_trans_destroy(trans); 8573 break; 8574 case NFT_MSG_DELTABLE: 8575 list_del_rcu(&trans->ctx.table->list); 8576 nf_tables_table_notify(&trans->ctx, NFT_MSG_DELTABLE); 8577 break; 8578 case NFT_MSG_NEWCHAIN: 8579 if (nft_trans_chain_update(trans)) { 8580 nft_chain_commit_update(trans); 8581 nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN); 8582 /* trans destroyed after rcu grace period */ 8583 } else { 8584 nft_chain_commit_drop_policy(trans); 8585 nft_clear(net, trans->ctx.chain); 8586 nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN); 8587 nft_trans_destroy(trans); 8588 } 8589 break; 8590 case NFT_MSG_DELCHAIN: 8591 nft_chain_del(trans->ctx.chain); 8592 nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN); 8593 nf_tables_unregister_hook(trans->ctx.net, 8594 trans->ctx.table, 8595 trans->ctx.chain); 8596 break; 8597 case NFT_MSG_NEWRULE: 8598 nft_clear(trans->ctx.net, nft_trans_rule(trans)); 8599 nf_tables_rule_notify(&trans->ctx, 8600 nft_trans_rule(trans), 8601 NFT_MSG_NEWRULE); 8602 nft_trans_destroy(trans); 8603 break; 8604 case NFT_MSG_DELRULE: 8605 list_del_rcu(&nft_trans_rule(trans)->list); 8606 nf_tables_rule_notify(&trans->ctx, 8607 nft_trans_rule(trans), 8608 NFT_MSG_DELRULE); 8609 nft_rule_expr_deactivate(&trans->ctx, 8610 nft_trans_rule(trans), 8611 NFT_TRANS_COMMIT); 8612 break; 8613 case NFT_MSG_NEWSET: 8614 nft_clear(net, nft_trans_set(trans)); 8615 /* This avoids hitting -EBUSY when deleting the table 8616 * from the transaction. 8617 */ 8618 if (nft_set_is_anonymous(nft_trans_set(trans)) && 8619 !list_empty(&nft_trans_set(trans)->bindings)) 8620 trans->ctx.table->use--; 8621 8622 nf_tables_set_notify(&trans->ctx, nft_trans_set(trans), 8623 NFT_MSG_NEWSET, GFP_KERNEL); 8624 nft_trans_destroy(trans); 8625 break; 8626 case NFT_MSG_DELSET: 8627 list_del_rcu(&nft_trans_set(trans)->list); 8628 nf_tables_set_notify(&trans->ctx, nft_trans_set(trans), 8629 NFT_MSG_DELSET, GFP_KERNEL); 8630 break; 8631 case NFT_MSG_NEWSETELEM: 8632 te = (struct nft_trans_elem *)trans->data; 8633 8634 nft_setelem_activate(net, te->set, &te->elem); 8635 nf_tables_setelem_notify(&trans->ctx, te->set, 8636 &te->elem, 8637 NFT_MSG_NEWSETELEM, 0); 8638 nft_trans_destroy(trans); 8639 break; 8640 case NFT_MSG_DELSETELEM: 8641 te = (struct nft_trans_elem *)trans->data; 8642 8643 nf_tables_setelem_notify(&trans->ctx, te->set, 8644 &te->elem, 8645 NFT_MSG_DELSETELEM, 0); 8646 nft_setelem_remove(net, te->set, &te->elem); 8647 if (!nft_setelem_is_catchall(te->set, &te->elem)) { 8648 atomic_dec(&te->set->nelems); 8649 te->set->ndeact--; 8650 } 8651 break; 8652 case NFT_MSG_NEWOBJ: 8653 if (nft_trans_obj_update(trans)) { 8654 nft_obj_commit_update(trans); 8655 nf_tables_obj_notify(&trans->ctx, 8656 nft_trans_obj(trans), 8657 NFT_MSG_NEWOBJ); 8658 } else { 8659 nft_clear(net, nft_trans_obj(trans)); 8660 nf_tables_obj_notify(&trans->ctx, 8661 nft_trans_obj(trans), 8662 NFT_MSG_NEWOBJ); 8663 nft_trans_destroy(trans); 8664 } 8665 break; 8666 case NFT_MSG_DELOBJ: 8667 nft_obj_del(nft_trans_obj(trans)); 8668 nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans), 8669 NFT_MSG_DELOBJ); 8670 break; 8671 case NFT_MSG_NEWFLOWTABLE: 8672 if (nft_trans_flowtable_update(trans)) { 8673 nft_trans_flowtable(trans)->data.flags = 8674 nft_trans_flowtable_flags(trans); 8675 nf_tables_flowtable_notify(&trans->ctx, 8676 nft_trans_flowtable(trans), 8677 &nft_trans_flowtable_hooks(trans), 8678 NFT_MSG_NEWFLOWTABLE); 8679 list_splice(&nft_trans_flowtable_hooks(trans), 8680 &nft_trans_flowtable(trans)->hook_list); 8681 } else { 8682 nft_clear(net, nft_trans_flowtable(trans)); 8683 nf_tables_flowtable_notify(&trans->ctx, 8684 nft_trans_flowtable(trans), 8685 &nft_trans_flowtable(trans)->hook_list, 8686 NFT_MSG_NEWFLOWTABLE); 8687 } 8688 nft_trans_destroy(trans); 8689 break; 8690 case NFT_MSG_DELFLOWTABLE: 8691 if (nft_trans_flowtable_update(trans)) { 8692 nft_flowtable_hooks_del(nft_trans_flowtable(trans), 8693 &nft_trans_flowtable_hooks(trans)); 8694 nf_tables_flowtable_notify(&trans->ctx, 8695 nft_trans_flowtable(trans), 8696 &nft_trans_flowtable_hooks(trans), 8697 NFT_MSG_DELFLOWTABLE); 8698 nft_unregister_flowtable_net_hooks(net, 8699 &nft_trans_flowtable_hooks(trans)); 8700 } else { 8701 list_del_rcu(&nft_trans_flowtable(trans)->list); 8702 nf_tables_flowtable_notify(&trans->ctx, 8703 nft_trans_flowtable(trans), 8704 &nft_trans_flowtable(trans)->hook_list, 8705 NFT_MSG_DELFLOWTABLE); 8706 nft_unregister_flowtable_net_hooks(net, 8707 &nft_trans_flowtable(trans)->hook_list); 8708 } 8709 break; 8710 } 8711 } 8712 8713 nft_commit_notify(net, NETLINK_CB(skb).portid); 8714 nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN); 8715 nf_tables_commit_audit_log(&adl, nft_net->base_seq); 8716 nf_tables_commit_release(net); 8717 8718 return 0; 8719 } 8720 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 390d4466567f..7f45b291be13 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -8444,6 +8444,16 @@ static int nf_tables_commit_audit_alloc(struct list_head *adl, return 0; } +static void nf_tables_commit_free(struct list_head *adl) +{ + struct nft_audit_data *adp, *adn; + + list_for_each_entry_safe(adp, adn, adl, list) { + list_del(&adp->list); + kfree(adp); + } +} + static void nf_tables_commit_audit_collect(struct list_head *adl, struct nft_table *table, u32 op) { @@ -8508,6 +8518,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb) ret = nf_tables_commit_audit_alloc(&adl, trans->ctx.table); if (ret) { nf_tables_commit_chain_prepare_cancel(net); + nf_tables_commit_free(adl); return ret; } if (trans->msg_type == NFT_MSG_NEWRULE || @@ -8517,6 +8528,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb) ret = nf_tables_commit_chain_prepare(net, chain); if (ret < 0) { nf_tables_commit_chain_prepare_cancel(net); + nf_tables_commit_free(adl); return ret; } }
In nf_tables_commit, if nf_tables_commit_audit_alloc fails, it does not free the adp variable. Fix this by freeing the linked list with head adl. backtrace: kmalloc include/linux/slab.h:591 [inline] kzalloc include/linux/slab.h:721 [inline] nf_tables_commit_audit_alloc net/netfilter/nf_tables_api.c:8439 [inline] nf_tables_commit+0x16e/0x1760 net/netfilter/nf_tables_api.c:8508 nfnetlink_rcv_batch+0x512/0xa80 net/netfilter/nfnetlink.c:562 nfnetlink_rcv_skb_batch net/netfilter/nfnetlink.c:634 [inline] nfnetlink_rcv+0x1fa/0x220 net/netfilter/nfnetlink.c:652 netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline] netlink_unicast+0x2c7/0x3e0 net/netlink/af_netlink.c:1340 netlink_sendmsg+0x36b/0x6b0 net/netlink/af_netlink.c:1929 sock_sendmsg_nosec net/socket.c:702 [inline] sock_sendmsg+0x56/0x80 net/socket.c:722 Reported-by: syzbot <syzkaller@googlegroups.com> Fixes: c520292f29b8 ("audit: log nftables configuration change events once per table") Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> --- net/netfilter/nf_tables_api.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)