Martin Natano wrote:
> Below a diff to convert tmpfs/tmpfs_{subr,vnops}.c. That's an easy one.
> Nearly all the relevant size variables already are a size_t, or a
> smaller unsigned type. In the last hunk tn_size (an off_t) might be
> passed to uiomove(), which is not problematic, because tn_size is
> constrained to only hold values which are also representable by a
> size_t.
Diff reads good to me and reviewed myself that tn_size is always >= 0.
I'm a bit uneasy though with passing signed values as-is to uiomove().
Can we somehow make it explicit that we know that the uiomove() argument is
>= 0?
Changing types all over the place would be too much churn though.
I'm leaning towards an explicit size_t cast for signed size arguments as
an annotation, e.g.
uiomove(buf, (size_t)signed_value, uio);
And a check in uiomove(). If a negative value gets passed in by
accident, it will be caught.
Thoughts?
Index: kern_subr.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_subr.c,v
retrieving revision 1.45
diff -u -p -U10 -r1.45 kern_subr.c
--- kern_subr.c 11 Dec 2015 16:07:02 -0000 1.45
+++ kern_subr.c 17 Jan 2016 10:56:11 -0000
@@ -46,20 +46,23 @@
#include <sys/resourcevar.h>
int
uiomove(void *cp, size_t n, struct uio *uio)
{
struct iovec *iov;
size_t cnt;
int error = 0;
struct proc *p;
+ if (n > SSIZE_MAX)
+ return (EINVAL);
+
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)
panic("uiomove: proc");
#endif
while (n > 0 && uio->uio_resid) {
iov = uio->uio_iov;
> cheers,
> natano
>
> Index: tmpfs_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/tmpfs/tmpfs_subr.c,v
> retrieving revision 1.14
> diff -u -p -u -r1.14 tmpfs_subr.c
> --- tmpfs_subr.c 17 Apr 2015 04:43:21 -0000 1.14
> +++ tmpfs_subr.c 1 Jan 2016 15:47:39 -0000
> @@ -740,7 +740,7 @@ tmpfs_dir_getdotents(tmpfs_node_t *node,
> return EJUSTRETURN;
> }
>
> - if ((error = uiomovei(dp, dp->d_reclen, uio)) != 0) {
> + if ((error = uiomove(dp, dp->d_reclen, uio)) != 0) {
> return error;
> }
>
> @@ -837,7 +837,7 @@ tmpfs_dir_getdents(tmpfs_node_t *node, s
> }
>
> /* Copy out the directory entry and continue. */
> - error = uiomovei(&dent, dent.d_reclen, uio);
> + error = uiomove(&dent, dent.d_reclen, uio);
> if (error) {
> break;
> }
> @@ -1225,7 +1225,7 @@ tmpfs_uiomove(tmpfs_node_t *node, struct
> if (pgoff + len < PAGE_SIZE) {
> va = tmpfs_uio_lookup(node, pgnum);
> if (va != (vaddr_t)NULL)
> - return uiomovei((void *)va + pgoff, len, uio);
> + return uiomove((void *)va + pgoff, len, uio);
> }
>
> if (len >= TMPFS_UIO_MAXBYTES) {
> @@ -1249,7 +1249,7 @@ tmpfs_uiomove(tmpfs_node_t *node, struct
> return error;
> }
>
> - error = uiomovei((void *)va + pgoff, sz, uio);
> + error = uiomove((void *)va + pgoff, sz, uio);
> if (error == 0 && pgoff + sz < PAGE_SIZE)
> tmpfs_uio_cache(node, pgnum, va);
> else
> Index: tmpfs_vnops.c
> ===================================================================
> RCS file: /cvs/src/sys/tmpfs/tmpfs_vnops.c,v
> retrieving revision 1.23
> diff -u -p -u -r1.23 tmpfs_vnops.c
> --- tmpfs_vnops.c 8 Dec 2015 15:26:25 -0000 1.23
> +++ tmpfs_vnops.c 1 Jan 2016 15:47:52 -0000
> @@ -1017,7 +1017,7 @@ tmpfs_readlink(void *v)
> KASSERT(vp->v_type == VLNK);
>
> node = VP_TO_TMPFS_NODE(vp);
> - error = uiomovei(node->tn_spec.tn_lnk.tn_link,
> + error = uiomove(node->tn_spec.tn_lnk.tn_link,
> MIN(node->tn_size, uio->uio_resid), uio);
> tmpfs_update(node, TMPFS_NODE_ACCESSED);
>
>
