> CVSROOT:      /cvs
> Module name:  src
> Changes by:   an...@cvs.openbsd.org   2022/05/23 23:14:30
> 
> Modified files:
>       regress/lib/libc/sys: t_truncate.c 
> 
> Log message:
> Recent changes to truncate(2) swapped the ordering of some validations
> causing EACCESS as opposed of ESDIR to be returned while trying to
> truncate a directory as a user lacking write permissions to the same
> directory. As this behavior is reasonable, change the truncate directory
> from /etc/ to /tmp which makes the test pass both as root and non-root.

Pretty certain that is a mistake in the kernel commit.  A big part of the
discussion around that diff wasn't just fixing the rlimit problem, but
trying to make sure the order of evaluation of error was correct.  Looks
like it is wrong..

I guess the VDIR check should be put back into the two callers, not the
helper function, which will prevent the VOP_ACCESS errno from being
returned.

Maybe something like this (untested)

Index: vfs_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.357
diff -u -p -u -r1.357 vfs_syscalls.c
--- vfs_syscalls.c      23 May 2022 15:17:11 -0000      1.357
+++ vfs_syscalls.c      24 May 2022 05:30:59 -0000
@@ -2821,8 +2821,6 @@ dotruncate(struct proc *p, struct vnode 
 
        if (len < 0)
                return EINVAL;
-       if (vp->v_type == VDIR)
-               return EISDIR;
        if ((error = vn_writechk(vp)) != 0)
                return error;
        if (vp->v_type == VREG && len > lim_cur_proc(p, RLIMIT_FSIZE)) {
@@ -2860,7 +2858,9 @@ sys_truncate(struct proc *p, void *v, re
                return (error);
        vp = nd.ni_vp;
        vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
-       if ((error = VOP_ACCESS(vp, VWRITE, p->p_ucred, p)) == 0)
+       if (vp->v_type == VDIR)
+               error = EISDIR;
+       else if ((error = VOP_ACCESS(vp, VWRITE, p->p_ucred, p)) == 0)
                error = dotruncate(p, vp, SCARG(uap, length));
        vput(vp);
        return (error);
@@ -2888,7 +2888,10 @@ sys_ftruncate(struct proc *p, void *v, r
        }
        vp = fp->f_data;
        vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
-       error = dotruncate(p, vp, SCARG(uap, length));
+       if (vp->v_type == VDIR)
+               error = EISDIR;
+       else
+               error = dotruncate(p, vp, SCARG(uap, length));
        VOP_UNLOCK(vp);
 bad:
        FRELE(fp, p);


Anton Lindqvist <an...@cvs.openbsd.org> wrote:

Reply via email to