Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/sys sys/kern: Allow custom fileops to specify fo_seek me...



details:   https://anonhg.NetBSD.org/src/rev/625e8e22399e
branches:  trunk
changeset: 1023495:625e8e22399e
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Sep 11 10:08:55 2021 +0000

description:
sys/kern: Allow custom fileops to specify fo_seek method.

Previously only vnodes allowed lseek/pread[v]/pwrite[v], which meant
converting a regular device to a cloning device doesn't always work.

Semantics is:

(*fp->f_ops->fo_seek)(fp, delta, whence, newoffp, flags)

1. Compute a new offset according to whence + delta -- that is, if
   whence is SEEK_CUR, add delta to fp->f_offset; if whence is
   SEEK_END, add delta to end of file; if whence is SEEK_CUR, use delta
   as is.

2. If newoffp is nonnull, return the new offset in *newoffp.

3. If flags & FOF_UPDATE_OFFSET, set fp->f_offset to the new offset.

Access to fp->f_offset, and *newoffp if newoffp = &fp->f_offset, must
happen under the object lock (e.g., vnode lock), in order to
synchronize fp->f_offset reads and writes.

This change has the side effect that every call to VOP_SEEK happens
under the vnode lock now, when previously it didn't.  However, from a
review of all the VOP_SEEK implementations, it does not appear that
any file system even examines the vnode, let alone locks it.  So I
think this is safe -- and essentially the only reasonable way to do
things, given that it is used to validate a change from oldoff to
newoff, and oldoff becomes stale the moment we unlock the vnode.

No kernel bump because this reuses a spare entry in struct fileops,
and it is safe for the entry to be null, so all existing fileops will
continue to work as before (rejecting seek).

diffstat:

 sys/compat/netbsd32/netbsd32_fs.c |  28 ++++----------
 sys/kern/sys_generic.c            |  38 ++++++++++---------
 sys/kern/vfs_syscalls.c           |  72 ++++++++++----------------------------
 sys/kern/vfs_vnops.c              |  55 ++++++++++++++++++++++++++++-
 sys/sys/file.h                    |   4 +-
 5 files changed, 103 insertions(+), 94 deletions(-)

diffs (truncated from 404 to 300 lines):

diff -r 7a72607260c9 -r 625e8e22399e sys/compat/netbsd32/netbsd32_fs.c
--- a/sys/compat/netbsd32/netbsd32_fs.c Sat Sep 11 09:16:14 2021 +0000
+++ b/sys/compat/netbsd32/netbsd32_fs.c Sat Sep 11 10:08:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: netbsd32_fs.c,v 1.93 2021/02/16 14:47:20 simonb Exp $  */
+/*     $NetBSD: netbsd32_fs.c,v 1.94 2021/09/11 10:08:55 riastradh Exp $       */
 
 /*
  * Copyright (c) 1998, 2001 Matthew R. Green
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: netbsd32_fs.c,v 1.93 2021/02/16 14:47:20 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: netbsd32_fs.c,v 1.94 2021/09/11 10:08:55 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -648,7 +648,6 @@
                syscallarg(netbsd32_off_t) offset;
        } */
        file_t *fp;
-       struct vnode *vp;
        off_t offset;
        int error, fd = SCARG(uap, fd);
 
@@ -660,19 +659,14 @@
                return EBADF;
        }
 
-       vp = fp->f_vnode;
-       if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+       if (fp->f_ops->fo_seek == NULL) {
                error = ESPIPE;
                goto out;
        }
 
        offset = SCARG(uap, offset);
-
-       /*
-        * XXX This works because no file systems actually
-        * XXX take any action on the seek operation.
-        */
-       if ((error = VOP_SEEK(vp, fp->f_offset, offset, fp->f_cred)) != 0)
+       error = (*fp->f_ops->fo_seek)(fp, offset, SEEK_SET, &offset, 0);
+       if (error)
                goto out;
 
        return dofilereadv32(fd, fp, SCARG_P32(uap, iovp),
@@ -694,7 +688,6 @@
                syscallarg(netbsd32_off_t) offset;
        } */
        file_t *fp;
-       struct vnode *vp;
        off_t offset;
        int error, fd = SCARG(uap, fd);
 
