2022-04-06 05:18:27

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 00/11] treewide: Fix a bunch of shift overflows

From: Borislav Petkov <[email protected]>

Hi all,

so this is the result of me trying to make allmodconfig actually build
here.

Due to some recent changes which added -fsanitize-shift to the build
options of an allmodconfig, it started failing here with an old gcc
because getting an overflow while shifting is undefined C99 behavior.

gcc warns/errors out with -Werror about this only on newer versions
where -pedantic is present while older ones do so even without it. The
whole details here:

https://lore.kernel.org/r/YkwQ6%[email protected]

Fixing all those is trivial so please pick up at your convenience.

In order to avoid spamming people unnecessarily, I'm not CCing everyone
on each patch but only the relevant maintainers and lists.

Thx.

Borislav Petkov (11):
scsi: aacraid: Fix undefined behavior due to shift overflowing the
constant
ALSA: usb-audio: Fix undefined behavior due to shift overflowing the
constant
bnx2x: Fix undefined behavior due to shift overflowing the constant
drm/r128: Fix undefined behavior due to shift overflowing the constant
i2c: ismt: Fix undefined behavior due to shift overflowing the
constant
brcmfmac: sdio: Fix undefined behavior due to shift overflowing the
constant
usb: typec: tcpm: Fix undefined behavior due to shift overflowing the
constant
mt76: Fix undefined behavior due to shift overflowing the constant
perf/imx_ddr: Fix undefined behavior due to shift overflowing the
constant
IB/mlx5: Fix undefined behavior due to shift overflowing the constant
drm/i915: Fix undefined behavior due to shift overflowing the constant

.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +-
.../i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +-
.../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 18 +++++++++---------
drivers/gpu/drm/r128/r128_drv.h | 4 ++--
drivers/i2c/busses/i2c-ismt.c | 4 ++--
.../net/ethernet/broadcom/bnx2x/bnx2x_reg.h | 2 +-
.../broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
.../net/wireless/mediatek/mt76/mt76x2/pci.c | 2 +-
drivers/perf/fsl_imx8_ddr_perf.c | 2 +-
drivers/scsi/aacraid/aacraid.h | 2 +-
include/linux/mlx5/port.h | 2 +-
include/linux/usb/pd_bdo.h | 2 +-
sound/usb/usbaudio.h | 2 +-
15 files changed, 25 insertions(+), 25 deletions(-)

--
2.35.1


2022-04-06 05:42:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 02/11] ALSA: usb-audio: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
case USB_ID(0xfc08, 0x0101): /* Unknown vendor Cable */
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Jaroslav Kysela <[email protected]>
Cc: Takashi Iwai <[email protected]>
Cc: [email protected]
---
sound/usb/usbaudio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 167834133b9b..2b3dd55e8ee0 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -8,7 +8,7 @@
*/

/* handling of USB vendor/product ID pairs as 32-bit numbers */
-#define USB_ID(vendor, product) (((vendor) << 16) | (product))
+#define USB_ID(vendor, product) ((((unsigned int)vendor) << 16) | (product))
#define USB_ID_VENDOR(id) ((id) >> 16)
#define USB_ID_PRODUCT(id) ((u16)(id))

--
2.35.1

2022-04-06 06:15:26

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 07/11] usb: typec: tcpm: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

drivers/usb/typec/tcpm/tcpm.c: In function ‘run_state_machine’:
drivers/usb/typec/tcpm/tcpm.c:4724:3: error: case label does not reduce to an integer constant
case BDO_MODE_TESTDATA:
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
include/linux/usb/pd_bdo.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/usb/pd_bdo.h b/include/linux/usb/pd_bdo.h
index 033fe3e17141..7c25b88d79f9 100644
--- a/include/linux/usb/pd_bdo.h
+++ b/include/linux/usb/pd_bdo.h
@@ -15,7 +15,7 @@
#define BDO_MODE_CARRIER2 (5 << 28)
#define BDO_MODE_CARRIER3 (6 << 28)
#define BDO_MODE_EYE (7 << 28)
-#define BDO_MODE_TESTDATA (8 << 28)
+#define BDO_MODE_TESTDATA (8U << 28)

#define BDO_MODE_MASK(mode) ((mode) & 0xf0000000)

--
2.35.1

2022-04-06 10:25:59

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

In file included from <command-line>:0:0:
drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’:
././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_1047’ \
declared with attribute error: FIELD_PREP: mask is not constant
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)

and other build errors due to shift overflowing values.

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +-
.../i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +-
.../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 18 +++++++++---------
5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index 7afdadc7656f..e835f28c0020 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -50,7 +50,7 @@

#define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u)
#define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ GUC_HXG_REQUEST_MSG_0_DATA0
-#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xffff << 16)
+#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xffffU << 16)
#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN (0xffff << 0)
#define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32 GUC_HXG_REQUEST_MSG_n_DATAn
#define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64 GUC_HXG_REQUEST_MSG_n_DATAn
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
index c9086a600bce..df83c1cc7c7a 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
@@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64);
#define GUC_CTB_HDR_LEN 1u
#define GUC_CTB_MSG_MIN_LEN GUC_CTB_HDR_LEN
#define GUC_CTB_MSG_MAX_LEN 256u
-#define GUC_CTB_MSG_0_FENCE (0xffff << 16)
+#define GUC_CTB_MSG_0_FENCE (0xffffU << 16)
#define GUC_CTB_MSG_0_FORMAT (0xf << 12)
#define GUC_CTB_FORMAT_HXG 0u
#define GUC_CTB_MSG_0_RESERVED (0xf << 8)
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
index 29ac823acd4c..7d5ba4d97d70 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
@@ -40,7 +40,7 @@
*/

