netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>,
	David Ahern <dsahern@gmail.com>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	syzbot <syzbot+618aacd49e8c8b8486bd@syzkaller.appspotmail.com>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>
Subject: Re: [Patch net] net_sched: add max len check for TCA_KIND
Date: Thu, 3 Oct 2019 16:45:25 -0300	[thread overview]
Message-ID: <20191003194525.GD3498@localhost.localdomain> (raw)
In-Reply-To: <20190921192434.765d7604@cakuba.netronome.com>

On Sat, Sep 21, 2019 at 07:24:34PM -0700, Jakub Kicinski wrote:
> On Wed, 18 Sep 2019 22:15:24 -0700, Cong Wang wrote:
> > On Wed, Sep 18, 2019 at 7:41 PM David Ahern <dsahern@gmail.com> wrote:
> > > On 9/18/19 5:24 PM, Cong Wang wrote:  
> > > > The TCA_KIND attribute is of NLA_STRING which does not check
> > > > the NUL char. KMSAN reported an uninit-value of TCA_KIND which
> > > > is likely caused by the lack of NUL.
> > > >
> > > > Change it to NLA_NUL_STRING and add a max len too.
> > > >
> > > > Fixes: 8b4c3cdd9dd8 ("net: sched: Add policy validation for tc attributes")  
> > >
> > > The commit referenced here did not introduce the ability to go beyond
> > > memory boundaries with string comparisons. Rather, it was not complete
> > > solution for attribute validation. I say that wrt to the fix getting
> > > propagated to the correct stable releases.  
> > 
> > I think this patch should be backported to wherever commit 8b4c3cdd9dd8
> > goes, this is why I picked it as Fixes.
> 
> Applied, queued for 4.14+, thanks!

Ahm, this breaks some user applications.

I'm getting "Attribute failed policy validation" extack error while
adding ingress qdisc on an app using libmnl, because it just doesn't
pack the null byte there if it uses mnl_attr_put_str():
https://git.netfilter.org/libmnl/tree/src/attr.c#n481
Unless it uses mnl_attr_put_strz() instead.

Though not sure who's to blame here, as one could argue that the
app should have been using the latter in the first place, but well..
it worked and produced the right results.

Ditto for 199ce850ce11 ("net_sched: add policy validation for action
attributes") on TCA_ACT_KIND.

  reply	other threads:[~2019-10-03 19:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-18 23:24 [Patch net] net_sched: add max len check for TCA_KIND Cong Wang
2019-09-19  2:41 ` David Ahern
2019-09-19  5:15   ` Cong Wang
2019-09-22  2:24     ` Jakub Kicinski
2019-10-03 19:45       ` Marcelo Ricardo Leitner [this message]
2019-10-04 22:54         ` Jakub Kicinski
2019-10-04 23:23           ` Cong Wang
2019-10-04 23:47             ` Jakub Kicinski
2019-09-19  6:40 ` Jiri Pirko

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=20191003194525.GD3498@localhost.localdomain \
    --to=marcelo.leitner@gmail.com \
    --cc=dsahern@gmail.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=syzbot+618aacd49e8c8b8486bd@syzkaller.appspotmail.com \
    --cc=xiyou.wangcong@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).