Received-SPF: pass (google.com: domain of linux-kernel+bounces-54591-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 6 Feb 2024 09:30:21 +0000
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Joy Chakraborty <joychakr@google.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	linux-kernel@vger.kernel.org, manugautam@google.com,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] nvmem: rmem: Fix return value of rmem_read()
Message-ID: <2024020647-submarine-lucid-ea7b@gregkh>
References: <20240206042408.224138-1-joychakr@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240206042408.224138-1-joychakr@google.com>

On Tue, Feb 06, 2024 at 04:24:08AM +0000, Joy Chakraborty wrote:
> reg_read() callback registered with nvmem core expects an integer error
> as a return value but rmem_read() returns the number of bytes read, as a
> result error checks in nvmem core fail even when they shouldn't.
> 
> Return 0 on success where number of bytes read match the number of bytes
> requested and a negative error -EINVAL on all other cases.
> 
> Fixes: 5a3fa75a4d9c ("nvmem: Add driver to expose reserved memory as nvmem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joy Chakraborty <joychakr@google.com>
> ---
>  drivers/nvmem/rmem.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvmem/rmem.c b/drivers/nvmem/rmem.c
> index 752d0bf4445e..a74dfa279ff4 100644
> --- a/drivers/nvmem/rmem.c
> +++ b/drivers/nvmem/rmem.c
> @@ -46,7 +46,12 @@ static int rmem_read(void *context, unsigned int offset,
>  
>  	memunmap(addr);
>  
> -	return count;
> +	if (count != bytes) {
> +		dev_err(priv->dev, "Failed read memory (%d)\n", count);
> +		return -EINVAL;

Why is a "short read" somehow illegal here?  What internal changes need
to be made now that this has changed?

And what will userspace do with this error message in the kernel log?

thanks,

greg k-h