PCI: Test for bit underflow in pcie_set_readrq()
Subsystems (3)
PATCH 1/1 PCI: Test for bit underflow in pcie_set_readrq()
After commit cbc654d18d37 ("bitops: Add __attribute_const__ to generic ffs()-family implementations"), which allows GCC's value range tracker to see past ffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:
v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);
could wrap below 0, leading to a very large value, which would be out of range for the FIELD_PREP() usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error: FIELD_PREP: value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'
v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);
^~~~~~~~~~If the result of the ffs() is bounds checked before being used in FIELD_PREP(), the value tracker seems happy again. :)
---
Cc: Bjorn Helgaas <[email protected]>
Cc: Anders Roxell <[email protected]>
Cc: Naresh Kamboju <[email protected]>
Cc: [email protected]
Cc: Linux Regressions <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Ben Copeland <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
--- drivers/pci/pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/pci/pci.c +5 -1
@@ -5932,6 +5932,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) {u16 v;int ret; +unsigned int firstbit;struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus);if (rq <128 || rq >4096 || !is_power_of_2 (rq))
@@ -5949,7 +5950,10 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) rq = mps; } - v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ ,ffs (rq) -8 ); + firstbit =ffs (rq); +if (firstbit <8 ) +return -EINVAL ; + v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ , firstbit -8 );if (bridge->no_inc_mrrs) {int max_mrrs =pcie_get_readrq (dev);
Signature · 1 line
2.34.1
Quoted 54 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP: value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to generic ffs()-family implementations")Closes https://lore.kernel.org/linux-pci/CA+G9fYuysVr6qT8bjF6f08WLyCJRG7aXAeSd2F7=zTaHHd7L+Q@mail.gmail.com/ open hereSigned-off-by Kees Cook <[email protected]>---
Cc: Bjorn Helgaas <[email protected]>
Cc: Anders Roxell <[email protected]>
Cc: Naresh Kamboju <[email protected]>
Cc: [email protected]
Cc: Linux Regressions <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Ben Copeland <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>--- drivers/pci/pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/pci/pci.c +1
-0@@ -5932,6 +5932,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) {u16 v;int ret; +unsigned int firstbit;struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus);if (rq <128 || rq >4096 || !is_power_of_2 (rq))
drivers/pci/pci.c int pcie_set_readrq(struct pci_dev *dev, int rq) · line 5956
Quoted 8 lines
drivers/pci/pci.c +4
-1@@ -5949,7 +5950,10 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) rq = mps; } - v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ ,ffs (rq) -8 ); + firstbit =ffs (rq); +if (firstbit <8 ) +return -EINVAL ; + v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ , firstbit -8 );
Hi Kees,
Thank you for looking into this.
These warnings are not a one time thing. the later versions of gcc can figure it
out that firstbit is at least 8 based on the "rq < 128" (i guess), so we're adding
bogus code. maybe we should just disable the check for gcc-8.
Maybe something like this:
include/linux/bitfield.h +1 -3
@@ -65,9 +65,20 @@BUILD_BUG_ON_MSG (!__builtin_constant_p (_mask), \ _pfx"mask is not constant" ); \BUILD_BUG_ON_MSG ((_mask) ==0 , _pfx"mask is zero" ); \ - BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \ - ~((_mask) >> __bf_shf(_mask)) & \ - (0 + (_val)) : 0, \ +/* Value validation disabled for gcc < 9 due to
__attribute_const__ issues.
+ */ \
+ BUILD_BUG_ON_MSG(__GNUC__ >= 9 &&__builtin_constant_p(_val) ? \
+ ~((_mask) >> __bf_shf(_mask)) &
\
+ (0 + (_val)) : 0,
\
_pfx "value too large for the field"); \
BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
__bf_cast_unsigned(_reg, ~0ull), \I found similar patterns with ffs and FIELD_PREP here drivers/dma/uniphier-xdmac.c row 156 and 165 drivers/gpu/drm/i915/display/intel_cursor_regs.h row 17
Cheers,
Anders
Quoted 22 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP:
value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)
Quoted 6 lines
Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to genericffs()-family implementations")
Signed-off-by Kees Cook <[email protected]>
This looks good to me individually, however I have now tried to do more randconfig tests with the __attribute_const change and gcc-8.5.0, and so far found two other files with the same issue:
In file included from <command-line>:
In function 'mtk_dai_etdm_out_configure.constprop',
inlined from 'mtk_dai_etdm_configure' at sound/soc/mediatek/mt8188/mt8188-dai-etdm.c:2168:3:
include/linux/compiler_types.h:575:38: error: call to '__compiletime_assert_416' declared with attribute error: FIELD_PREP: value too large for the field sound/soc/mediatek/mt8188/mt8188-dai-etdm.c:2065:10: note: in expansion of macro 'FIELD_PREP'
val |= FIELD_PREP(ETDM_OUT_CON4_FS_MASK, get_etdm_fs_timing(rate));sound/soc/mediatek/mt8188/mt8188-dai-etdm.c:1971:10: note: in expansion of macro 'FIELD_PREP'
val |= FIELD_PREP(ETDM_IN_CON3_FS_MASK, get_etdm_fs_timing(rate));
drivers/mmc/host/meson-gx-mmc.c: In function 'meson_mmc_start_cmd':drivers/mmc/host/meson-gx-mmc.c:811:14: note: in expansion of macro 'FIELD_PREP'
cmd_cfg |= FIELD_PREP(CMD_CFG_TIMEOUT_MASK,
This is fairly rare, but over time there are likely going to be others like them. I see three possible ways forward here:
a) fix them individually as we run into them, hoping for the best
b) skip that one compile time check on older compilers, like Anders' new patch
c) try to come up with a more robust FIELD_PREP() implementation
I already tried hacking on FIELD_PREP(), but my feeling is that it is already getting out of hand with its complexity, and needs to become simpler instead. The root cause for the warning here is apparently the way it evaluates the macro arguments multiple times, and that causes both extra compile-time complexity and extra code paths where part of the invocation goes through an inline function and another path that does not. It may be possible to change this in a way that moves the BUILD_BUG_ON() portions into an __always_inline function, and only uses the macro to deal with the type differences.
Arnd
Quoted 13 lines · Anders Roxell
Review int pcie_set_readrq(struct pci_dev *dev, int rq) · line 5956
Quoted 9 lines · Kees Cook
(quoted diff) +4
-1@@ -5949,7 +5950,10 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) rq = mps; } - v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); + firstbit = ffs(rq); + if (firstbit < 8) + return -EINVAL; + v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, firstbit - 8);Hi Kees,
Quoted 7 lines
Thank you for looking into this.
These warnings are not a one time thing. the later versions of gcc can figure it
out that firstbit is at least 8 based on the "rq < 128" (i guess), so we're adding
bogus code. maybe we should just disable the check for gcc-8.
Out of the three failures I saw, two also happened with gcc-9, but gcc-10 looks clean so far.
Quoted 10 lines
\ + (0 + (_val)) : 0, \ _pfx "value too large for the field"); \ BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \ __bf_cast_unsigned(_reg, ~0ull), \I found similar patterns with ffs and
FIELD_PREPheredrivers/dma/uniphier-xdmac.crow 156 and 165drivers/gpu/drm/i915/display/intel_cursor_regs.hrow 17
I did not come across build failures for these.
Arnd
Quoted 14 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP: value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'
Quoted 7 lines
v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to generic ffs()-family implementations")
What's your plan for merging cbc654d18d37? I suppose it's intended for v6.18? If it will appear in v6.17, let me know so I can merge this for it as well.
Maybe this should go in v6.17 regardless, to avoid a warning regression between this patch and cbc654d18d37?
Quoted 46 lines
Closes https://lore.kernel.org/linux-pci/CA+G9fYuysVr6qT8bjF6f08WLyCJRG7aXAeSd2F7=zTaHHd7L+Q@mail.gmail.com/ open hereSigned-off-by Kees Cook <[email protected]>---
Cc: Bjorn Helgaas <[email protected]>
Cc: Anders Roxell <[email protected]>
Cc: Naresh Kamboju <[email protected]>
Cc: [email protected]
Cc: Linux Regressions <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Ben Copeland <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>--- drivers/pci/pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/pci/pci.c +5
-1@@ -5932,6 +5932,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) {u16 v;int ret; +unsigned int firstbit;struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus);if (rq <128 || rq >4096 || !is_power_of_2 (rq))@@ -5949,7 +5950,10 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) rq = mps; } - v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ ,ffs (rq) -8 ); + firstbit =ffs (rq); +if (firstbit <8 ) +return -EINVAL ; + v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ , firstbit -8 );if (bridge->no_inc_mrrs) {int max_mrrs =pcie_get_readrq (dev);Signature · 1 line
2.34.1
Quoted 18 lines · Bjorn Helgaas
Quoted 17 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP: value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~
Quoted 8 lines
Quoted 4 lines
If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to generic ffs()-family implementations")What's your plan for merging
cbc654d18d37? I suppose it's intended for v6.18? If it will appear in v6.17, let me know so I can merge this for it as well.
I had it planned for v6.18.
Quoted 2 lines
Maybe this should go in v6.17 regardless, to avoid a warning regression between this patch and
cbc654d18d37?
Sure, or I could take it as part of the ffs series?
-Kees
Signature · 1 line
Kees Cook
Quoted 8 lines · Anders Roxell
Review
Quoted 5 lines
(quoted diff) +4
-1- v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); + firstbit = ffs(rq); + if (firstbit < 8) + return -EINVAL; + v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, firstbit - 8);Hi Kees,
Quoted 7 lines
Thank you for looking into this.
These warnings are not a one time thing. the later versions of gcc can figure it
out that firstbit is at least 8 based on the "rq < 128" (i guess), so we're adding
bogus code. maybe we should just disable the check for gcc-8.
I think the issue is that GCC thinks it knows the range for ffs is not the entire [0..UINT_MAX], but it _doesn't_ know how "rq" affects the outcome. (The range checker warnings kick in when it's not the whole range of a given type.) But I am just guessing, based on what how I've seen in behave in the past.
Quoted 26 lines
Maybe something like this:
include/linux/bitfield.h +1
-3@@ -65,9 +65,20 @@BUILD_BUG_ON_MSG (!__builtin_constant_p (_mask), \ _pfx"mask is not constant" ); \BUILD_BUG_ON_MSG ((_mask) ==0 , _pfx"mask is zero" ); \ - BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \ - ~((_mask) >> __bf_shf(_mask)) & \ - (0 + (_val)) : 0, \ +/* Value validation disabled for gcc < 9 due to
__attribute_const__issues.include/linux/bitfield.h +2
-0+*/ \ + BUILD_BUG_ON_MSG(__GNUC__ >= 9 &&
__builtin_constant_p(_val)? \include/linux/bitfield.h +2
-0+ ~((_mask) >>__bf_shf (_mask)) & \ + (0 + (_val)) : 0, \ _pfx "value too large for the field"); \ BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \ __bf_cast_unsigned(_reg, ~0ull), \
Quoted 3 lines
I found similar patterns with ffs and
FIELD_PREPheredrivers/dma/uniphier-xdmac.crow 156 and 165drivers/gpu/drm/i915/display/intel_cursor_regs.hrow 17
You got warnings for these?
Signature · 1 line
Kees Cook
Quoted 46 lines · Arnd Bergmann
Quoted 22 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP:
value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)Quoted 6 lines
Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to genericffs()-family implementations")
Signed-off-by Kees Cook <[email protected]>Acked-by Arnd Bergmann <[email protected]>This looks good to me individually, however I have now tried to do more randconfig tests with the
__attribute_constchange and gcc-8.5.0, and so far found two other files with the same issue:In file included from <command-line>:
In function 'mtk_dai_etdm_out_configure.constprop',inlined from 'mtk_dai_etdm_configure' at sound/soc/mediatek/mt8188/mt8188-dai-etdm.c:2168:3:
include/linux/compiler_types.h:575:38: error: call to '__compiletime_assert_416' declared with attribute error:FIELD_PREP: value too large for the fieldsound/soc/mediatek/mt8188/mt8188-dai-etdm.c:2065:10: note: in expansion of macro 'FIELD_PREP'
val |=FIELD_PREP(ETDM_OUT_CON4_FS_MASK, get_etdm_fs_timing(rate));sound/soc/mediatek/mt8188/mt8188-dai-etdm.c:1971:10: note: in expansion of macro 'FIELD_PREP'
val |=FIELD_PREP(ETDM_IN_CON3_FS_MASK, get_etdm_fs_timing(rate));
Quoted 3 lines
drivers/mmc/host/meson-gx-mmc.c: In function 'meson_mmc_start_cmd':drivers/mmc/host/meson-gx-mmc.c:811:14: note: in expansion of macro 'FIELD_PREP'
cmd_cfg|= FIELD_PREP(CMD_CFG_TIMEOUT_MASK,
I can't see how these are related to ffs()...
Quoted 4 lines
This is fairly rare, but over time there are likely going to be others like them. I see three possible ways forward here:
a) fix them individually as we run into them, hoping for the best
I think they are valid warnings, so my instinct would be to fix them as they appear. (e.g. "0 - 8" isn't something FIELD_PREP can do anything about.)
-Kees
Signature · 1 line
Kees Cook
Quoted 17 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP: value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~
Quoted 7 lines
If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to generic ffs()-family implementations")Closes https://lore.kernel.org/linux-pci/CA+G9fYuysVr6qT8bjF6f08WLyCJRG7aXAeSd2F7=zTaHHd7L+Q@mail.gmail.com/ open hereSigned-off-by Kees Cook <[email protected]>
Would be great if you included this as part of your series, thanks!
Quoted 43 lines
---
Cc: Bjorn Helgaas <[email protected]>
Cc: Anders Roxell <[email protected]>
Cc: Naresh Kamboju <[email protected]>
Cc: [email protected]
Cc: Linux Regressions <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Ben Copeland <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>--- drivers/pci/pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/pci/pci.c +5
-1@@ -5932,6 +5932,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) {u16 v;int ret; +unsigned int firstbit;struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus);if (rq <128 || rq >4096 || !is_power_of_2 (rq))@@ -5949,7 +5950,10 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) rq = mps; } - v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ ,ffs (rq) -8 ); + firstbit =ffs (rq); +if (firstbit <8 ) +return -EINVAL ; + v =FIELD_PREP (PCI_EXP_DEVCTL_READRQ , firstbit -8 );if (bridge->no_inc_mrrs) {int max_mrrs =pcie_get_readrq (dev);Signature · 1 line
2.34.1
Quoted 21 lines · Bjorn Helgaas
Quoted 20 lines · Kees Cook
After commit
cbc654d18d37("bitops: Add__attribute_const__to generic ffs()-family implementations"), which allows GCC's value range tracker to see pastffs(), GCC 8 on ARM thinks that it might be possible that "ffs(rq) - 8" used here:v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);could wrap below 0, leading to a very large value, which would be out of range for the
FIELD_PREP()usage:
drivers/pci/pci.c: In function 'pcie_set_readrq':include/linux/compiler_types.h:572:38: error: call to '__compiletime_assert_471' declared with attribute error:FIELD_PREP: value too large for the field
...drivers/pci/pci.c:5896:6: note: in expansion of macro 'FIELD_PREP'v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); ^~~~~~~~~~If the result of the
ffs()is bounds checked before being used inFIELD_PREP(), the value tracker seems happy again. :)
Quoted 8 lines
Quoted 4 lines
Fixes cbc654d18d37 ("bitops: Add __attribute_const__ to generic ffs()-family implementations")Closes https://lore.kernel.org/linux-pci/CA+G9fYuysVr6qT8bjF6f08WLyCJRG7aXAeSd2F7=zTaHHd7L+Q@mail.gmail.com/ open hereSigned-off-by Kees Cook <[email protected]>Acked-by Bjorn Helgaas <[email protected]>Would be great if you included this as part of your series, thanks!
Okay, thanks! :)
-Kees
Signature · 1 line
Kees Cook