On Mon, 17 Jul 2023 at 22:35, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > Coverity points out that in page_table_walk_refill() we can shift by > a negative number, which is undefined behaviour (CID 1452918, > 1452920, 1452922). We already catch the negative directory_shift and > leaf_shift as being a "bail out early" case, but not until we've > already used them to calculated some offset values. > > Move the calculation of the offset values to after we've done the > "return early if ptew > 1" check. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > [PMD: Check for ptew > 1, use unsigned type] > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
I think I would expand the commit message a bit, so instead of the current paragraph 2, something like: The shifts can be negative only if ptew > 1, so make the bail-out-early check look directly at that, and only calculate the shift amounts and the offsets based on them after we have done that check. This allows us to simplify the expressions used to calculate the shift amounts, use an unsigned type, and avoids the undefined behaviour. ? Anyway Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM
