From: Yongseok Koh <yskoh@mellanox.com>
To: Andrius Sirvys <andrius.sirvys@intel.com>
Cc: "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [17.11] drivers/net: fix shifting 32-bit signed variable 31 times
Date: Fri, 2 Aug 2019 00:05:43 +0000 [thread overview]
Message-ID: <FF5E0A3A-1CC9-4D3B-9CC9-C2AFC54C5D15@mellanox.com> (raw)
In-Reply-To: <20190725093342.5290-1-andrius.sirvys@intel.com>
> On Jul 25, 2019, at 2:33 AM, Andrius Sirvys <andrius.sirvys@intel.com> wrote:
>
> [ backported from upstream commit 902f389f9b4cad1feba320e8b779432eeb76cada ]
>
> Shifting signed 32-bit values by 31-bits has the potential for
> unexpected outcomes as compiler can overwrite a bit.
> Specified that values are unsigned.
>
> Errors are observed from running cppcheck.
>
> Bugzilla ID: 58
> Fixes: b5bf7719221d ("bnx2x: driver support routines")
> Fixes: bdb244b96920 ("e1000: whitespace changes")
> Fixes: 5a32a257f957 ("e1000: more NICs in base driver")
> Fixes: 2fe669f4bcd2 ("net/nfp: support MAC address change")
> Fixes: defb9a5dd156 ("nfp: introduce driver initialization")
> Fixes: ec94dbc57362 ("qede: add base driver")
> Fixes: d2e7d931d0ad ("net/qede/base: formatting changes")
> Cc: stable@dpdk.org
>
> Signed-off-by: Andrius Sirvys <andrius.sirvys@intel.com>
>
> ---
applied to stable/17.11
thanks,
>
> Cc: ravi1.kumar@amd.com
> Cc: stephen@networkplumber.org
> Cc: alejandro.lucero@netronome.com
> Cc: rasesh.mody@qlogic.com
> ---
> drivers/net/bnx2x/ecore_hsi.h | 2 +-
> drivers/net/bnx2x/ecore_reg.h | 2 +-
> drivers/net/bnx2x/elink.h | 2 +-
> drivers/net/e1000/base/e1000_82575.h | 4 ++--
> drivers/net/e1000/base/e1000_ich8lan.c | 2 +-
> drivers/net/nfp/nfp_net_ctrl.h | 2 +-
> drivers/net/qede/base/common_hsi.h | 4 ++--
> drivers/net/qede/base/ecore_hsi_common.h | 2 +-
> drivers/net/qede/base/ecore_hsi_eth.h | 2 +-
> drivers/net/qede/base/ecore_hw_defs.h | 2 +-
> 10 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/bnx2x/ecore_hsi.h b/drivers/net/bnx2x/ecore_hsi.h
> index 5cce66474..e7878c0e4 100644
> --- a/drivers/net/bnx2x/ecore_hsi.h
> +++ b/drivers/net/bnx2x/ecore_hsi.h
> @@ -3529,7 +3529,7 @@ struct igu_regular
> #define IGU_REGULAR_CLEANUP_TYPE_SHIFT 28
> #define IGU_REGULAR_CLEANUP_SET (0x1<<30) /* BitField sb_id_and_flags */
> #define IGU_REGULAR_CLEANUP_SET_SHIFT 30
> -#define IGU_REGULAR_BCLEANUP (0x1<<31) /* BitField sb_id_and_flags */
> +#define IGU_REGULAR_BCLEANUP (0x1U<<31) /* BitField sb_id_and_flags */
> #define IGU_REGULAR_BCLEANUP_SHIFT 31
> uint32_t reserved_2;
> };
> diff --git a/drivers/net/bnx2x/ecore_reg.h b/drivers/net/bnx2x/ecore_reg.h
> index 33cea4eb9..e2bdc8bd7 100644
> --- a/drivers/net/bnx2x/ecore_reg.h
> +++ b/drivers/net/bnx2x/ecore_reg.h
> @@ -1983,7 +1983,7 @@
> #define AEU_INPUTS_ATTN_BITS_ATC_PARITY_ERROR (0x1<<5)
> #define AEU_INPUTS_ATTN_BITS_BRB_HW_INTERRUPT (0x1<<19)
> #define AEU_INPUTS_ATTN_BITS_BRB_PARITY_ERROR (0x1<<18)
> -#define AEU_INPUTS_ATTN_BITS_CCM_HW_INTERRUPT (0x1<<31)
> +#define AEU_INPUTS_ATTN_BITS_CCM_HW_INTERRUPT (0x1U<<31)
> #define AEU_INPUTS_ATTN_BITS_CCM_PARITY_ERROR (0x1<<30)
> #define AEU_INPUTS_ATTN_BITS_CDU_HW_INTERRUPT (0x1<<9)
> #define AEU_INPUTS_ATTN_BITS_CDU_PARITY_ERROR (0x1<<8)
> diff --git a/drivers/net/bnx2x/elink.h b/drivers/net/bnx2x/elink.h
> index 9401b7cd5..cae354b92 100644
> --- a/drivers/net/bnx2x/elink.h
> +++ b/drivers/net/bnx2x/elink.h
> @@ -396,7 +396,7 @@ struct elink_params {
> #define ELINK_EEE_MODE_OUTPUT_TIME (1<<28)
> #define ELINK_EEE_MODE_OVERRIDE_NVRAM (1<<29)
> #define ELINK_EEE_MODE_ENABLE_LPI (1<<30)
> -#define ELINK_EEE_MODE_ADV_LPI (1<<31)
> +#define ELINK_EEE_MODE_ADV_LPI (1U<<31)
>
> uint16_t hw_led_mode; /* part of the hw_config read from the shmem */
> uint32_t multi_phy_config;
> diff --git a/drivers/net/e1000/base/e1000_82575.h b/drivers/net/e1000/base/e1000_82575.h
> index 4133cdd82..6f2b22c13 100644
> --- a/drivers/net/e1000/base/e1000_82575.h
> +++ b/drivers/net/e1000/base/e1000_82575.h
> @@ -383,7 +383,7 @@ struct e1000_adv_tx_context_desc {
> #define E1000_ETQF_FILTER_ENABLE (1 << 26)
> #define E1000_ETQF_IMM_INT (1 << 29)
> #define E1000_ETQF_1588 (1 << 30)
> -#define E1000_ETQF_QUEUE_ENABLE (1 << 31)
> +#define E1000_ETQF_QUEUE_ENABLE (1U << 31)
> /*
> * ETQF filter list: one static filter per filter consumer. This is
> * to avoid filter collisions later. Add new filters
> @@ -410,7 +410,7 @@ struct e1000_adv_tx_context_desc {
> #define E1000_DTXSWC_LLE_MASK 0x00FF0000 /* Per VF Local LB enables */
> #define E1000_DTXSWC_VLAN_SPOOF_SHIFT 8
> #define E1000_DTXSWC_LLE_SHIFT 16
> -#define E1000_DTXSWC_VMDQ_LOOPBACK_EN (1 << 31) /* global VF LB enable */
> +#define E1000_DTXSWC_VMDQ_LOOPBACK_EN (1U << 31) /* global VF LB enable */
>
> /* Easy defines for setting default pool, would normally be left a zero */
> #define E1000_VT_CTL_DEFAULT_POOL_SHIFT 7
> diff --git a/drivers/net/e1000/base/e1000_ich8lan.c b/drivers/net/e1000/base/e1000_ich8lan.c
> index 6dd046d2f..5475a69d8 100644
> --- a/drivers/net/e1000/base/e1000_ich8lan.c
> +++ b/drivers/net/e1000/base/e1000_ich8lan.c
> @@ -5166,7 +5166,7 @@ STATIC void e1000_initialize_hw_bits_ich8lan(struct e1000_hw *hw)
> /* Device Status */
> if (hw->mac.type == e1000_ich8lan) {
> reg = E1000_READ_REG(hw, E1000_STATUS);
> - reg &= ~(1 << 31);
> + reg &= ~(1U << 31);
> E1000_WRITE_REG(hw, E1000_STATUS, reg);
> }
>
> diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
> index 1ebd99caf..b7f98e3cf 100644
> --- a/drivers/net/nfp/nfp_net_ctrl.h
> +++ b/drivers/net/nfp/nfp_net_ctrl.h
> @@ -131,7 +131,7 @@
> #define NFP_NET_CFG_UPDATE_RESET (0x1 << 7) /* Update due to FLR */
> #define NFP_NET_CFG_UPDATE_IRQMOD (0x1 << 8) /* IRQ mod change */
> #define NFP_NET_CFG_UPDATE_VXLAN (0x1 << 9) /* VXLAN port change */
> -#define NFP_NET_CFG_UPDATE_ERR (0x1 << 31) /* A error occurred */
> +#define NFP_NET_CFG_UPDATE_ERR (0x1U << 31) /* A error occurred */
> #define NFP_NET_CFG_TXRS_ENABLE 0x0008
> #define NFP_NET_CFG_RXRS_ENABLE 0x0010
> #define NFP_NET_CFG_MTU 0x0018
> diff --git a/drivers/net/qede/base/common_hsi.h b/drivers/net/qede/base/common_hsi.h
> index 9a6059ac2..4dce5426d 100644
> --- a/drivers/net/qede/base/common_hsi.h
> +++ b/drivers/net/qede/base/common_hsi.h
> @@ -402,9 +402,9 @@
> #define QM_BYTE_CRD_REG_WIDTH 24
> #define QM_BYTE_CRD_REG_SIGN_BIT (1 << (QM_BYTE_CRD_REG_WIDTH - 1))
> #define QM_WFQ_CRD_REG_WIDTH 32
> -#define QM_WFQ_CRD_REG_SIGN_BIT (1 << (QM_WFQ_CRD_REG_WIDTH - 1))
> +#define QM_WFQ_CRD_REG_SIGN_BIT (1U << (QM_WFQ_CRD_REG_WIDTH - 1))
> #define QM_RL_CRD_REG_WIDTH 32
> -#define QM_RL_CRD_REG_SIGN_BIT (1 << (QM_RL_CRD_REG_WIDTH - 1))
> +#define QM_RL_CRD_REG_SIGN_BIT (1U << (QM_RL_CRD_REG_WIDTH - 1))
>
> /*****************/
> /* CAU CONSTANTS */
> diff --git a/drivers/net/qede/base/ecore_hsi_common.h b/drivers/net/qede/base/ecore_hsi_common.h
> index 31ae2a0fe..e499a5b4f 100644
> --- a/drivers/net/qede/base/ecore_hsi_common.h
> +++ b/drivers/net/qede/base/ecore_hsi_common.h
> @@ -2146,7 +2146,7 @@ struct igu_cleanup {
> #define IGU_CLEANUP_CLEANUP_TYPE_MASK 0x7
> #define IGU_CLEANUP_CLEANUP_TYPE_SHIFT 28
> /* must always be set (use enum command_type_bit) */
> -#define IGU_CLEANUP_COMMAND_TYPE_MASK 0x1
> +#define IGU_CLEANUP_COMMAND_TYPE_MASK 0x1U
> #define IGU_CLEANUP_COMMAND_TYPE_SHIFT 31
> __le32 reserved1;
> };
> diff --git a/drivers/net/qede/base/ecore_hsi_eth.h b/drivers/net/qede/base/ecore_hsi_eth.h
> index ffbf5c712..1ddd99c27 100644
> --- a/drivers/net/qede/base/ecore_hsi_eth.h
> +++ b/drivers/net/qede/base/ecore_hsi_eth.h
> @@ -2368,7 +2368,7 @@ struct gft_ram_line {
> #define GFT_RAM_LINE_TCP_FLAG_NS_SHIFT 29
> #define GFT_RAM_LINE_DST_PORT_MASK 0x1
> #define GFT_RAM_LINE_DST_PORT_SHIFT 30
> -#define GFT_RAM_LINE_SRC_PORT_MASK 0x1
> +#define GFT_RAM_LINE_SRC_PORT_MASK 0x1U
> #define GFT_RAM_LINE_SRC_PORT_SHIFT 31
> __le32 hi;
> #define GFT_RAM_LINE_DSCP_MASK 0x1
> diff --git a/drivers/net/qede/base/ecore_hw_defs.h b/drivers/net/qede/base/ecore_hw_defs.h
> index 4456af437..d189712e4 100644
> --- a/drivers/net/qede/base/ecore_hw_defs.h
> +++ b/drivers/net/qede/base/ecore_hw_defs.h
> @@ -53,7 +53,7 @@ struct igu_ctrl_reg {
> #define IGU_CTRL_REG_PXP_ADDR_SHIFT 16
> #define IGU_CTRL_REG_RESERVED_MASK 0x1
> #define IGU_CTRL_REG_RESERVED_SHIFT 28
> -#define IGU_CTRL_REG_TYPE_MASK 0x1 /* use enum igu_ctrl_cmd */
> +#define IGU_CTRL_REG_TYPE_MASK 0x1U /* use enum igu_ctrl_cmd */
> #define IGU_CTRL_REG_TYPE_SHIFT 31
> };
>
> --
> 2.17.1
>
prev parent reply other threads:[~2019-08-02 0:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 9:33 Andrius Sirvys
2019-08-02 0:05 ` Yongseok Koh [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=FF5E0A3A-1CC9-4D3B-9CC9-C2AFC54C5D15@mellanox.com \
--to=yskoh@mellanox.com \
--cc=andrius.sirvys@intel.com \
--cc=stable@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).