#define GUC_HXG_MSG_MIN_LEN 1u
-#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31)
+#define GUC_HXG_MSG_0_ORIGIN (0x1U << 31)
#define GUC_HXG_ORIGIN_HOST 0u
#define GUC_HXG_ORIGIN_GUC 1u
#define GUC_HXG_MSG_0_TYPE (0x7 << 28)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
index 66027a42cda9..ad570fa002a6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
@@ -28,7 +28,7 @@
#define GS_MIA_HALT_REQUESTED (0x02 << GS_MIA_SHIFT)
#define GS_MIA_ISR_ENTRY (0x04 << GS_MIA_SHIFT)
#define GS_AUTH_STATUS_SHIFT 30
-#define GS_AUTH_STATUS_MASK (0x03 << GS_AUTH_STATUS_SHIFT)
+#define GS_AUTH_STATUS_MASK (0x03U << GS_AUTH_STATUS_SHIFT)
#define GS_AUTH_STATUS_BAD (0x01 << GS_AUTH_STATUS_SHIFT)
#define GS_AUTH_STATUS_GOOD (0x02 << GS_AUTH_STATUS_SHIFT)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3c87d77d2cf6..f3ba3d0a430b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7555,19 +7555,19 @@ enum skl_power_gate {
#define PORT_CLK_SEL_LCPLL_810 (2 << 29)
#define PORT_CLK_SEL_SPLL (3 << 29)
#define PORT_CLK_SEL_WRPLL(pll) (((pll) + 4) << 29)
-#define PORT_CLK_SEL_WRPLL1 (4 << 29)
-#define PORT_CLK_SEL_WRPLL2 (5 << 29)
-#define PORT_CLK_SEL_NONE (7 << 29)
-#define PORT_CLK_SEL_MASK (7 << 29)
+#define PORT_CLK_SEL_WRPLL1 (4U << 29)
+#define PORT_CLK_SEL_WRPLL2 (5U << 29)
+#define PORT_CLK_SEL_NONE (7U << 29)
+#define PORT_CLK_SEL_MASK (7U << 29)

/* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */
#define DDI_CLK_SEL(port) PORT_CLK_SEL(port)
#define DDI_CLK_SEL_NONE (0x0 << 28)
-#define DDI_CLK_SEL_MG (0x8 << 28)
-#define DDI_CLK_SEL_TBT_162 (0xC << 28)
-#define DDI_CLK_SEL_TBT_270 (0xD << 28)
-#define DDI_CLK_SEL_TBT_540 (0xE << 28)
-#define DDI_CLK_SEL_TBT_810 (0xF << 28)
+#define DDI_CLK_SEL_MG (0x8U << 28)
+#define DDI_CLK_SEL_TBT_162 (0xCU << 28)
+#define DDI_CLK_SEL_TBT_270 (0xDU << 28)
+#define DDI_CLK_SEL_TBT_540 (0xEU << 28)
+#define DDI_CLK_SEL_TBT_810 (0xFU << 28)
#define DDI_CLK_SEL_MASK (0xF << 28)

/* Transcoder clock selection */
--
2.35.1

2022-04-06 10:38:00

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 10/11] IB/mlx5: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

drivers/infiniband/hw/mlx5/main.c: In function ‘translate_eth_legacy_proto_oper’:
drivers/infiniband/hw/mlx5/main.c:370:2: error: case label does not reduce to an integer constant
case MLX5E_PROT_MASK(MLX5E_50GBASE_KR2):
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Leon Romanovsky <[email protected]>
Cc: Saeed Mahameed <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
include/linux/mlx5/port.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mlx5/port.h b/include/linux/mlx5/port.h
index 28a928b0684b..e96ee1e348cb 100644
--- a/include/linux/mlx5/port.h
+++ b/include/linux/mlx5/port.h
@@ -141,7 +141,7 @@ enum mlx5_ptys_width {
MLX5_PTYS_WIDTH_12X = 1 << 4,
};

-#define MLX5E_PROT_MASK(link_mode) (1 << link_mode)
+#define MLX5E_PROT_MASK(link_mode) (1U << link_mode)
#define MLX5_GET_ETH_PROTO(reg, out, ext, field) \
(ext ? MLX5_GET(reg, out, ext_##field) : \
MLX5_GET(reg, out, field))
--
2.35.1

2022-04-06 11:14:08

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 06/11] brcmfmac: sdio: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c: In function ‘brcmf_sdio_drivestrengthinit’:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:3798:2: error: case label does not reduce to an integer constant
case SDIOD_DRVSTR_KEY(BRCM_CC_43143_CHIP_ID, 17):
^~~~
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:3809:2: error: case label does not reduce to an integer constant
case SDIOD_DRVSTR_KEY(BRCM_CC_43362_CHIP_ID, 13):
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Arend van Spriel <[email protected]>
Cc: Franky Lin <[email protected]>
Cc: Hante Meuleman <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index ba3c159111d3..d78ccc223709 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -557,7 +557,7 @@ enum brcmf_sdio_frmtype {
BRCMF_SDIO_FT_SUB,
};

-#define SDIOD_DRVSTR_KEY(chip, pmu) (((chip) << 16) | (pmu))
+#define SDIOD_DRVSTR_KEY(chip, pmu) (((unsigned int)(chip) << 16) | (pmu))

