Commit b7047a1c886386b10a103b4fea26678db8b57832

Authored by Patrick McHardy
Committed by David S. Miller
1 parent 019f692ea7

[NETFILTER]: nfnetlink_log: fix EPERM when binding/unbinding and instance 0 exists

When binding or unbinding to an address family, the res_id is usually set
to zero. When logging instance 0 already exists and is owned by a different
process, this makes nfunl_recv_config return -EPERM without performing
the bind operation.

Since no operation on the foreign logging instance itself was requested,
this is incorrect. Move bind/unbind commands before the queue instance
permissions checks.

Also remove an incorrect comment.

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 1 changed file with 16 additions and 14 deletions Side-by-side Diff

net/netfilter/nfnetlink_log.c
... ... @@ -702,20 +702,30 @@
702 702 struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
703 703 u_int16_t group_num = ntohs(nfmsg->res_id);
704 704 struct nfulnl_instance *inst;
  705 + struct nfulnl_msg_config_cmd *cmd = NULL;
705 706 int ret = 0;
706 707  
  708 + if (nfula[NFULA_CFG_CMD]) {
  709 + u_int8_t pf = nfmsg->nfgen_family;
  710 + cmd = nla_data(nfula[NFULA_CFG_CMD]);
  711 +
  712 + /* Commands without queue context */
  713 + switch (cmd->command) {
  714 + case NFULNL_CFG_CMD_PF_BIND:
  715 + return nf_log_register(pf, &nfulnl_logger);
  716 + case NFULNL_CFG_CMD_PF_UNBIND:
  717 + nf_log_unregister_pf(pf);
  718 + return 0;
  719 + }
  720 + }
  721 +
707 722 inst = instance_lookup_get(group_num);
708 723 if (inst && inst->peer_pid != NETLINK_CB(skb).pid) {
709 724 ret = -EPERM;
710 725 goto out_put;
711 726 }
712 727  
713   - if (nfula[NFULA_CFG_CMD]) {
714   - u_int8_t pf = nfmsg->nfgen_family;
715   - struct nfulnl_msg_config_cmd *cmd;
716   -
717   - cmd = nla_data(nfula[NFULA_CFG_CMD]);
718   -
  728 + if (cmd != NULL) {
719 729 switch (cmd->command) {
720 730 case NFULNL_CFG_CMD_BIND:
721 731 if (inst) {
... ... @@ -738,14 +748,6 @@
738 748  
739 749 instance_destroy(inst);
740 750 goto out;
741   - case NFULNL_CFG_CMD_PF_BIND:
742   - ret = nf_log_register(pf, &nfulnl_logger);
743   - break;
744   - case NFULNL_CFG_CMD_PF_UNBIND:
745   - /* This is a bug and a feature. We cannot unregister
746   - * other handlers, like nfnetlink_inst can */
747   - nf_log_unregister_pf(pf);
748   - break;
749 751 default:
750 752 ret = -ENOTSUPP;
751 753 break;