[PATCH 0/2] fs: __read/__write: fix EOF checks for negative file sizes

[PATCH 0/2] fs: __read/__write: fix EOF checks for negative file sizes

[Stefan Kerkmann]

Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
Stefan Kerkmann (2):
      fs: __read: fix EOF count clamping for negative file sizes
      fs: __write: fix EOF growth checks for negative file sizes

 fs/fs.c | 54 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 13 deletions(-)
---
base-commit: a59cb95e1133c7f4844c68398c570694d1c7d9ed
change-id: 20260903-fix-fs_read_write_eof_checks-3393cd6d42bd

Best regards,
--  
Stefan Kerkmann <s.kerkmann@pengutronix.de>

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

[Stefan Kerkmann]

__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;
+		}
+		if (f->f_pos > f->f_size)
+			count = 0;
+		else
+			count = min_t(u64, (u64)f->f_size - (u64)f->f_pos, count);
+	}
 
 	if (!count)
 		return 0;

-- 
2.47.3

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

[Sascha Hauer]

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 |

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

[Stefan Kerkmann]

Hi Sascha,

On 9/4/26 11:20, Sascha Hauer wrote:
> 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.
> 

Agreed, that is cleaner and better. Depending on whether you incorporate the
checks into your series I can send another series that builds on top of yours.

> 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 |
> 

Best regards,
Stefan

-- 
Pengutronix e.K.                       | Stefan Kerkmann             |
Steuerwalder Str. 21                   | https://www.pengutronix.de/ |
31137 Hildesheim, Germany              | Phone: +49-5121-206917-128  |
Amtsgericht Hildesheim, HRA 2686       | Fax:   +49-5121-206917-9    |

[PATCH 2/2] fs: __write: fix EOF growth checks for negative file sizes

[Stefan Kerkmann]

__write() grows the file when the write end would pass EOF, but based
the decision on the same target dependent mixed sign comparison that
__read() had: f->f_pos + count > f->f_size compares the signed 64-bit
position and size (loff_t) against count (size_t), which is 32-bit on
32-bit arches and 64-bit on 64-bit arches. For negative file sizes
except for the FILE_SIZE_STREAM sentinel this is broken:

- 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 evaluated true.
  fsdev_truncate() was then called for the corrupted file size,
  attempting to grow the file to f->f_pos + count.

- On 64-bit arches size_t cannot be represented by signed 64-bit, so the
  usual arithmetic conversions turned the whole comparison unsigned:
  f->f_size = -512 was reinterpreted as a value near 2^64, the
  comparison stayed false, the file was never grown and the write
  proceeded unclamped against the bogus size.

Additionally, when writing at a position past the end of the file
(reachable via pwrite() with a large offset) and fsdev_truncate() failed
with -ENOSPC, the fallback count f->f_size - f->f_pos was negative and
wrapped to a huge value in the unsigned count.

__write() now rejects negative file sizes with -EINVAL, like __read()
does. The write end f->f_pos + count is computed in u64, making the
growth check target independent, and extending the file beyond
MAX_LFS_FILESIZE is rejected with -EFBIG. On -ENOSPC the write is now
limited to the bytes remaining until EOF, or aborted when f->f_pos is at
or past the end of the file, instead of wrapping the negative remainder.

Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
 fs/fs.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index a8f2b78294..3803decc2a 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -482,7 +482,10 @@ EXPORT_SYMBOL(read);
 
 static ssize_t __write(struct file *f, const void *buf, size_t count)
 {
+	u64 size = (u64)f->f_size;
+	u64 pos = (u64)f->f_pos;
 	struct fs_driver *fsdrv;
+	u64 end;
 	int ret;
 
 	fsdrv = f->fsdev->driver;
@@ -495,18 +498,35 @@ static ssize_t __write(struct file *f, const 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) {
-		ret = fsdev_truncate(f, f->f_pos + count);
-		if (ret) {
-			if (ret == -EPERM)
-				ret = -ENOSPC;
-			if (ret != -ENOSPC)
+	if (f->f_size != FILE_SIZE_STREAM) {
+		if (f->f_size < 0) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		/* Writing past the end of the file requires growing it first */
+		end = pos + count;
+		if (end > size) {
+			/* New file size must be representable as loff_t */
+			if (end > (u64)MAX_LFS_FILESIZE ||
+			    (f->f_pos >= 0 && end < pos)) {
+				ret = -EFBIG;
 				goto out;
-			count = f->f_size - f->f_pos;
-			if (!count)
-				goto out;
-		} else {
-			f->f_size = f->f_pos + count;
+			}
+
+			ret = fsdev_truncate(f, end);
+			if (ret) {
+				if (ret == -EPERM)
+					ret = -ENOSPC;
+				if (ret != -ENOSPC)
+					goto out;
+				/* Truncate failed; write what fits into the file */
+				count = pos < size ? size - pos : 0;
+				if (!count)
+					goto out;
+			} else {
+				f->f_size = end;
+			}
 		}
 	}
 

-- 
2.47.3