netdev - Re: [PATCH] netlink: limit recursion depth in policy 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]

Message-ID: <e86c0165d9c6966eaa2f653674ab16485b20da47.camel@sipsolutions.net>
Date:   Tue, 30 Apr 2019 08:58:10 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] netlink: limit recursion depth in policy validation

On Mon, 2019-04-29 at 23:08 -0400, David Miller wrote:
> From: Johannes Berg <johannes@...solutions.net>
> Date: Fri, 26 Apr 2019 14:13:46 +0200
> 
> > From: Johannes Berg <johannes.berg@...el.com>
> > 
> > Now that we have nested policies, we can theoretically
> > recurse forever parsing attributes if a (sub-)policy
> > refers back to a higher level one. This is a situation
> > that has happened in nl80211, and we've avoided it there
> > by not linking it.
> > 
> > Add some code to netlink parsing to limit recursion depth,
> > allowing us to safely change nl80211 to actually link the
> > nested policy, which in turn allows some code cleanups.
> > 
> > Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> 
> This doesn't apply cleanly to 'net', is there some dependency I am
> unaware of or is this because of a recent mac80211 pull into my tree?

Sorry, I should've made it clear that this applies on top of the
patchset to expose netlink policies to userspace; due to all the
overlaping changes in lib/nlattr.c that seemed like the best solution to
me.

There's no real need to have this safeguard right now in net as nothing
there actually specifies a recursive policy (I knew about this issue and
explicitly made nl80211 *not* have a recursive policy as you can see in
this patch changing that), so I figured net-next was fine.

I'll rebase this on net-next along with the policy export (fixing the
full signed range thing) and resend as a combined set to clarify the
dependencies.

If you prefer to have the safeguard in net even if it shouldn't be
needed now, let me know and I'll make a version that applies there, but
note that will invariably cause conflicts with all the other changes in
lib/nlattr.c.

johannes