Re: [PATCH 2/2] pci: Sanity check mask argument to pci_set_*_by_mask()

[Richard Henderson]

On 7/26/22 09:32, Peter Maydell wrote:

Coverity complains that in functions like pci_set_word_by_mask()
we might end up shifting by more than 31 bits. This is true,
but only if the caller passes in a zero mask. Help Coverity out
by asserting that the mask argument is valid.

Fixes: CID 1487168

Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
Note that only 1 of these 4 functions is used, and that only
in 2 places in the codebase. In both cases the mask argument
is a compile-time constant.
---
  include/hw/pci/pci.h | 20 ++++++++++++++++----
  1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c79144bc5ef..0392b947b8b 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -688,7 +688,10 @@ static inline void
  pci_set_byte_by_mask(uint8_t *config, uint8_t mask, uint8_t reg)
  {
      uint8_t val = pci_get_byte(config);
-    uint8_t rval = reg << ctz32(mask);
+    uint8_t rval;
+
+    assert(mask & 0xff);

Why the and, especially considering the uint8_t type?

@@ -696,7 +699,10 @@ static inline void
  pci_set_word_by_mask(uint8_t *config, uint16_t mask, uint16_t reg)
  {
      uint16_t val = pci_get_word(config);
-    uint16_t rval = reg << ctz32(mask);
+    uint16_t rval;
+
+    assert(mask & 0xffff);

Similarly.

Otherwise,
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>


r~