Re: vfs_syscall: doreadlinkat()
Committed, thanks!
Re: vfs_syscall: doreadlinkat()
Le 24/01/2013 19:10, Matthew Dempsky a écrit : > On Thu, Jan 24, 2013 at 9:57 AM, Maxime Villard wrote: > >> > Hum here, if vp->v_type != VLNK, auio is untouched, but before returning >> > we use auio.uio_resid, which is not initialized. Is it? >> > > Nice catch. We should probably move the *retval assignment up just after > the VOP_READLINK() call, since this can technically result in undefined > behavior if you try to readlink on a non-symlink file. > Yes. Index: vfs_syscalls.c === RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v retrieving revision 1.189 diff -u -r1.189 vfs_syscalls.c --- vfs_syscalls.c 10 Sep 2012 11:10:59 - 1.189 +++ vfs_syscalls.c 25 Jan 2013 15:30:30 - @@ -1843,9 +1843,9 @@ auio.uio_procp = p; auio.uio_resid = count; error = VOP_READLINK(vp, &auio, p->p_ucred); + *retval = count - auio.uio_resid; } vput(vp); - *retval = count - auio.uio_resid; return (error); } > I don't think it should leak any information moment though, since the > EINVAL errno will take precedence instead of *retval when we return to > userspace. > >
Re: vfs_syscall: doreadlinkat()
On Thu, Jan 24, 2013 at 9:57 AM, Maxime Villard wrote: > Hum here, if vp->v_type != VLNK, auio is untouched, but before returning > we use auio.uio_resid, which is not initialized. Is it? > Nice catch. We should probably move the *retval assignment up just after the VOP_READLINK() call, since this can technically result in undefined behavior if you try to readlink on a non-symlink file. I don't think it should leak any information moment though, since the EINVAL errno will take precedence instead of *retval when we return to userspace.
vfs_syscall: doreadlinkat()
Hi, I was looking at readlink syscall. There is the following function in kern/vfs_syscalls.c: int doreadlinkat(struct proc *p, int fd, const char *path, char *buf, size_t count, register_t *retval) { struct vnode *vp; struct iovec aiov; struct uio auio; int error; struct nameidata nd; NDINITAT(&nd, LOOKUP, NOFOLLOW | LOCKLEAF, UIO_USERSPACE, fd, path, p); if ((error = namei(&nd)) != 0) return (error); vp = nd.ni_vp; if (vp->v_type != VLNK) error = EINVAL; else { aiov.iov_base = buf; aiov.iov_len = count; auio.uio_iov = &aiov; auio.uio_iovcnt = 1; auio.uio_offset = 0; auio.uio_rw = UIO_READ; auio.uio_segflg = UIO_USERSPACE; auio.uio_procp = p; auio.uio_resid = count; error = VOP_READLINK(vp, &auio, p->p_ucred); } vput(vp); *retval = count - auio.uio_resid; return (error); } Hum here, if vp->v_type != VLNK, auio is untouched, but before returning we use auio.uio_resid, which is not initialized. Is it?