netdev - Re: [PATCH 0/6] stricter netlink validation

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:   Thu, 04 Apr 2019 22:20:20 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     David Miller <davem@...emloft.net>
Cc:     linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        pablo@...filter.org, dsa@...ulusnetworks.com
Subject: Re: [PATCH 0/6] stricter netlink validation

On Thu, 2019-04-04 at 10:28 -0700, David Miller wrote:
> From: Johannes Berg <johannes@...solutions.net>
> Date: Thu,  4 Apr 2019 08:54:02 +0200
> 
> > Here's a version that has passed build testing ;-)
> 
> :-)

Actually it passed more than that - I did test the nl80211 bits etc.,
but I hadn't build-tested everything before so some missing function
renames were caught by the full build testing.

> I really like the approach taken here, and done in such a way that
> new attributes added get strict checking by default.

It's two things really

 * new commands (aka new instances of nla_parse/nlmsg_parse and friends)
   --> strict checking for everything, including existing attributes
       because we reason that you're writing some new userspace code,
       and even if that might use some existing functionality, which
       might even be wrong, you're going to fix it here

 * new attributes on existing commands (in the policy)
   --> can be set up (with the strict_start_type from patch 4) to be
       strictly checked

> I'll let David Ahern et al. have time to review this.

Sure.

FWIW, I wasn't really entirely sure I liked doing a cross-tree rename,
but ultimately I felt that we should discourage uses of what I now
called *_deprecated() and *_strict_deprecated() APIs, and having sort of
the "default" names do the thing we believe is right (strict checking)
helps with that - in a sort of 'social engineering' way, people will not
want to type out "_deprecated" all the time ;-)

I do realize that this may be a bit controversial and am certainly open
to other suggestions on this.

Similarly, I engineered the generic netlink stuff in a way that adding
non-strict behaviour needs extra work, so that hopefully new stuff will
not do that extra work.

Also, both of these are then easier to see in reviews, since you can see
"deprecated" in the function names, or "DONT_VALIDATE" in the generic
netlink things.

johannes