netdev - net: sxgbe: issue with incorrect masking / case values in switch statements

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: <e3f20a0f-058b-9f57-57aa-91455b2f34a5@canonical.com>
Date:   Fri, 9 Apr 2021 14:27:31 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Siva Reddy <siva.kallam@...sung.com>,
        Byungho An <bh74.an@...sung.com>
Cc:     Vipul Pandya <vipul.pandya@...sung.com>,
        Girish K S <ks.giri@...sung.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: net: sxgbe: issue with incorrect masking / case values in switch
 statements

Hi,

Static analysis with Coverity has found an issue with the sxgbe driver
in drivers/net/ethernet/samsung/sxgbe/sxgbe_mtl.c from the following commit:

commit 1edb9ca69e8a7988900fc0283e10550b5592164d
Author: Siva Reddy <siva.kallam@...sung.com>
Date:   Tue Mar 25 12:10:54 2014 -0700

    net: sxgbe: add basic framework for Samsung 10Gb ethernet driver


The analysis is as follows:

 20 static void sxgbe_mtl_init(void __iomem *ioaddr, unsigned int etsalg,
 21                           unsigned int raa)
 22 {
 23        u32 reg_val;
 24
 25        reg_val = readl(ioaddr + SXGBE_MTL_OP_MODE_REG);
 26        reg_val &= ETS_RST;
 27
 28        /* ETS Algorith */
 29        switch (etsalg & SXGBE_MTL_OPMODE_ESTMASK) {

Logically dead code (DEADCODE)

 30        case ETS_WRR:
 31                reg_val &= ETS_WRR;
 32                break;

Logically dead code (DEADCODE)

 33        case ETS_WFQ:
 34                reg_val |= ETS_WFQ;
 35                break;

Logically dead code (DEADCODE)

 36        case ETS_DWRR:
 37                reg_val |= ETS_DWRR;
 38                break;
 39        }

Above:
  SXGBE_MTL_OPMODE_ESTMASK is 0x3
  ETS_WRR is 0xFFFFFFFB
  ETS_WFQ is 0x00000020
  ETS_DWRR is 0x00000040

so none of the case statements are ever reachable because of the mask
being used.


 40        writel(reg_val, ioaddr + SXGBE_MTL_OP_MODE_REG);
 41
 42        switch (raa & SXGBE_MTL_OPMODE_RAAMASK) {

Logically dead code (DEADCODE)

 43        case RAA_SP:
 44                reg_val &= RAA_SP;
 45                break;

Logically dead code (DEADCODE)

 46        case RAA_WSP:
 47                reg_val |= RAA_WSP;
 48                break;
 49        }
 50        writel(reg_val, ioaddr + SXGBE_MTL_OP_MODE_REG);
 51}

And above,
  SXGBE_MTL_OPMODE_RAAMASK is 0x1
  RAA_SP is 0xFFFFFFFB
  RAA_WSP is 0x00000004

again, none of the case statements are ever reachable because of the
mask being used.

Not sure of how this was meant to work, so I can't determine a fix,
hence I'm reporting this issue.

Colin