netdev - Re: [PATCH net-next v2 00/10] genetlink: support per-command policy dump

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]

Date:   Fri, 2 Oct 2020 13:50:59 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, andrew@...n.ch,
        jiri@...nulli.us, mkubecek@...e.cz, dsahern@...nel.org,
        pablo@...filter.org
Subject: Re: [PATCH net-next v2 00/10] genetlink: support per-command policy
 dump

On Fri, 02 Oct 2020 22:27:19 +0200 Johannes Berg wrote:
> On Fri, 2020-10-02 at 08:09 -0700, Jakub Kicinski wrote:
> > On Fri, 02 Oct 2020 17:04:11 +0200 Johannes Berg wrote:  
> > > > > Yeah, that'd work. I'd probably wonder if we shouldn't do
> > > > > 
> > > > > [OP_POLICY]
> > > > >   [OP] -> (u32, u32)
> > > > > 
> > > > > in a struct with two u32's, since that's quite a bit more compact.    
> > > > 
> > > > What do we do if the op doesn't have a dump or do callback?
> > > > 0 is a valid policy ID, sadly :(    
> > > 
> > > Hm, good point. We could do -1 since that can't ever be reached though.
> > > 
> > > But compactness isn't really that necessary here anyway, so ...  
> > 
> > Cool, sounds like a plan.
> > 
> > This series should be good to merge, then.  
> 
> So I'm having second thoughts on this now :)
> 
> If you ask me to split the policy dump to do/dump, like we discussed
> above, then what you did here for "retrieve a single policy" doesn't
> really make any sense? Because you'd be able to do that properly only
> for do, or you need my patches to get both?
> 
> Perhaps it would make sense if you removed patch 10 from your set, and
> we add it back after my patches?
> 
> Or I could submit my patches right after yours, but that leaves the code
> between the commits doing something weird, in that it would only give
> you the policies but no indication of which is for do/dump? Obviously
> today it'd only be one, but still, from a uAPI perspective.

My thinking was that until kernel actually start using separate dump
policies user space can assume policy 0 is relevant. But yeah, merging
your changes first would probably be best.

> I guess it doesn't matter too much though, we get to the state that we
> want to be in, just the intermediate steps won't necessarily make much
> sense.
> 
> For now I'll respin my patches so we see how the above do/dump
> separating looks.

I, OTOH, am having second thoughts about not implementing separate
policies for dump right away, since Michal said he'll need them soon :)

Any ideas on how to do that cleanly? At some point it will make sense
to have dumps and doits in separate structures, as you said earlier,
but can we have "small" and "full" ops for both? That seems like too
much :/