Re: [PATCH] 9p/net: fix improper handling of bogus negative read/write replies

[Dominique Martinet]

Christian Schoenebeck wrote on Sun, Dec 22, 2024 at 04:29:58PM +0100:
> > @@ -1592,11 +1593,11 @@ p9_client_read_once(struct p9_fid *fid, u64 offset, struct iov_iter *to,
> >  		return 0;
> >  	}
> >  	if (rsize < received) {
> > -		pr_err("bogus RREAD count (%d > %d)\n", received, rsize);
> > +		pr_err("bogus RREAD count (%u > %u)\n", received, rsize);
> >  		received = rsize;
> 
> Does `received = rsize` make sense here? I would rather do `received = 0` to
> prevent copying garbage below, that would be ignored by caller on error case
> anyway.

Good point.. We're not setting *err here so this isn't an error (and
thus not ignored), so I guess it sort of makes sense if you look at it
from a "best effort" point of view, but I agree a bogus reply probably
better return an error.

OTOH it's a change of behaviour so I'd rather have it in another patch,
will send separately.

> > @@ -1623,9 +1624,9 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
> >  	*err = 0;
> >  
> >  	while (iov_iter_count(from)) {
> > -		int count = iov_iter_count(from);
> > -		int rsize = fid->iounit;
> > -		int written;
> > +		size_t count = iov_iter_count(from);
> > +		size_t rsize = fid->iounit;
> 
> I think that would break 64-bit big-endian systems, as `rsize` is passed via
> format below as "d" (32-bit) type.

Good catch, thanks!
-- 
Dominique Martinet | Asmadeus