netdev - Re: [RFC net-next 9/9] genetlink: allow dumping command-specific policy

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: <3de28a3b54737ffd5c7d9944acc0614745242a30.camel@sipsolutions.net>
Date:   Thu, 01 Oct 2020 18:00:58 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, andrew@...n.ch, jiri@...nulli.us,
        mkubecek@...e.cz, dsahern@...nel.org, pablo@...filter.org
Subject: Re: [RFC net-next 9/9] genetlink: allow dumping command-specific
 policy

On Thu, 2020-10-01 at 08:41 -0700, Jakub Kicinski wrote:

> > Even if I don't have a good idea now on how to avoid the duplication, it
> > might be nicer to have a (flag) attribute here for "CTRL_ATTR_ALL_OPS"?
> 
> Hm. How would you see the dump structured? 

Yeah, that's the tricky part ... Hence why I said "I don't have a good
idea now" :)

> We need to annotate the root
> policies with the command. Right now I have:
> 
>  [ATTR_FAMILY_ID]
>  [ATTR_OP]
>  [ATTR_POLICY]
>    [policy idx]
>      [attr idx]
>        [bla]
>        [bla]
>        [bla]
> 
> But if we're dumping _all_ the policy to op mapping is actually 1:n,
> so we'd need to restructure the dump a lil' bit and have OP only
> reported on root of the policy and make it a nested array.

So today you see something like

[ATTR_FAMILY_ID]
[ATTR_POLICY]
  [policy idx, 0 = main policy]
    [bla]
    ...
  ...

I guess the most compact representation, that also preserves the most
data about sharing, would be to do something like

[ATTR_FAMILY_ID]
[ATTR_POLICY]
  [policy idx, 0 = main policy]
    [bla]
    ...
  ...
[ATTR_OP_POLICY]
  [op] = [policy idx]
  ...

This preserves all the information because it tells you which policies
actually are identical (shared), each per-op policy can have nested
policies referring to common ones, like in the ethtool case, etc.

OTOH, it's a lot trickier to implement - I haven't really come up with a
good way of doing it "generically" with the net/netlink/policy.c code.
I'm sure it can be solved, but I haven't really given it enough thought.
Perhaps by passing a "policy iterator" to netlink_policy_dump_start(),
instead of just a single policy (i.e. a function & a data ptr or so),
and then it can walk all the policies using that, assign the idxes etc.,
and dump them out in netlink_policy_dump_write()?

But then we'd still have to get the policy idx for a given policy, and
not clean up all the state when netlink_policy_dump_loop() returns
false, because you still need it for ATTR_OP_POLICY to find the idx from
the pointer?

I guess it's doable. Just seems a bit more complex. OTOH, it may be that
such complexity also completely makes sense for non-generic netlink
families anyway, I haven't looked at them much at all.

johannes