/* SDIO Pad drive strength to select value mappings */
struct sdiod_drive_str {
--
2.35.1

2022-04-06 11:19:06

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 02/11] ALSA: usb-audio: Fix undefined behavior due to shift overflowing the constant

On Tue, 05 Apr 2022 17:15:08 +0200,
Borislav Petkov wrote:
>
> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
> case USB_ID(0xfc08, 0x0101): /* Unknown vendor Cable */
> ^~~~
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Jaroslav Kysela <[email protected]>
> Cc: Takashi Iwai <[email protected]>
> Cc: [email protected]
> ---
> sound/usb/usbaudio.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> index 167834133b9b..2b3dd55e8ee0 100644
> --- a/sound/usb/usbaudio.h
> +++ b/sound/usb/usbaudio.h
> @@ -8,7 +8,7 @@
> */
>
> /* handling of USB vendor/product ID pairs as 32-bit numbers */
> -#define USB_ID(vendor, product) (((vendor) << 16) | (product))
> +#define USB_ID(vendor, product) ((((unsigned int)vendor) << 16) | (product))

Parentheses are needed around vendor (as usual for a macro).
Could you resubmit with it?


Thanks.

Takashi

2022-04-06 12:15:47

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’:
drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce to an integer constant
case R128_PM4_64BM_64VCBM_64INDBM:
^~~~
drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce to an integer constant
case R128_PM4_64PIO_64VCPIO_64INDPIO:
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: [email protected]
---
drivers/gpu/drm/r128/r128_drv.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
index 2e1bc01aa5c9..970e192b0d51 100644
--- a/drivers/gpu/drm/r128/r128_drv.h
+++ b/drivers/gpu/drm/r128/r128_drv.h
@@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, unsigned int cmd,
# define R128_PM4_64PIO_128INDBM (5 << 28)
# define R128_PM4_64BM_128INDBM (6 << 28)
# define R128_PM4_64PIO_64VCBM_64INDBM (7 << 28)
-# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28)
-# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28)
+# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28)
+# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28)
# define R128_PM4_BUFFER_CNTL_NOUPDATE (1 << 27)

#define R128_PM4_BUFFER_WM_CNTL 0x0708
--
2.35.1

2022-04-06 12:28:47

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 05/11] i2c: ismt: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

drivers/i2c/busses/i2c-ismt.c: In function ‘ismt_hw_init’:
drivers/i2c/busses/i2c-ismt.c:770:2: error: case label does not reduce to an integer constant
case ISMT_SPGT_SPD_400K:
^~~~
drivers/i2c/busses/i2c-ismt.c:773:2: error: case label does not reduce to an integer constant
case ISMT_SPGT_SPD_1M:
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Seth Heasley <[email protected]>
Cc: Neil Horman <[email protected]>
Cc: Wolfram Sang <[email protected]>
Cc: [email protected]
---
drivers/i2c/busses/i2c-ismt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ismt.c b/drivers/i2c/busses/i2c-ismt.c
index f4820fd3dc13..c0364314877e 100644
--- a/drivers/i2c/busses/i2c-ismt.c
+++ b/drivers/i2c/busses/i2c-ismt.c
@@ -145,8 +145,8 @@
#define ISMT_SPGT_SPD_MASK 0xc0000000 /* SMBus Speed mask */
#define ISMT_SPGT_SPD_80K 0x00 /* 80 kHz */
#define ISMT_SPGT_SPD_100K (0x1 << 30) /* 100 kHz */
-#define ISMT_SPGT_SPD_400K (0x2 << 30) /* 400 kHz */
-#define ISMT_SPGT_SPD_1M (0x3 << 30) /* 1 MHz */
+#define ISMT_SPGT_SPD_400K (0x2U << 30) /* 400 kHz */
+#define ISMT_SPGT_SPD_1M (0x3U << 30) /* 1 MHz */


/* MSI Control Register (MSICTL) bit definitions */
--
2.35.1

2022-04-06 12:49:56

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 09/11] perf/imx_ddr: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

In file included from <command-line>:0:0:
In function ‘ddr_perf_counter_enable’,
inlined from ‘ddr_perf_irq_handler’ at drivers/perf/fsl_imx8_ddr_perf.c:651:2:
././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_729’ \
declared with attribute error: FIELD_PREP: mask is not constant
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
...

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: [email protected]
---
drivers/perf/fsl_imx8_ddr_perf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
index 94ebc1ecace7..b1b2a55de77f 100644
--- a/drivers/perf/fsl_imx8_ddr_perf.c
+++ b/drivers/perf/fsl_imx8_ddr_perf.c
@@ -29,7 +29,7 @@
#define CNTL_OVER_MASK 0xFFFFFFFE

#define CNTL_CSV_SHIFT 24
-#define CNTL_CSV_MASK (0xFF << CNTL_CSV_SHIFT)
+#define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT)

#define EVENT_CYCLES_ID 0
#define EVENT_CYCLES_COUNTER 0
--
2.35.1

2022-04-06 12:57:48

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 06/11] brcmfmac: sdio: Fix undefined behavior due to shift overflowing the constant

+ linux-wireless

Borislav Petkov <[email protected]> writes:

> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c: In function ‘brcmf_sdio_drivestrengthinit’:
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:3798:2: error: case label does not reduce to an integer constant
> case SDIOD_DRVSTR_KEY(BRCM_CC_43143_CHIP_ID, 17):
> ^~~~
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:3809:2: error: case label does not reduce to an integer constant
> case SDIOD_DRVSTR_KEY(BRCM_CC_43362_CHIP_ID, 13):
> ^~~~
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Arend van Spriel <[email protected]>
> Cc: Franky Lin <[email protected]>
> Cc: Hante Meuleman <[email protected]>
> Cc: Kalle Valo <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index ba3c159111d3..d78ccc223709 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -557,7 +557,7 @@ enum brcmf_sdio_frmtype {
> BRCMF_SDIO_FT_SUB,
> };
>
> -#define SDIOD_DRVSTR_KEY(chip, pmu) (((chip) << 16) | (pmu))
> +#define SDIOD_DRVSTR_KEY(chip, pmu) (((unsigned int)(chip) << 16) | (pmu))

Via which tree is this going? I assume not the wireless tree, so:

Acked-by: Kalle Valo <[email protected]>

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-04-06 13:03:14

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 03/11] bnx2x: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix:

drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c: In function ‘bnx2x_check_blocks_with_parity3’:
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:4917:4: error: case label does not reduce to an integer constant
case AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY:
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Ariel Elior <[email protected]>
Cc: Sudarsana Kalluru <[email protected]>
Cc: Manish Chopra <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: [email protected]
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h
index 5caa75b41b73..881ac33fe914 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h
@@ -6218,7 +6218,7 @@
#define AEU_INPUTS_ATTN_BITS_GPIO0_FUNCTION_0 (0x1<<2)
#define AEU_INPUTS_ATTN_BITS_IGU_PARITY_ERROR (0x1<<12)
#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_ROM_PARITY (0x1<<28)
-#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY (0x1<<31)
+#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY (0x1U<<31)
#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_UMP_RX_PARITY (0x1<<29)
#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_UMP_TX_PARITY (0x1<<30)
#define AEU_INPUTS_ATTN_BITS_MISC_HW_INTERRUPT (0x1<<15)
--
2.35.1

2022-04-06 13:19:44

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 02/11] ALSA: usb-audio: Fix undefined behavior due to shift overflowing the constant

On Tue, 05 Apr 2022 18:06:19 +0200,
Borislav Petkov wrote:
>
> On Tue, Apr 05, 2022 at 05:32:48PM +0200, Takashi Iwai wrote:
> > > +#define USB_ID(vendor, product) ((((unsigned int)vendor) << 16) | (product))
> >
> > Parentheses are needed around vendor (as usual for a macro).
> > Could you resubmit with it?
>
> Or you can fix it up while applying. :)

If you prefer, sure.


Takashi

2022-04-06 13:46:19

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 02/11] ALSA: usb-audio: Fix undefined behavior due to shift overflowing the constant

On Tue, Apr 05, 2022 at 05:32:48PM +0200, Takashi Iwai wrote:
> > +#define USB_ID(vendor, product) ((((unsigned int)vendor) << 16) | (product))
>
> Parentheses are needed around vendor (as usual for a macro).
> Could you resubmit with it?

Or you can fix it up while applying. :)

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-04-06 13:49:09

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 01/11] scsi: aacraid: Fix undefined behavior due to shift overflowing the constant

From: Borislav Petkov <[email protected]>

Fix

drivers/scsi/aacraid/commsup.c: In function ‘aac_handle_sa_aif’:
drivers/scsi/aacraid/commsup.c:1983:2: error: case label does not reduce to an integer constant
case SA_AIF_BPCFG_CHANGE:
^~~~

See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
details as to why it triggers with older gccs only.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Adaptec OEM Raid Solutions <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: [email protected]
---
drivers/scsi/aacraid/aacraid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index f849e7c9d428..5e115e8b2ba4 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -121,7 +121,7 @@ enum {
#define SA_AIF_PDEV_CHANGE (1<<4)
#define SA_AIF_LDEV_CHANGE (1<<5)
#define SA_AIF_BPSTAT_CHANGE (1<<30)
-#define SA_AIF_BPCFG_CHANGE (1<<31)
+#define SA_AIF_BPCFG_CHANGE (1U<<31)

#define HBA_MAX_SG_EMBEDDED 28
#define HBA_MAX_SG_SEPARATE 90
--
2.35.1

2022-04-06 14:16:10

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH 10/11] IB/mlx5: Fix undefined behavior due to shift overflowing the constant

On Tue, Apr 05, 2022 at 05:15:16PM +0200, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> drivers/infiniband/hw/mlx5/main.c: In function ‘translate_eth_legacy_proto_oper’:
> drivers/infiniband/hw/mlx5/main.c:370:2: error: case label does not reduce to an integer constant
> case MLX5E_PROT_MASK(MLX5E_50GBASE_KR2):
> ^~~~
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Leon Romanovsky <[email protected]>
> Cc: Saeed Mahameed <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> include/linux/mlx5/port.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to mlx5-next.

0276bd3a94c0 ("IB/mlx5: Fix undefined behavior due to shift overflowing the constant")

2022-04-06 15:12:55

by David Laight

[permalink] [raw]
Subject: RE: [PATCH 00/11] treewide: Fix a bunch of shift overflows

From: Borislav Petkov
> Sent: 05 April 2022 16:15
...
> Due to some recent changes which added -fsanitize-shift to the build
> options of an allmodconfig, it started failing here with an old gcc
> because getting an overflow while shifting is undefined C99 behavior.

Annoyingly it is extremely unlikely that a 2s compliment
system would ever generate anything other than the expected
value - ie the value obtained by doing the calculation
with unsigned values and then getting the result cast back
to signed (which is IIRC implementation defined).

It isn't as though there are many 1s compliment systems out
there, and probably even fewer 'sign overpunch' ones.
Never mind trying to find a working C compiler for them.

