[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f3d2eedc-23eb-0b45-4c15-15b887e05164@android.com>
Date: Thu, 23 Jul 2020 13:13:18 -0700
From: Mark Salyzyn <salyzyn@...roid.com>
To: Eric Dumazet <eric.dumazet@...il.com>, linux-kernel@...r.kernel.org
Cc: kernel-team@...roid.com, netdev@...r.kernel.org,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH] netlink: add buffer boundary checking
On 7/23/20 12:35 PM, Eric Dumazet wrote:
> I believe this will hide bugs, that syzbot was able to catch.
syzbot failed to catch the problem because of padding u8, u16 and u32
were all immune because they would go out of bounds into a padded buffer :-(
On 7/23/20 12:19 PM, David Miller wrote:
> Now it is going to be expensive to move several small attributes,
> which is common. And there's a multiplier when dumping, for example,
> thousands of networking devices, routes, or whatever, and all of their
> attributes in a dump.
So it _is_ performance critical (!)
> If you can document actual out of bounds accesses, let's fix them. Usually
> contextually the attribute type and size has been validated by the time we
> execute these accessors.
A PoC was written for nl80211_tdls_mgmt supplied no bytes for
NL80211_ATTR_STATUS_CODE and instrumented code could report back OOB.
I was initially considering pushback because padding bytes were being
read and considered it harmless. Best way to find out if it is really a
problem probably was to ask, but as Linus said once 'show me the code'
and that is just as effective in the asking ;->
My Gut remains to respond WAI unless you'all reading padding is 'bad'
-- Mark
Powered by blists - more mailing lists