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] [day] [month] [year] [list]
Message-ID: <9fa13591-38dc-5dbf-5271-5c2bd364f16f@linux.intel.com>
Date:   Mon, 31 Jul 2023 16:02:09 +0200
From:   Alexander Antonov <alexander.antonov@...ux.intel.com>
To:     "Colin King (gmail)" <colin.i.king@...il.com>
Cc:     linux-perf-users@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: potential return of uninitialized variable ret in function
 skx_upi_topology_cb

Hi Colin,
Thank you so much for reporting the issue. Seems fix wasn't picked up 
before, I will resend it.

Thank you


On 7/27/2023 3:55 PM, Colin King (gmail) wrote:
> Hi,
>
> Static analysis with cppcheck has detected a potential return of an 
> uninitialized variable in function skx_upi_topology_cb. The issue was 
> introduced with commit:
>
> commit 4cfce57fa42d277497cd2c425021312eae2f223c
> Author: Alexander Antonov <alexander.antonov@...ux.intel.com>
> Date:   Thu Nov 17 12:28:28 2022 +0000
>
>     perf/x86/intel/uncore: Enable UPI topology discovery for Skylake 
> Server
>
> static int skx_upi_topology_cb(struct intel_uncore_type *type, int 
> segment,
>                                 int die, u64 cpu_bus_msr)
> {
>         int idx, ret;
>
> ^^ ret is not initialized
>
>         struct intel_uncore_topology *upi;
>         unsigned int devfn;
>         struct pci_dev *dev = NULL;
>         u8 bus = cpu_bus_msr >> (3 * BUS_NUM_STRIDE);
>
>         for (idx = 0; idx < type->num_boxes; idx++) {
>                 upi = &type->topology[die][idx];
>                 devfn = PCI_DEVFN(SKX_UPI_REGS_ADDR_DEVICE_LINK0 + idx,
>                                   SKX_UPI_REGS_ADDR_FUNCTION);
>                 dev = pci_get_domain_bus_and_slot(segment, bus, devfn);
>
> ^^ dev may be null, so ret is never assigned
>
>                 if (dev) {
>                         ret = upi_fill_topology(dev, upi, idx);
>                         if (ret)
>                                 break;
>                 }
>         }
>
>         pci_dev_put(dev);
>         return ret;
> }
>
> I suspect this probably is very unlikely, but it would be useful to 
> have ret initialized to some value to avoid garbage being returned. 
> Not sure what the best value is to set as as default in this corner case.
>
> Colin

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