Florian Westphal <f...@strlen.de> writes:
> 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.
Sort of. At the same time we fundamentally need a per namespace
variable to ask if things were initialized. So using the address as
that variable changes the equation not at all, and keeps the code
simpler.
>> 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;
> }
That does not work because the real question were the things this piece
of code cares about initialized.
> Another alternative is this patch (not even compile tested, just to
> illustrate the idea):
Very much no.
I believe that changing net_generic to return a value so that
nfqnl_nf_hook_drop can detect if the timing is wrong is an error.
I also don't think your changes to net_generic will work as the number
of pointers should have been allocted properly from the start.
Your proposed change is just papering over the problem and fixing it at
the wrong level. (The obvious level yes) but still the wrong level.
> What do you think?
Eric
