Re: [PATCH-for-9.0? 3/3] hw/block/nand: Fix out-of-bound access in NAND block buffer

[Richard Henderson]

On 4/7/24 22:36, Philippe Mathieu-Daudé wrote:
> nand_command() and nand_getio() don't check @offset points
> into the block, nor the available data length (s->iolen) is
> not negative.
>
> In order to fix:
>
> - check the offset is in range in nand_blk_load_NAND_PAGE_SIZE(),
> - do not set @iolen if blk_load() failed.

Do not set, or do not set to non-zero?  I had been wondering if the final assignment to 
s->iolen should go into nand_load_block as well...

> diff --git a/hw/block/nand.c b/hw/block/nand.c
> index 3627c799b5..d90dc965a1 100644
> --- a/hw/block/nand.c
> +++ b/hw/block/nand.c
> @@ -255,7 +255,9 @@ static int nand_load_block(NANDFlashState *s, int offset)
>   {
>       int iolen;
>   
> -    s->blk_load(s, s->addr, offset);
> +    if (!s->blk_load(s, s->addr, offset)) {
> +        return 0;
> +    }
>   
>       iolen = (1 << s->page_shift) - offset;
>       if (s->gnd) {
> @@ -780,6 +782,10 @@ static bool glue(nand_blk_load_, 
> NAND_PAGE_SIZE)(NANDFlashState *s,
>           return false;
>       }
>   
> +    if (offset > NAND_PAGE_SIZE + OOB_SIZE) {
> +        return false;
> +    }
> +

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~