netdev - Re: [Patch net-next 1/3] ipv4: fail early when creating netdev named all or default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 23 Jul 2014 23:00:54 -0400
From:	Andy Gospodarek <gospo@...ulusnetworks.com>
To:	Cong Wang <xiyou.wangcong@...il.com>,
	Florian Fainelli <f.fainelli@...il.com>
Cc:	netdev <netdev@...r.kernel.org>, stephane.chazelas@...il.com,
	David Miller <davem@...emloft.net>
Subject: Re: [Patch net-next 1/3] ipv4: fail early when creating netdev named
 all or default

On Wed, Jul 23, 2014 at 06:05:07PM -0700, Florian Fainelli wrote:
> 2014-07-23 15:17 GMT-07:00 Cong Wang <xiyou.wangcong@...il.com>:
> > We create a proc dir for each network device, this will cause
> > conflicts when the devices have name "all" or "default".
> >
> > Rather than emitting an ugly kernel warning, we could just
> > fail earlier by checking the device name.
> >
> > Reported-by: Stephane Chazelas <stephane.chazelas@...il.com>
> > Cc: "David S. Miller" <davem@...emloft.net>
> > Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
> > ---
> 
> [snip]
> 
> >
> > -static void devinet_sysctl_register(struct in_device *idev)
> > +static int devinet_sysctl_register(struct in_device *idev)
> >  {
> > -       neigh_sysctl_register(idev->dev, idev->arp_parms, NULL);
> > -       __devinet_sysctl_register(dev_net(idev->dev), idev->dev->name,
> > +       int err;
> > +
> > +       if (!strcmp(idev->dev->name, "default") ||
> > +           !strcmp(idev->dev->name, "all"))
> > +               return -EINVAL;
> 
> Since this exact same check is done in the ipv6 counterpart, how about
> moving this to a helper function like: sysctl_dev_name_is_allowed()
> such that you centralize where the naming policy is enforced? In case
> we need to blacklist some other interface names, there should be one
> place to update.

Agree that a common function would be useful.

There also might be cases where one would want to impose more limits
than just what is limited based on /proc, so this may be something to consider
in core networking code.

> 
> > +
> > +       err = neigh_sysctl_register(idev->dev, idev->arp_parms, NULL);
> > +       if (err)
> > +               return err;
> > +       err = __devinet_sysctl_register(dev_net(idev->dev), idev->dev->name,
> >                                         &idev->cnf);
> > +       if (err)
> > +               neigh_sysctl_unregister(idev->arp_parms);
> > +       return err;
> >  }
> >
> >  static void devinet_sysctl_unregister(struct in_device *idev)
> > --
> > 1.8.3.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> -- 
> Florian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html