All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Salyzyn <salyzyn@android.com>
To: Eric Dumazet <eric.dumazet@gmail.com>, linux-kernel@vger.kernel.org
Cc: kernel-team@android.com, netdev@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Thomas Graf <tgraf@suug.ch>
Subject: Re: [PATCH] netlink: add buffer boundary checking
Date: Thu, 23 Jul 2020 13:13:18 -0700	[thread overview]
Message-ID: <f3d2eedc-23eb-0b45-4c15-15b887e05164@android.com> (raw)
In-Reply-To: <09cd1829-8e41-bef5-ba5e-1c446c166778@gmail.com>

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


  reply	other threads:[~2020-07-23 20:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23 18:21 [PATCH] netlink: add buffer boundary checking Mark Salyzyn
2020-07-23 19:19 ` David Miller
2020-07-23 19:35 ` Eric Dumazet
2020-07-23 20:13   ` Mark Salyzyn [this message]
2020-07-24 21:14   ` Jacob Keller
2020-07-24 21:45     ` Mark Salyzyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f3d2eedc-23eb-0b45-4c15-15b887e05164@android.com \
    --to=salyzyn@android.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=kernel-team@android.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.