linux-kernel - Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function

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]

Message-ID: <20201214114831.GE2809@kadam>
Date:   Mon, 14 Dec 2020 14:48:31 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     UNGLinuxDriver@...rochip.com, vladimir.oltean@....com,
        claudiu.manoil@....com, alexandre.belloni@...tlin.com,
        davem@...emloft.net, kuba@...nel.org, andrew@...n.ch,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error
 handling path of the probe function

On Sun, Dec 13, 2020 at 12:48:38PM +0100, Christophe JAILLET wrote:
> In case of error after calling 'ocelot_init()', it must be undone by a
> corresponding 'ocelot_deinit()' call, as already done in the remove
> function.
> 

This changes the behavior slightly in another way as well, but it's
probably a bug fix.

drivers/net/ethernet/mscc/ocelot_vsc7514.c
  1250          ports = of_get_child_by_name(np, "ethernet-ports");
  1251          if (!ports) {
  1252                  dev_err(ocelot->dev, "no ethernet-ports child node found\n");
  1253                  return -ENODEV;
  1254          }
  1255  
  1256          ocelot->num_phys_ports = of_get_child_count(ports);
  1257          ocelot->num_flooding_pgids = 1;
  1258  
  1259          ocelot->vcap = vsc7514_vcap_props;
  1260          ocelot->inj_prefix = OCELOT_TAG_PREFIX_NONE;
  1261          ocelot->xtr_prefix = OCELOT_TAG_PREFIX_NONE;
  1262          ocelot->npi = -1;
  1263  
  1264          err = ocelot_init(ocelot);
  1265          if (err)
  1266                  goto out_put_ports;
  1267  
  1268          err = mscc_ocelot_init_ports(pdev, ports);
  1269          if (err)
  1270                  goto out_put_ports;
  1271  
  1272          if (ocelot->ptp) {
  1273                  err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
  1274                  if (err) {
  1275                          dev_err(ocelot->dev,
  1276                                  "Timestamp initialization failed\n");
  1277                          ocelot->ptp = 0;
  1278                  }

In the original code, if ocelot_init_timestamp() failed we returned
a negative error code but now we return success.  This probably is what
the original authors intended, though.

  1279          }
  1280  
  1281          register_netdevice_notifier(&ocelot_netdevice_nb);
  1282          register_switchdev_notifier(&ocelot_switchdev_nb);
  1283          register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
  1284  
  1285          dev_info(&pdev->dev, "Ocelot switch probed\n");
  1286  
  1287  out_put_ports:
  1288          of_node_put(ports);
  1289          return err;
  1290  }

regards,
dan carpenter