From mboxrd@z Thu Jan  1 00:00:00 1970
From: Niklas Cassel <niklas.cassel@axis.com>
Subject: stmmac smatch error rx_queue_routing
Date: Mon, 22 Jan 2018 17:43:04 +0100
Message-ID: <20180122164304.GC23171@axis.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: netdev@vger.kernel.org
To: peppe.cavallaro@st.com, alexandre.torgue@st.com,
        jpinto@synopsys.com, joabreu@synopsys.com
Return-path: <netdev-owner@vger.kernel.org>
Received: from bastet.se.axis.com ([195.60.68.11]:37643 "EHLO
        bastet.se.axis.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751028AbeAVQnG (ORCPT
        <rfc822;netdev@vger.kernel.org>); Mon, 22 Jan 2018 11:43:06 -0500
Content-Disposition: inline
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hello stmmac peeps,

I found this smatch error:

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:132 dwmac4_tx_queue_routing() error:
  buffer overflow 'route_possibilities' 5 <= 254
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:133 dwmac4_tx_queue_routing() error:
  buffer overflow 'route_possibilities' 5 <= 254

Looking at the code raises some questions:


static void dwmac4_tx_queue_routing(struct mac_device_info *hw,
                                    u8 packet, u32 queue)
{
        ...
        static const struct stmmac_rx_routing route_possibilities[] = {
                { GMAC_RXQCTRL_AVCPQ_MASK, GMAC_RXQCTRL_AVCPQ_SHIFT },
                { GMAC_RXQCTRL_PTPQ_MASK, GMAC_RXQCTRL_PTPQ_SHIFT },
                { GMAC_RXQCTRL_DCBCPQ_MASK, GMAC_RXQCTRL_DCBCPQ_SHIFT },
                { GMAC_RXQCTRL_UPQ_MASK, GMAC_RXQCTRL_UPQ_SHIFT },
                { GMAC_RXQCTRL_MCBCQ_MASK, GMAC_RXQCTRL_MCBCQ_SHIFT },
        };

        value = readl(ioaddr + GMAC_RXQ_CTRL1);

        /* routing configuration */
        value &= ~route_possibilities[packet - 1].reg_mask;
        value |= (queue << route_possibilities[packet-1].reg_shift) &
                 route_possibilities[packet - 1].reg_mask;


Calling the function with e.g. packet == 0 will lead to interesting stuff,
so the smatch warning is absolutely warranted.



Looking where this function is used:

static const struct stmmac_ops dwmac4_ops = {
        ...
        .rx_queue_routing = dwmac4_tx_queue_routing,

Mixing rx and tx.. is this really correct?



Looking where the rx_queue_routing function is used:
git grep rx_queue_routing
stmmac_main.c:  if (rx_queues_count > 1 && priv->hw->mac->rx_queue_routing)

it is just referenced in a single place, and we only check if function is
non-NULL, we never even call the function, so right now it is just unused
code.


Regards,
Niklas