linux-kernel - Re: net/dsa/tag_8021q.c:109 dsa_8021q_restore_pvid() error: uninitialized symbol 'pvid'.

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, 27 May 2020 19:43:29 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     kbuild@...ts.01.org, kbuild test robot <lkp@...el.com>,
        kbuild-all@...ts.01.org, lkml <linux-kernel@...r.kernel.org>,
        Andrew Lunn <andrew@...n.ch>
Subject: Re: net/dsa/tag_8021q.c:109 dsa_8021q_restore_pvid() error:
 uninitialized symbol 'pvid'.

Hi Dan,

On Tue, 26 May 2020 at 17:55, Dan Carpenter <dan.carpenter@...cle.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   9cb1fd0efd195590b828b9b865421ad345a4a145
> commit: c80ed84e76886487703bf04b38ce10e92e2d6e26 net: dsa: tag_8021q: Fix dsa_8021q_restore_pvid for an absent pvid
> date:   6 months ago
> config: x86_64-randconfig-m001-20200526 (attached as .config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
>
> smatch warnings:
> net/dsa/tag_8021q.c:109 dsa_8021q_restore_pvid() error: uninitialized symbol 'pvid'.
>
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c80ed84e76886487703bf04b38ce10e92e2d6e26
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git remote update linus
> git checkout c80ed84e76886487703bf04b38ce10e92e2d6e26
> vim +/pvid +109 net/dsa/tag_8021q.c
>
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30   96  static int dsa_8021q_restore_pvid(struct dsa_switch *ds, int port)
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30   97  {
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30   98          struct bridge_vlan_info vinfo;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30   99          struct net_device *slave;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  100          u16 pvid;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  101          int err;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  102
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  103          if (!dsa_is_user_port(ds, port))
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  104                  return 0;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  105
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  106          slave = ds->ports[port].slave;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  107
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  108          err = br_vlan_get_pvid(slave, &pvid);
> c80ed84e768864 Vladimir Oltean 2019-11-16 @109          if (!pvid || err < 0)
>                                                             ^^^^^^^^^^^^^^^^
> I don't know why this warning is only showing up now in May 2020...
> Anyway, we could solve the problem by checking for errors first:
>
>         if (err < 0 || !pvid) {
>
> The UBSan tool can detect uninitialized values used at runtime so it
> might generate a splat as well.  Other than that, obviously it is
> harmless.
>
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  110                  /* There is no pvid on the bridge for this port, which is
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  111                   * perfectly valid. Nothing to restore, bye-bye!
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  112                   */
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  113                  return 0;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  114
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  115          err = br_vlan_get_info(slave, pvid, &vinfo);
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  116          if (err < 0) {
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  117                  dev_err(ds->dev, "Couldn't determine PVID attributes\n");
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  118                  return err;
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  119          }
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  120
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  121          return dsa_port_vid_add(&ds->ports[port], pvid, vinfo.flags);
> 5f33183b7fdfeb Vladimir Oltean 2019-08-30  122  }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

For net-next I just sent a patch that removes this function altogether:
https://patchwork.ozlabs.org/project/netdev/patch/20200527164134.1081548-1-olteanv@gmail.com/
Do you think a patch for the net tree is still necessary, that
reorders the checks? Will you send that patch or should I?

Thanks,
-Vladimir