Re: [PATCH] netlink: add buffer boundary checking

From: Mark Salyzyn
Date: Thu Jul 23 2020 - 16:13:23 EST


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