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 <[email protected]> > --- > 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 |
