netdev - Re: [RFC 4/5] netlink: prepare validate extack setting for recursion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1537350299.10305.33.camel@sipsolutions.net>
Date:   Wed, 19 Sep 2018 11:44:59 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [RFC 4/5] netlink: prepare validate extack setting for recursion

On Wed, 2018-09-19 at 11:28 +0200, Jiri Benc wrote:

> > It might be possible to do this differently, in theory, but all the ways
> > I've tried to come up with so far made the code vastly more complex.
> 
> Wouldn't still make sense to store the flag in the struct
> netlink_ext_ack, though?

Does it make sense to store a flag there that only has a single,
localized, user? I'd say no.

> The way the parameters are passed around in
> this patch looks ugly. 

Yeah, it's not the best in some ways.

I considered having a cleared new extack on the stack in the outer-most
call (nla_parse/nla_validate), and then we can "set if not already set",
and at the end unconditionally copy/set to the real one... But then I
either need to duplicate that code in both, or pass another argument to
nla_validate_parse() anyway to indicate whether to do this or not (since
the inner calls to it shouldn't, since that would defeat the purpose),
or add another level of function call indirection?

I'm not really sure any of these are better ... and more complex, in
some ways, since we have to copy all the data around.

> And adding more users of the flag later (there
> may be other cases when we want the earlier messages to be preserved)
> would mean adding parameters all around, while the flag in the struct
> would be readily available.

I can't really think of any other users of such a thing?

johannes