Martin Natano wrote:
> The uiomove() manual page states that "The lesser of n or uio->uio_resid
> bytes are copied." However, this is only true, when uio_resid ends up on
> an iovec boundary. Otherwise more bytes may be copied, even if they
> exceed uio_resid. The patch below changes uiomove() to actually never
> copy more than uio_resid bytes, as described in the manual.
>
> Also, uiomove() assumes that the iovecs supplied to the function can
> accomodate the requested size, which is a reasonable assumption. But
> still, a KASSERT() might be a good idea to detect a mismatch as early
> as possible.
>
> While there I removed some diagnostic code outside of #ifdef DIAGNOSTIC.
Yes, it makes sense to make uiomove() behave as the manual says.
And I've seen code that manipulates the uio_resid fields outside of
uiomove (and then calls uiomove later; soreceive for example).
That means we can't enforce or always be sure that
uio_resid == the sum of all iov_len.
So the diff looks good to me.
> Index: kern/kern_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_subr.c,v
> retrieving revision 1.45
> diff -u -p -u -r1.45 kern_subr.c
> --- kern/kern_subr.c 11 Dec 2015 16:07:02 -0000 1.45
> +++ kern/kern_subr.c 10 Jan 2016 11:10:38 -0000
> @@ -51,20 +51,22 @@ uiomove(void *cp, size_t n, struct uio *
> struct iovec *iov;
> size_t cnt;
> int error = 0;
> - struct proc *p;
> -
> - p = uio->uio_procp;
>
> #ifdef DIAGNOSTIC
> if (uio->uio_rw != UIO_READ && uio->uio_rw != UIO_WRITE)
> panic("uiomove: mode");
> - if (uio->uio_segflg == UIO_USERSPACE && p != curproc)
> + if (uio->uio_segflg == UIO_USERSPACE && uio->uio_procp != curproc)
> panic("uiomove: proc");
> #endif
> - while (n > 0 && uio->uio_resid) {
> +
> + if (n > uio->uio_resid)
> + n = uio->uio_resid;
> +
> + while (n > 0) {
> iov = uio->uio_iov;
> cnt = iov->iov_len;
> if (cnt == 0) {
> + KASSERT(uio->uio_iovcnt > 0);
> uio->uio_iov++;
> uio->uio_iovcnt--;
> continue;
>
> cheers,
> natano
>
