Eric W. Biederman <ebied...@xmission.com> wrote:
> > AFAICS no other callers do something similar, but yes,
> > we'd need this all over the place if there are others.
> >
> > Maybe we need a saner fix, e.g. by adding bounds check to net_generic(),
> > and making sure that net_generic() returns non-NULL only if the per
> > netns memory was allocated properly.
>
> As a first approximiation I am thinking we should fix by making
> nf_queue_register_handler a per netns operation.
We can do that but then we'd end up storing the very same address
for each namespace which I think isn't nice either.
> Either that or give nfnetlink_queue it's own dedicated place in
> struct net for struct nfnl_queue_net.
That would work too, but then I don't see why that is preferrable
to this proposed patch. We could add a helper for this so that
we have something like
static bool netns_was_inited(const struct net *n)
{
return net->list.next != NULL;
}
Another alternative is this patch (not even compile tested, just to
illustrate the idea):
diff --git a/include/net/netns/generic.h b/include/net/netns/generic.h
--- a/include/net/netns/generic.h
+++ b/include/net/netns/generic.h
@@ -38,7 +38,11 @@ static inline void *net_generic(const struct net *net, int
id)
rcu_read_lock();
ng = rcu_dereference(net->gen);
- ptr = ng->ptr[id - 1];
+
+ if (ng->len < id)
+ ptr = ng->ptr[id - 1];
+ else
+ ptr = NULL;
rcu_read_unlock();
return ptr;
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -111,6 +111,7 @@ static int ops_init(const struct pernet_operations *ops,
struct net *net)
return 0;
cleanup:
+ net_assign_generic(net, *ops->id, NULL);
kfree(data);
out:
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -927,6 +927,9 @@ static void nfqnl_nf_hook_drop(struct net *net, struct
nf_hook_ops *hook)
struct nfnl_queue_net *q = nfnl_queue_pernet(net);
int i;
+ if (!q)
+ return;
+
rcu_read_lock();
for (i = 0; i < INSTANCE_BUCKETS; i++) {
struct nfqnl_instance *inst;
What do you think?
