linux-kernel - potential return of uninitialized variable ret in function skx_upi_topology

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-next>] [day] [month] [year] [list]

Message-ID: <21e2ea66-9524-8e34-a2f0-8e16b9617d68@gmail.com>
Date:   Thu, 27 Jul 2023 14:55:00 +0100
From:   "Colin King (gmail)" <colin.i.king@...il.com>
To:     Alexander Antonov <alexander.antonov@...ux.intel.com>
Cc:     linux-perf-users@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: potential return of uninitialized variable ret in function
 skx_upi_topology_cb

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