uiomove() might copy more than uio->uio

Stefan Kempf

2016-02-06 11:47:25 UTC

Post by Martin Natano
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.

Post by Martin Natano
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