What sort of crack are the C standards body and compiler
people actually smoking?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2022-04-06 16:54:26

by Seth Heasley

[permalink] [raw]
Subject: Re: [PATCH 05/11] i2c: ismt: Fix undefined behavior due to shift overflowing the constant

On Tue, 2022-04-05 at 17:15 +0200, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> drivers/i2c/busses/i2c-ismt.c: In function ‘ismt_hw_init’:
> drivers/i2c/busses/i2c-ismt.c:770:2: error: case label does not
> reduce to an integer constant
> case ISMT_SPGT_SPD_400K:
> ^~~~
> drivers/i2c/busses/i2c-ismt.c:773:2: error: case label does not
> reduce to an integer constant
> case ISMT_SPGT_SPD_1M:
> ^~~~
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Seth Heasley <[email protected]>

> Cc: Seth Heasley <[email protected]>
> Cc: Neil Horman <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> Cc: [email protected]
> ---
> drivers/i2c/busses/i2c-ismt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-ismt.c b/drivers/i2c/busses/i2c-
> ismt.c
> index f4820fd3dc13..c0364314877e 100644
> --- a/drivers/i2c/busses/i2c-ismt.c
> +++ b/drivers/i2c/busses/i2c-ismt.c
> @@ -145,8 +145,8 @@
> #define ISMT_SPGT_SPD_MASK 0xc0000000 /* SMBus Speed mask
> */
> #define ISMT_SPGT_SPD_80K 0x00 /* 80 kHz */
> #define ISMT_SPGT_SPD_100K (0x1 << 30) /* 100 kHz */
> -#define ISMT_SPGT_SPD_400K (0x2 << 30) /* 400 kHz */
> -#define ISMT_SPGT_SPD_1M (0x3 << 30) /* 1 MHz */
> +#define ISMT_SPGT_SPD_400K (0x2U << 30) /* 400 kHz */
> +#define ISMT_SPGT_SPD_1M (0x3U << 30) /* 1 MHz */
>
>
> /* MSI Control Register (MSICTL) bit definitions */

2022-04-08 12:31:30

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf/imx_ddr: Fix undefined behavior due to shift overflowing the constant

On Fri, Apr 08, 2022 at 11:47:40AM +0100, Will Deacon wrote:
> (let me know if you'd prefer for me to queue this directly)

Yes please.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-04-08 16:49:45

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf/imx_ddr: Fix undefined behavior due to shift overflowing the constant

On Tue, Apr 05, 2022 at 05:15:15PM +0200, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> In file included from <command-line>:0:0:
> In function ‘ddr_perf_counter_enable’,
> inlined from ‘ddr_perf_irq_handler’ at drivers/perf/fsl_imx8_ddr_perf.c:651:2:
> ././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_729’ \
> declared with attribute error: FIELD_PREP: mask is not constant
> _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> ...
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Frank Li <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Pengutronix Kernel Team <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: NXP Linux Team <[email protected]>
> Cc: [email protected]
> ---
> drivers/perf/fsl_imx8_ddr_perf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index 94ebc1ecace7..b1b2a55de77f 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -29,7 +29,7 @@
> #define CNTL_OVER_MASK 0xFFFFFFFE
>
> #define CNTL_CSV_SHIFT 24
> -#define CNTL_CSV_MASK (0xFF << CNTL_CSV_SHIFT)
> +#define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT)

Acked-by: Will Deacon <[email protected]>

(let me know if you'd prefer for me to queue this directly)

Will

2022-04-11 06:58:07

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 00/11] treewide: Fix a bunch of shift overflows

On Tue, 5 Apr 2022 17:15:06 +0200, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Hi all,
>
> so this is the result of me trying to make allmodconfig actually build
> here.
>
> [...]

Applied perf/imx_ddr patch only to arm64 (for-next/fixes), thanks!

[09/11] perf/imx_ddr: Fix undefined behavior due to shift overflowing the constant
https://git.kernel.org/arm64/c/d02b4dd84e1a

Cheers,
--
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

2022-04-26 02:52:33

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant



On 4/5/22 08:15, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’:
> drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce to an integer constant
> case R128_PM4_64BM_64VCBM_64INDBM:
> ^~~~
> drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce to an integer constant
> case R128_PM4_64PIO_64VCPIO_64INDPIO:
> ^~~~
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: [email protected]

Reviewed-by: Randy Dunlap <[email protected]>

Thanks.

> ---
> drivers/gpu/drm/r128/r128_drv.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
> index 2e1bc01aa5c9..970e192b0d51 100644
> --- a/drivers/gpu/drm/r128/r128_drv.h
> +++ b/drivers/gpu/drm/r128/r128_drv.h
> @@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, unsigned int cmd,
> # define R128_PM4_64PIO_128INDBM (5 << 28)
> # define R128_PM4_64BM_128INDBM (6 << 28)
> # define R128_PM4_64PIO_64VCBM_64INDBM (7 << 28)
> -# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28)
> -# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28)
> +# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28)
> +# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28)
> # define R128_PM4_BUFFER_CNTL_NOUPDATE (1 << 27)
>
> #define R128_PM4_BUFFER_WM_CNTL 0x0708

--
~Randy

2022-04-26 09:12:59

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 01/11] scsi: aacraid: Fix undefined behavior due to shift overflowing the constant



