linux-kernel - Potential issue setting vid_regs in Rockchip AXI PCIe endpoint controller driver

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

Message-ID: <cd70cd13-5b0e-ea14-f7b1-fb8866c4dbbf@canonical.com>
Date:   Wed, 9 Oct 2019 11:18:56 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Shawn Lin <shawn.lin@...k-chips.com>,
        linux-rockchip@...ts.infradead.org,
        Bjorn Helgaas <bhelgaas@...gle.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Potential issue setting vid_regs in Rockchip AXI PCIe endpoint
 controller driver

Hi,

Static analysis with Coverity has detected a potential issue in the
Rockchip AXI PCIe endpoint controller driver.  The issue is in
drivers/pci/controller/pcie-rockchip-ep.c, function
rockchip_pcie_ep_write_header:

The coverity report is as follows:

124 static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn,
125                                         struct pci_epf_header *hdr)
126 {
127        struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
128        struct rockchip_pcie *rockchip = &ep->rockchip;
129
130        /* All functions share the same vendor ID with function 0 */
131        if (fn == 0) {
132                u32 vid_regs = (hdr->vendorid & GENMASK(15, 0)) |

   CID 12883 (#1 of 1): Operands don't affect result

(CONSTANT_EXPRESSION_RESULT) result_independent_of_operands:

hdr->subsys_vendor_id & (4294901760UL /* ~0UL - (1UL << 16) + 1 & (~0UL
>> 64 - 1 - 31) */) is always 0 regardless of the values of its
operands. This occurs as a value.

133                               (hdr->subsys_vendor_id & GENMASK(31,
16)) << 16;

The expression (hdr->subsys_vendor_id & GENMASK(31, 16)) << 16 always
results in zero. Since the GENMASK gets the top 16 bits of
hdr->subsys_vendor_id then it looks like the 16 bit left shift is not
required, but I don't know if that is a correct assumption or not.

Colin