Commit 7181ebafd4306c9328fa1cd0ead69afa397ffe75
1 parent
2385eb0c5f
netfilter: fix possible removal of wrong hook
nf_unregister_net_hook() uses the nf_hook_ops fields as tuple to look up for the corresponding hook in the list. However, we may have two hooks with exactly the same configuration. This shouldn't be a problem for nftables since every new chain has an unique priv field set, but this may still cause us problems in the future, so better address this problem now by keeping a reference to the original nf_hook_ops structure to make sure we delete the right hook from nf_unregister_net_hook(). Fixes: 085db2c04557 ("netfilter: Per network namespace netfilter hooks.") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Showing 1 changed file with 19 additions and 22 deletions Side-by-side Diff
net/netfilter/core.c
... | ... | @@ -78,26 +78,27 @@ |
78 | 78 | return nf_hook_list; |
79 | 79 | } |
80 | 80 | |
81 | +struct nf_hook_entry { | |
82 | + const struct nf_hook_ops *orig_ops; | |
83 | + struct nf_hook_ops ops; | |
84 | +}; | |
85 | + | |
81 | 86 | int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg) |
82 | 87 | { |
83 | 88 | struct list_head *nf_hook_list; |
84 | - struct nf_hook_ops *elem, *new; | |
89 | + struct nf_hook_entry *entry; | |
90 | + struct nf_hook_ops *elem; | |
85 | 91 | |
86 | - new = kzalloc(sizeof(*new), GFP_KERNEL); | |
87 | - if (!new) | |
92 | + entry = kmalloc(sizeof(*entry), GFP_KERNEL); | |
93 | + if (!entry) | |
88 | 94 | return -ENOMEM; |
89 | 95 | |
90 | - new->hook = reg->hook; | |
91 | - new->dev = reg->dev; | |
92 | - new->owner = reg->owner; | |
93 | - new->priv = reg->priv; | |
94 | - new->pf = reg->pf; | |
95 | - new->hooknum = reg->hooknum; | |
96 | - new->priority = reg->priority; | |
96 | + entry->orig_ops = reg; | |
97 | + entry->ops = *reg; | |
97 | 98 | |
98 | 99 | nf_hook_list = find_nf_hook_list(net, reg); |
99 | 100 | if (!nf_hook_list) { |
100 | - kfree(new); | |
101 | + kfree(entry); | |
101 | 102 | return -ENOENT; |
102 | 103 | } |
103 | 104 | |
... | ... | @@ -106,7 +107,7 @@ |
106 | 107 | if (reg->priority < elem->priority) |
107 | 108 | break; |
108 | 109 | } |
109 | - list_add_rcu(&new->list, elem->list.prev); | |
110 | + list_add_rcu(&entry->ops.list, elem->list.prev); | |
110 | 111 | mutex_unlock(&nf_hook_mutex); |
111 | 112 | #ifdef CONFIG_NETFILTER_INGRESS |
112 | 113 | if (reg->pf == NFPROTO_NETDEV && reg->hooknum == NF_NETDEV_INGRESS) |
... | ... | @@ -122,6 +123,7 @@ |
122 | 123 | void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg) |
123 | 124 | { |
124 | 125 | struct list_head *nf_hook_list; |
126 | + struct nf_hook_entry *entry; | |
125 | 127 | struct nf_hook_ops *elem; |
126 | 128 | |
127 | 129 | nf_hook_list = find_nf_hook_list(net, reg); |
... | ... | @@ -130,14 +132,9 @@ |
130 | 132 | |
131 | 133 | mutex_lock(&nf_hook_mutex); |
132 | 134 | list_for_each_entry(elem, nf_hook_list, list) { |
133 | - if ((reg->hook == elem->hook) && | |
134 | - (reg->dev == elem->dev) && | |
135 | - (reg->owner == elem->owner) && | |
136 | - (reg->priv == elem->priv) && | |
137 | - (reg->pf == elem->pf) && | |
138 | - (reg->hooknum == elem->hooknum) && | |
139 | - (reg->priority == elem->priority)) { | |
140 | - list_del_rcu(&elem->list); | |
135 | + entry = container_of(elem, struct nf_hook_entry, ops); | |
136 | + if (entry->orig_ops == reg) { | |
137 | + list_del_rcu(&entry->ops.list); | |
141 | 138 | break; |
142 | 139 | } |
143 | 140 | } |
... | ... | @@ -154,8 +151,8 @@ |
154 | 151 | static_key_slow_dec(&nf_hooks_needed[reg->pf][reg->hooknum]); |
155 | 152 | #endif |
156 | 153 | synchronize_net(); |
157 | - nf_queue_nf_hook_drop(net, elem); | |
158 | - kfree(elem); | |
154 | + nf_queue_nf_hook_drop(net, &entry->ops); | |
155 | + kfree(entry); | |
159 | 156 | } |
160 | 157 | EXPORT_SYMBOL(nf_unregister_net_hook); |
161 | 158 |