All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: edumazet@google.com
Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com,
	syzkaller@googlegroups.com, maximmi@mellanox.com
Subject: Re: [PATCH net] ipv6/addrconf: fix potential NULL deref in inet6_set_link_af()
Date: Thu, 06 Feb 2020 14:13:00 +0100 (CET)	[thread overview]
Message-ID: <20200206.141300.1752448469848126511.davem@davemloft.net> (raw)
In-Reply-To: <20200205165544.242623-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Wed,  5 Feb 2020 08:55:44 -0800

> __in6_dev_get(dev) called from inet6_set_link_af() can return NULL.
> 
> The needed check has been recently removed, let's add it back.

I am having trouble understanding this one.

When we have a do_setlink operation the flow is that we first validate
the AFs and then invoke setlink operations after that validation.

do_setlink() {
 ..
	err = validate_linkmsg(dev, tb);
	if (err < 0)
		return err;
 ..
	if (tb[IFLA_AF_SPEC]) {
 ...
			err = af_ops->set_link_af(dev, af);
			if (err < 0) {
				rcu_read_unlock();
				goto errout;
			}

By definition, we only get to ->set_link_af() if there is an
IFLA_AF_SPEC nested attribute and if we look at the validation
performed by validate_linkmsg() it goes:

	if (tb[IFLA_AF_SPEC]) {
 ...
			if (af_ops->validate_link_af) {
				err = af_ops->validate_link_af(dev, af);
 ...

And validate_link_af in net/ipv6/addrconf.c clearly does the
following:

static int inet6_validate_link_af(const struct net_device *dev,
				  const struct nlattr *nla)
 ...
	if (dev) {
		idev = __in6_dev_get(dev);
		if (!idev)
			return -EAFNOSUPPORT;
	}
 ...

It checks the idev and makes sure it is not-NULL.

I therefore cannot find a path by which we arrive at inet6_set_link_af
with a NULL idev.  The above validation code should trap it.

Please explain.

Thank you.

  reply	other threads:[~2020-02-06 13:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05 16:55 [PATCH net] ipv6/addrconf: fix potential NULL deref in inet6_set_link_af() Eric Dumazet
2020-02-06 13:13 ` David Miller [this message]
2020-02-06 15:18   ` Eric Dumazet
2020-02-07  2:50     ` Eric Dumazet
2020-02-07  9:36       ` David Miller

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=20200206.141300.1752448469848126511.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=maximmi@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=syzkaller@googlegroups.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 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.