Re: [PATCH 1/2] fs: __read: fix EOF count clamping for negative file sizes

["Sascha Hauer" <s.hauer@pengutronix.de>]

Hi Stefan,

On 2026-09-03 18:29, Stefan Kerkmann wrote:
> __read() clamps count to the bytes remaining until EOF, but compared
> the signed 64-bit f->f_pos/f->f_size (loff_t) against count (size_t),
> which is 32-bit on 32-bit arches and 64-bit on 64-bit arches.
> 
> This made the comparison type targent dependent, breaking it for
> negative file sizes except for the FILE_SIZE_STREAM sentinel.
> 
> - On 32-bit arches count is converted to the signed 64-bit type of
>   f->f_pos, so for e.g. f->f_size = -512 the comparison f->f_pos + count
>   > f->f_size evaluated true. The clamp then assigned the negative
>   difference of f->f_size - f->f_pos to the unsigned count, wrapping it
>   to a value near 2^32 and turning a small read into a huge out of
>   bounds read request.
> 
> - On 64-bit arches size_t cannot be represented by signed 64-bit, so the
>   arithmetic C conversions turned the whole comparison unsigned:
>   f->f_size = -512 was reinterpreted as a value near 2^64, the
>   comparison stayed false and the clamp never ran, leaving count
>   unclamped and the bogus size undetected.
> 
> __read() now rejects negative file sizes (except for the
> FILE_SIZE_STREAM sentinel) with -EINVAL. Reads at or past the end of the
> file (reachable via pread() with an offset beyond EOF) now return 0.
> Remaining reads are clamped to the bytes left until EOF.
> 
> Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
> ---
>  fs/fs.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index dc6c30802d..a8f2b78294 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -427,8 +427,16 @@ static ssize_t __read(struct file *f, void *buf, size_t count)
>  	if (fsdrv != ramfs_driver)
>  		assert_command_context();
>  
> -	if (f->f_size != FILE_SIZE_STREAM && f->f_pos + count > f->f_size)
> -		count = f->f_size - f->f_pos;
> +	if (f->f_size != FILE_SIZE_STREAM) {
> +		if (f->f_size < 0) {
> +			ret = -EINVAL;
> +			goto out;
> +		}

I think we should start by rejecting negative file sizes at open time as
done in the patch I just sent. This is likely not the full solution to
the problem as f_pos + count could still become negative, but we
shouldn't allow to even open a file with negative file size.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |