Re: [PATCH] libnvdimm: Handle negative return from nvdimm_clear_poison() in nsio_rw_bytes()

[Dave Jiang]

On 10/2/26 9:56 AM, Serhat Kumral wrote:
> When nvdimm_clear_poison() fails, it returns a negative error code.
> However, 'cleared' is a signed long while 'size' is size_t.
> The comparison 'cleared < size' promotes 'cleared' to an unsigned long.
> So the error condition can be bypassed and the function can
> incorrectly return success if the subsequent flush succeeds.
> 
> Explicitly check for negative error codes before comparing with 'size'.
> 
> Fixes: 868f036fee4b ("libnvdimm: fix mishandled nvdimm_clear_poison() return value")
> Assisted-by: LLM
> Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> Closes: https://lore.kernel.org/all/90d3d353-28e9-4f6d-b141-a9b7157d5514@moroto.mountain/
> Signed-off-by: Serhat Kumral <serhatkumral1@xxxxxxxxx>

Makes sense
Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
>  drivers/nvdimm/claim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
> index 309cd2cddb0e..2725dceaf938 100644
> --- a/drivers/nvdimm/claim.c
> +++ b/drivers/nvdimm/claim.c
> @@ -263,7 +263,7 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns,
>  			might_sleep();
>  			cleared = nvdimm_clear_poison(&ndns->dev,
>  					nsio->res.start + offset, size);
> -			if (cleared < size)
> +			if (cleared < 0 || cleared < size)
>  				rc = -EIO;
>  			if (cleared > 0 && cleared / 512) {
>  				cleared /= 512;