On 4/5/22 08:15, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Fix
>
> drivers/scsi/aacraid/commsup.c: In function ‘aac_handle_sa_aif’:
> drivers/scsi/aacraid/commsup.c:1983:2: error: case label does not reduce to an integer constant
> case SA_AIF_BPCFG_CHANGE:
> ^~~~
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Adaptec OEM Raid Solutions <[email protected]>
> Cc: "James E.J. Bottomley" <[email protected]>
> Cc: "Martin K. Petersen" <[email protected]>
> Cc: [email protected]

Reviewed-by: Randy Dunlap <[email protected]>

Thanks.

> ---
> drivers/scsi/aacraid/aacraid.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index f849e7c9d428..5e115e8b2ba4 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -121,7 +121,7 @@ enum {
> #define SA_AIF_PDEV_CHANGE (1<<4)
> #define SA_AIF_LDEV_CHANGE (1<<5)
> #define SA_AIF_BPSTAT_CHANGE (1<<30)
> -#define SA_AIF_BPCFG_CHANGE (1<<31)
> +#define SA_AIF_BPCFG_CHANGE (1U<<31)
>
> #define HBA_MAX_SG_EMBEDDED 28
> #define HBA_MAX_SG_SEPARATE 90

--
~Randy

2022-04-26 17:55:29

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 01/11] scsi: aacraid: Fix undefined behavior due to shift overflowing the constant


Borislav,

> drivers/scsi/aacraid/commsup.c: In function ‘aac_handle_sa_aif’:
> drivers/scsi/aacraid/commsup.c:1983:2: error: case label does not reduce to an integer constant
> case SA_AIF_BPCFG_CHANGE:
> ^~~~

Applied to 5.19/scsi-staging, thanks!

--
Martin K. Petersen Oracle Linux Engineering

2022-05-04 11:54:40

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 01/11] scsi: aacraid: Fix undefined behavior due to shift overflowing the constant

On Tue, 5 Apr 2022 17:15:07 +0200, Borislav Petkov wrote:

> From: Borislav Petkov <[email protected]>
>
> Fix
>
> drivers/scsi/aacraid/commsup.c: In function ‘aac_handle_sa_aif’:
> drivers/scsi/aacraid/commsup.c:1983:2: error: case label does not reduce to an integer constant
> case SA_AIF_BPCFG_CHANGE:
> ^~~~
>
> [...]

Applied to 5.19/scsi-queue, thanks!

[01/11] scsi: aacraid: Fix undefined behavior due to shift overflowing the constant
https://git.kernel.org/mkp/scsi/c/331c6e910f1a

--
Martin K. Petersen Oracle Linux Engineering

2022-05-18 04:54:22

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant



On 4/5/22 08:15, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Fix:
>
> In file included from <command-line>:0:0:
> drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’:
> ././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_1047’ \
> declared with attribute error: FIELD_PREP: mask is not constant
> _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>
> and other build errors due to shift overflowing values.
>
> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> details as to why it triggers with older gccs only.
>

Acked-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]>

Is this merged anywhere?
It could/should at least be in linux-next so that other people
don't waste time on it.

thanks.

> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Joonas Lahtinen <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: Tvrtko Ursulin <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +-
> .../i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +-
> .../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +-
> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 18 +++++++++---------
> 5 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index 7afdadc7656f..e835f28c0020 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -50,7 +50,7 @@
>
> #define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u)
> #define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ GUC_HXG_REQUEST_MSG_0_DATA0
> -#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xffff << 16)
> +#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xffffU << 16)
> #define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN (0xffff << 0)
> #define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32 GUC_HXG_REQUEST_MSG_n_DATAn
> #define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64 GUC_HXG_REQUEST_MSG_n_DATAn
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> index c9086a600bce..df83c1cc7c7a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> @@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64);
> #define GUC_CTB_HDR_LEN 1u
> #define GUC_CTB_MSG_MIN_LEN GUC_CTB_HDR_LEN
> #define GUC_CTB_MSG_MAX_LEN 256u
> -#define GUC_CTB_MSG_0_FENCE (0xffff << 16)
> +#define GUC_CTB_MSG_0_FENCE (0xffffU << 16)
> #define GUC_CTB_MSG_0_FORMAT (0xf << 12)
> #define GUC_CTB_FORMAT_HXG 0u
> #define GUC_CTB_MSG_0_RESERVED (0xf << 8)
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> index 29ac823acd4c..7d5ba4d97d70 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> @@ -40,7 +40,7 @@
> */
>
> #define GUC_HXG_MSG_MIN_LEN 1u
> -#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31)
> +#define GUC_HXG_MSG_0_ORIGIN (0x1U << 31)
> #define GUC_HXG_ORIGIN_HOST 0u
> #define GUC_HXG_ORIGIN_GUC 1u
> #define GUC_HXG_MSG_0_TYPE (0x7 << 28)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> index 66027a42cda9..ad570fa002a6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> @@ -28,7 +28,7 @@
> #define GS_MIA_HALT_REQUESTED (0x02 << GS_MIA_SHIFT)
> #define GS_MIA_ISR_ENTRY (0x04 << GS_MIA_SHIFT)
> #define GS_AUTH_STATUS_SHIFT 30
> -#define GS_AUTH_STATUS_MASK (0x03 << GS_AUTH_STATUS_SHIFT)
> +#define GS_AUTH_STATUS_MASK (0x03U << GS_AUTH_STATUS_SHIFT)
> #define GS_AUTH_STATUS_BAD (0x01 << GS_AUTH_STATUS_SHIFT)
> #define GS_AUTH_STATUS_GOOD (0x02 << GS_AUTH_STATUS_SHIFT)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3c87d77d2cf6..f3ba3d0a430b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7555,19 +7555,19 @@ enum skl_power_gate {
> #define PORT_CLK_SEL_LCPLL_810 (2 << 29)
> #define PORT_CLK_SEL_SPLL (3 << 29)
> #define PORT_CLK_SEL_WRPLL(pll) (((pll) + 4) << 29)
> -#define PORT_CLK_SEL_WRPLL1 (4 << 29)
> -#define PORT_CLK_SEL_WRPLL2 (5 << 29)
> -#define PORT_CLK_SEL_NONE (7 << 29)
> -#define PORT_CLK_SEL_MASK (7 << 29)
> +#define PORT_CLK_SEL_WRPLL1 (4U << 29)
> +#define PORT_CLK_SEL_WRPLL2 (5U << 29)
> +#define PORT_CLK_SEL_NONE (7U << 29)
> +#define PORT_CLK_SEL_MASK (7U << 29)
>
> /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */
> #define DDI_CLK_SEL(port) PORT_CLK_SEL(port)
> #define DDI_CLK_SEL_NONE (0x0 << 28)
> -#define DDI_CLK_SEL_MG (0x8 << 28)
> -#define DDI_CLK_SEL_TBT_162 (0xC << 28)
> -#define DDI_CLK_SEL_TBT_270 (0xD << 28)
> -#define DDI_CLK_SEL_TBT_540 (0xE << 28)
> -#define DDI_CLK_SEL_TBT_810 (0xF << 28)
> +#define DDI_CLK_SEL_MG (0x8U << 28)
> +#define DDI_CLK_SEL_TBT_162 (0xCU << 28)
> +#define DDI_CLK_SEL_TBT_270 (0xDU << 28)
> +#define DDI_CLK_SEL_TBT_540 (0xEU << 28)
> +#define DDI_CLK_SEL_TBT_810 (0xFU << 28)
> #define DDI_CLK_SEL_MASK (0xF << 28)
>
> /* Transcoder clock selection */