@@ -706,19 +699,14 @@
                return EBADF;
        }
 
-       vp = fp->f_vnode;
-       if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+       if (fp->f_ops->fo_seek == NULL) {
                error = ESPIPE;
                goto out;
        }
 
        offset = SCARG(uap, offset);
-
-       /*
-        * XXX This works because no file systems actually
-        * XXX take any action on the seek operation.
-        */
-       if ((error = VOP_SEEK(vp, fp->f_offset, offset, fp->f_cred)) != 0)
+       error = (*fp->f_ops->fo_seek)(fp, offset, SEEK_SET, &offset, 0);
+       if (error)
                goto out;
 
        return dofilewritev32(fd, fp, SCARG_P32(uap, iovp),
diff -r 7a72607260c9 -r 625e8e22399e sys/kern/sys_generic.c
--- a/sys/kern/sys_generic.c    Sat Sep 11 09:16:14 2021 +0000
+++ b/sys/kern/sys_generic.c    Sat Sep 11 10:08:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sys_generic.c,v 1.132 2020/05/23 23:42:43 ad Exp $     */
+/*     $NetBSD: sys_generic.c,v 1.133 2021/09/11 10:08:55 riastradh Exp $      */
 
 /*-
  * Copyright (c) 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_generic.c,v 1.132 2020/05/23 23:42:43 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_generic.c,v 1.133 2021/09/11 10:08:55 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -208,17 +208,18 @@
        if (offset == NULL)
                offset = &fp->f_offset;
        else {
-               struct vnode *vp = fp->f_vnode;
-               if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+               /*
+                * Caller must not specify &fp->f_offset -- we can't
+                * safely dereference it for the call to fo_seek
+                * without holding some underlying object lock.
+                */
+               KASSERT(offset != &fp->f_offset);
+               if (fp->f_ops->fo_seek == NULL) {
                        error = ESPIPE;
                        goto out;
                }
-               /*
-                * Test that the device is seekable ?
-                * XXX This works because no file systems actually
-                * XXX take any action on the seek operation.
-                */
-               error = VOP_SEEK(vp, fp->f_offset, *offset, fp->f_cred);
+               error = (*fp->f_ops->fo_seek)(fp, *offset, SEEK_SET, NULL,
+                   0);
                if (error != 0)
                        goto out;
        }
@@ -408,17 +409,18 @@
        if (offset == NULL)
                offset = &fp->f_offset;
        else {
-               struct vnode *vp = fp->f_vnode;
-               if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+               /*
+                * Caller must not specify &fp->f_offset -- we can't
+                * safely dereference it for the call to fo_seek
+                * without holding some underlying object lock.
+                */
+               KASSERT(offset != &fp->f_offset);
+               if (fp->f_ops->fo_seek == NULL) {
                        error = ESPIPE;
                        goto out;
                }
-               /*
-                * Test that the device is seekable ?
-                * XXX This works because no file systems actually
-                * XXX take any action on the seek operation.
-                */
-               error = VOP_SEEK(vp, fp->f_offset, *offset, fp->f_cred);
+               error = (*fp->f_ops->fo_seek)(fp, *offset, SEEK_SET, NULL,
+                   0);
                if (error != 0)
                        goto out;
        }
diff -r 7a72607260c9 -r 625e8e22399e sys/kern/vfs_syscalls.c
--- a/sys/kern/vfs_syscalls.c   Sat Sep 11 09:16:14 2021 +0000
+++ b/sys/kern/vfs_syscalls.c   Sat Sep 11 10:08:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_syscalls.c,v 1.551 2021/07/03 09:39:26 mlelstv Exp $       */
+/*     $NetBSD: vfs_syscalls.c,v 1.552 2021/09/11 10:08:55 riastradh Exp $     */
 
 /*-
  * Copyright (c) 2008, 2009, 2019, 2020 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_syscalls.c,v 1.551 2021/07/03 09:39:26 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_syscalls.c,v 1.552 2021/09/11 10:08:55 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_fileassoc.h"
@@ -2856,50 +2856,30 @@
                syscallarg(off_t) offset;
                syscallarg(int) whence;
        } */
-       kauth_cred_t cred = l->l_cred;
        file_t *fp;
-       struct vnode *vp;
-       struct vattr vattr;
-       off_t newoff;
        int error, fd;
 
+       switch (SCARG(uap, whence)) {
+       case SEEK_CUR:
+       case SEEK_END:
+       case SEEK_SET:
+               break;
+       default:
+               return EINVAL;
+       }
+
        fd = SCARG(uap, fd);
 
        if ((fp = fd_getfile(fd)) == NULL)
                return (EBADF);
 
-       vp = fp->f_vnode;
-       if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+       if (fp->f_ops->fo_seek == NULL) {
                error = ESPIPE;
                goto out;
        }
 
-       vn_lock(vp, LK_SHARED | LK_RETRY);
-
-       switch (SCARG(uap, whence)) {
-       case SEEK_CUR:
-               newoff = fp->f_offset + SCARG(uap, offset);
-               break;
-       case SEEK_END:
-               error = VOP_GETATTR(vp, &vattr, cred);
-               if (error) {
-                       VOP_UNLOCK(vp);
-                       goto out;
-               }
-               newoff = SCARG(uap, offset) + vattr.va_size;
-               break;
-       case SEEK_SET:
-               newoff = SCARG(uap, offset);
-               break;
-       default:
-               error = EINVAL;
-               VOP_UNLOCK(vp);
-               goto out;
-       }
-       VOP_UNLOCK(vp);
-       if ((error = VOP_SEEK(vp, fp->f_offset, newoff, cred)) == 0) {
-               *(off_t *)retval = fp->f_offset = newoff;
-       }
+       error = (*fp->f_ops->fo_seek)(fp, SCARG(uap, offset),
+           SCARG(uap, whence), (off_t *)retval, FOF_UPDATE_OFFSET);
  out:
        fd_putfile(fd);
        return (error);
@@ -2918,7 +2898,6 @@
                syscallarg(off_t) offset;
        } */
        file_t *fp;
-       struct vnode *vp;
        off_t offset;
        int error, fd = SCARG(uap, fd);
 
@@ -2930,19 +2909,14 @@
                return (EBADF);
        }
 
-       vp = fp->f_vnode;
-       if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+       if (fp->f_ops->fo_seek == NULL) {
                error = ESPIPE;
                goto out;
        }
 
        offset = SCARG(uap, offset);
-
-       /*
-        * XXX This works because no file systems actually
-        * XXX take any action on the seek operation.
-        */
-       if ((error = VOP_SEEK(vp, fp->f_offset, offset, fp->f_cred)) != 0)
+       error = (*fp->f_ops->fo_seek)(fp, offset, SEEK_SET, &offset, 0);
+       if (error)
                goto out;
 
        /* dofileread() will unuse the descriptor for us */
@@ -2985,7 +2959,6 @@
                syscallarg(off_t) offset;
        } */
        file_t *fp;
-       struct vnode *vp;
        off_t offset;
        int error, fd = SCARG(uap, fd);
 
@@ -2997,19 +2970,14 @@
                return (EBADF);
        }
 
-       vp = fp->f_vnode;
-       if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+       if (fp->f_ops->fo_seek == NULL) {
                error = ESPIPE;
                goto out;
        }
 
        offset = SCARG(uap, offset);
-
-       /*
-        * XXX This works because no file systems actually
-        * XXX take any action on the seek operation.
-        */
-       if ((error = VOP_SEEK(vp, fp->f_offset, offset, fp->f_cred)) != 0)
+       error = (*fp->f_ops->fo_seek)(fp, offset, SEEK_SET, &offset, 0);
+       if (error)
                goto out;
 
        /* dofilewrite() will unuse the descriptor for us */
diff -r 7a72607260c9 -r 625e8e22399e sys/kern/vfs_vnops.c
--- a/sys/kern/vfs_vnops.c      Sat Sep 11 09:16:14 2021 +0000
+++ b/sys/kern/vfs_vnops.c      Sat Sep 11 10:08:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_vnops.c,v 1.221 2021/07/18 09:30:36 dholland Exp $ */



Home | Main Index | Thread Index | Old Index