--
~Randy

2022-05-18 07:47:46

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant

On Tue, May 17, 2022 at 04:05:46PM -0700, Randy Dunlap wrote:
>
>
> On 4/5/22 08:15, Borislav Petkov wrote:
> > From: Borislav Petkov <[email protected]>
> >
> > Fix:
> >
> > In file included from <command-line>:0:0:
> > drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’:
> > ././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_1047’ \
> > declared with attribute error: FIELD_PREP: mask is not constant
> > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >
> > and other build errors due to shift overflowing values.
> >
> > See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> > details as to why it triggers with older gccs only.
> >
>
> Acked-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]>
>
> Is this merged anywhere?

It's state is "new" in their patchwork:

https://patchwork.freedesktop.org/patch/480756/

so I guess not yet.

> It could/should at least be in linux-next so that other people
> don't waste time on it.

-ETOOMANYPATCHES I guess. :-\

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-05-18 11:40:02

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant

On Wed, 18 May 2022, Borislav Petkov <[email protected]> wrote:
> On Tue, May 17, 2022 at 04:05:46PM -0700, Randy Dunlap wrote:
>>
>>
>> On 4/5/22 08:15, Borislav Petkov wrote:
>> > From: Borislav Petkov <[email protected]>
>> >
>> > Fix:
>> >
>> > In file included from <command-line>:0:0:
>> > drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’:
>> > ././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_1047’ \
>> > declared with attribute error: FIELD_PREP: mask is not constant
>> > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>> >
>> > and other build errors due to shift overflowing values.
>> >
>> > See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
>> > details as to why it triggers with older gccs only.
>> >
>>
>> Acked-by: Randy Dunlap <[email protected]>
>> Tested-by: Randy Dunlap <[email protected]>
>>
>> Is this merged anywhere?
>
> It's state is "new" in their patchwork:
>
> https://patchwork.freedesktop.org/patch/480756/

Basically we run all patches through CI before merging, and because only
one patch was sent to intel-gfx, the CI just sat waiting for the rest of
the series to arrive...

Anyway, didn't really like the changes in i915_reg.h, sent my version of
that and the rest separately [1].

> so I guess not yet.
>
>> It could/should at least be in linux-next so that other people
>> don't waste time on it.
>
> -ETOOMANYPATCHES I guess. :-\

Yeah, sorry about that.


BR,
Jani.

[1] https://patchwork.freedesktop.org/series/104122/


--
Jani Nikula, Intel Open Source Graphics Center

2022-05-19 18:32:33

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant

On Mon, Apr 25, 2022 at 11:46:53AM -0700, Randy Dunlap wrote:
>
>
> On 4/5/22 08:15, Borislav Petkov wrote:
> > From: Borislav Petkov <[email protected]>
> >
> > Fix:
> >
> > drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’:
> > drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce to an integer constant
> > case R128_PM4_64BM_64VCBM_64INDBM:
> > ^~~~
> > drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce to an integer constant
> > case R128_PM4_64PIO_64VCPIO_64INDPIO:
> > ^~~~
> >
> > See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> > details as to why it triggers with older gccs only.
> >
> > Signed-off-by: Borislav Petkov <[email protected]>
> > Cc: David Airlie <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: Alex Deucher <[email protected]>
> > Cc: [email protected]
>
> Reviewed-by: Randy Dunlap <[email protected]>

Pushed to drm-misc-next, thanks for patch&review.
-Daniel

>
> Thanks.
>
> > ---
> > drivers/gpu/drm/r128/r128_drv.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
> > index 2e1bc01aa5c9..970e192b0d51 100644
> > --- a/drivers/gpu/drm/r128/r128_drv.h
> > +++ b/drivers/gpu/drm/r128/r128_drv.h
> > @@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, unsigned int cmd,
> > # define R128_PM4_64PIO_128INDBM (5 << 28)
> > # define R128_PM4_64BM_128INDBM (6 << 28)
> > # define R128_PM4_64PIO_64VCBM_64INDBM (7 << 28)
> > -# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28)
> > -# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28)
> > +# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28)
> > +# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28)
> > # define R128_PM4_BUFFER_CNTL_NOUPDATE (1 << 27)
> >
> > #define R128_PM4_BUFFER_WM_CNTL 0x0708
>
> --
> ~Randy

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-06-16 16:20:41

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant



On 5/19/22 06:05, Daniel Vetter wrote:
> On Mon, Apr 25, 2022 at 11:46:53AM -0700, Randy Dunlap wrote:
>>
>>
>> On 4/5/22 08:15, Borislav Petkov wrote:
>>> From: Borislav Petkov <[email protected]>
>>>
>>> Fix:
>>>
>>> drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’:
>>> drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce to an integer constant
>>> case R128_PM4_64BM_64VCBM_64INDBM:
>>> ^~~~
>>> drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce to an integer constant
>>> case R128_PM4_64PIO_64VCPIO_64INDPIO:
>>> ^~~~
>>>
>>> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
>>> details as to why it triggers with older gccs only.
>>>
>>> Signed-off-by: Borislav Petkov <[email protected]>
>>> Cc: David Airlie <[email protected]>
>>> Cc: Daniel Vetter <[email protected]>
>>> Cc: Alex Deucher <[email protected]>
>>> Cc: [email protected]
>>
>> Reviewed-by: Randy Dunlap <[email protected]>
>
> Pushed to drm-misc-next, thanks for patch&review.
> -Daniel
>

Hi,

Will this be merged to mainline any time soonish?

thanks.

>>
>> Thanks.
>>
>>> ---
>>> drivers/gpu/drm/r128/r128_drv.h | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
>>> index 2e1bc01aa5c9..970e192b0d51 100644
>>> --- a/drivers/gpu/drm/r128/r128_drv.h
>>> +++ b/drivers/gpu/drm/r128/r128_drv.h
>>> @@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, unsigned int cmd,
>>> # define R128_PM4_64PIO_128INDBM (5 << 28)
>>> # define R128_PM4_64BM_128INDBM (6 << 28)
>>> # define R128_PM4_64PIO_64VCBM_64INDBM (7 << 28)
>>> -# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28)
>>> -# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28)
>>> +# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28)
>>> +# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28)
>>> # define R128_PM4_BUFFER_CNTL_NOUPDATE (1 << 27)
>>>
>>> #define R128_PM4_BUFFER_WM_CNTL 0x0708
>>
>> --
>> ~Randy
>

--
~Randy

2022-06-24 20:35:52

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant

On Thu, Jun 16, 2022 at 09:06:45AM -0700, Randy Dunlap wrote:
>
>
> On 5/19/22 06:05, Daniel Vetter wrote:
> > On Mon, Apr 25, 2022 at 11:46:53AM -0700, Randy Dunlap wrote:
> >>
> >>
> >> On 4/5/22 08:15, Borislav Petkov wrote:
> >>> From: Borislav Petkov <[email protected]>
> >>>
> >>> Fix:
> >>>
> >>> drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’:
> >>> drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce to an integer constant
> >>> case R128_PM4_64BM_64VCBM_64INDBM:
> >>> ^~~~
> >>> drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce to an integer constant
> >>> case R128_PM4_64PIO_64VCPIO_64INDPIO:
> >>> ^~~~
> >>>
> >>> See https://lore.kernel.org/r/YkwQ6%[email protected] for the gory
> >>> details as to why it triggers with older gccs only.
> >>>
> >>> Signed-off-by: Borislav Petkov <[email protected]>
> >>> Cc: David Airlie <[email protected]>
> >>> Cc: Daniel Vetter <[email protected]>
> >>> Cc: Alex Deucher <[email protected]>
> >>> Cc: [email protected]
> >>
> >> Reviewed-by: Randy Dunlap <[email protected]>
> >
> > Pushed to drm-misc-next, thanks for patch&review.
> > -Daniel
> >
>
> Hi,
>
> Will this be merged to mainline any time soonish?

It missed the merge window by a hair, so it's in linux-next and will get
into the next one.
-Daniel

>
> thanks.
>
> >>
> >> Thanks.
> >>
> >>> ---
> >>> drivers/gpu/drm/r128/r128_drv.h | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
> >>> index 2e1bc01aa5c9..970e192b0d51 100644
> >>> --- a/drivers/gpu/drm/r128/r128_drv.h
> >>> +++ b/drivers/gpu/drm/r128/r128_drv.h
> >>> @@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, unsigned int cmd,
> >>> # define R128_PM4_64PIO_128INDBM (5 << 28)
> >>> # define R128_PM4_64BM_128INDBM (6 << 28)
> >>> # define R128_PM4_64PIO_64VCBM_64INDBM (7 << 28)
> >>> -# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28)
> >>> -# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28)
> >>> +# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28)
> >>> +# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28)
> >>> # define R128_PM4_BUFFER_CNTL_NOUPDATE (1 << 27)
> >>>
> >>> #define R128_PM4_BUFFER_WM_CNTL 0x0708
> >>
> >> --
> >> ~Randy
> >
>
> --
> ~Randy

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch