Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Jul 2019 12:51:59 -0400
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Kyle Evans <kevans@freebsd.org>
Cc:        Rick Macklem <rmacklem@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org>
Subject:   Re: svn commit: r350315 - in head/sys: kern sys
Message-ID:  <20190725165159.y3zhaafdkrundew2@mutt-hbsd>
In-Reply-To: <CACNAnaHVWUGG%2B09wQ1MKAWVeQFKzww8VSMT09CFPJUKX1fWZwQ@mail.gmail.com>
References:  <201907250546.x6P5kHWq076756@repo.freebsd.org> <20190725164607.zpa7w2pgrnahaxz4@mutt-hbsd> <CACNAnaHVWUGG%2B09wQ1MKAWVeQFKzww8VSMT09CFPJUKX1fWZwQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--sugm4n5urmhr6ogw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jul 25, 2019 at 11:48:39AM -0500, Kyle Evans wrote:
> On Thu, Jul 25, 2019 at 11:46 AM Shawn Webb <shawn.webb@hardenedbsd.org> =
wrote:
> >
> > Hey Rick,
> >
> > On Thu, Jul 25, 2019 at 05:46:17AM +0000, Rick Macklem wrote:
> > > Author: rmacklem
> > > Date: Thu Jul 25 05:46:16 2019
> > > New Revision: 350315
> > > URL: https://svnweb.freebsd.org/changeset/base/350315
> > >
> > > Log:
> > >   Add kernel support for a Linux compatible copy_file_range(2) syscal=
l.
> > >
> > >   This patch adds support to the kernel for a Linux compatible
> > >   copy_file_range(2) syscall and the related VOP_COPY_FILE_RANGE(9).
> > >   This syscall/VOP can be used by the NFSv4.2 client to implement the
> > >   Copy operation against an NFSv4.2 server to do file copies locally =
on
> > >   the server.
> > >   The vn_generic_copy_file_range() function in this patch can be used
> > >   by the NFSv4.2 server to implement the Copy operation.
> > >   Fuse may also me able to use the VOP_COPY_FILE_RANGE() method.
> > >
> > >   vn_generic_copy_file_range() attempts to maintain holes in the outp=
ut
> > >   file in the range to be copied, but may fail to do so if the input =
and
> > >   output files are on different file systems with different _PC_MIN_H=
OLE_SIZE
> > >   values.
> > >
> > >   Separate commits will be done for the generated syscall files and u=
serland
> > >   changes. A commit for a compat32 syscall will be done later.
> > >
> > >   Reviewed by:        kib, asomers (plus comments by brooks, jilles)
> > >   Relnotes:   yes
> > >   Differential Revision:      https://reviews.freebsd.org/D20584
> > >
> > > Modified:
> > >   head/sys/kern/syscalls.master
> > >   head/sys/kern/vfs_default.c
> > >   head/sys/kern/vfs_syscalls.c
> > >   head/sys/kern/vfs_vnops.c
> > >   head/sys/kern/vnode_if.src
> > >   head/sys/sys/syscallsubr.h
> > >   head/sys/sys/vnode.h
> > >
> > > Modified: head/sys/kern/syscalls.master
> > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > > --- head/sys/kern/syscalls.master     Thu Jul 25 03:55:05 2019       =
 (r350314)
> > > +++ head/sys/kern/syscalls.master     Thu Jul 25 05:46:16 2019       =
 (r350315)
> > > @@ -3175,6 +3175,16 @@
> > >                   int flag
> > >               );
> > >       }
> > > +569  AUE_NULL        STD {
> > > +             ssize_t copy_file_range(
> > > +                 int infd,
> > > +                 _Inout_opt_ off_t *inoffp,
> > > +                 int outfd,
> > > +                 _Inout_opt_ off_t *outoffp,
> > > +                 size_t len,
> > > +                 unsigned int flags
> > > +             );
> > > +     }
> > >
> > >  ; Please copy any additions and changes to the following compatabili=
ty tables:
> > >  ; sys/compat/freebsd32/syscalls.master
> > >
> > > Modified: head/sys/kern/vfs_default.c
> > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > > --- head/sys/kern/vfs_default.c       Thu Jul 25 03:55:05 2019       =
 (r350314)
> > > +++ head/sys/kern/vfs_default.c       Thu Jul 25 05:46:16 2019       =
 (r350315)
> > > @@ -83,6 +83,7 @@ static int  dirent_exists(struct vnode *vp, const c=
har
> > >  static int vop_stdis_text(struct vop_is_text_args *ap);
> > >  static int vop_stdunset_text(struct vop_unset_text_args *ap);
> > >  static int vop_stdadd_writecount(struct vop_add_writecount_args *ap);
> > > +static int vop_stdcopy_file_range(struct vop_copy_file_range_args *a=
p);
> > >  static int vop_stdfdatasync(struct vop_fdatasync_args *ap);
> > >  static int vop_stdgetpages_async(struct vop_getpages_async_args *ap);
> > >
> > > @@ -140,6 +141,7 @@ struct vop_vector default_vnodeops =3D {
> > >       .vop_set_text =3D         vop_stdset_text,
> > >       .vop_unset_text =3D       vop_stdunset_text,
> > >       .vop_add_writecount =3D   vop_stdadd_writecount,
> > > +     .vop_copy_file_range =3D  vop_stdcopy_file_range,
> > >  };
> > >
> > >  /*
> > > @@ -1210,6 +1212,17 @@ vfs_stdnosync (mp, waitfor)
> > >  {
> > >
> > >       return (0);
> > > +}
> > > +
> > > +static int
> > > +vop_stdcopy_file_range(struct vop_copy_file_range_args *ap)
> > > +{
> > > +     int error;
> > > +
> > > +     error =3D vn_generic_copy_file_range(ap->a_invp, ap->a_inoffp,
> > > +         ap->a_outvp, ap->a_outoffp, ap->a_lenp, ap->a_flags, ap->a_=
incred,
> > > +         ap->a_outcred, ap->a_fsizetd);
> > > +     return (error);
> > >  }
> > >
> > >  int
> > >
> > > Modified: head/sys/kern/vfs_syscalls.c
> > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > > --- head/sys/kern/vfs_syscalls.c      Thu Jul 25 03:55:05 2019       =
 (r350314)
> > > +++ head/sys/kern/vfs_syscalls.c      Thu Jul 25 05:46:16 2019       =
 (r350315)
> > > @@ -4814,3 +4814,122 @@ sys_posix_fadvise(struct thread *td, struct p=
osix_fadv
> > >           uap->advice);
> > >       return (kern_posix_error(td, error));
> > >  }
> > > +
> > > +int
> > > +kern_copy_file_range(struct thread *td, int infd, off_t *inoffp, int=
 outfd,
> > > +    off_t *outoffp, size_t len, unsigned int flags)
> > > +{
> > > +     struct file *infp, *outfp;
> > > +     struct vnode *invp, *outvp;
> > > +     int error;
> > > +     size_t retlen;
> > > +     void *rl_rcookie, *rl_wcookie;
> > > +     off_t savinoff, savoutoff;
> > > +
> > > +     infp =3D outfp =3D NULL;
> > > +     rl_rcookie =3D rl_wcookie =3D NULL;
> > > +     savinoff =3D -1;
> > > +     error =3D 0;
> > > +     retlen =3D 0;
> > > +
> > > +     if (flags !=3D 0) {
> > > +             error =3D EINVAL;
> > > +             goto out;
> > > +     }
> > > +     if (len > SSIZE_MAX)
> > > +             /*
> > > +              * Although the len argument is size_t, the return argu=
ment
> > > +              * is ssize_t (which is signed).  Therefore a size that=
 won't
> > > +              * fit in ssize_t can't be returned.
> > > +              */
> > > +             len =3D SSIZE_MAX;
> > > +
> > > +     /* Get the file structures for the file descriptors. */
> > > +     error =3D fget_read(td, infd, &cap_read_rights, &infp);
> > > +     if (error !=3D 0)
> > > +             goto out;
> > > +     error =3D fget_write(td, outfd, &cap_write_rights, &outfp);
> > > +     if (error !=3D 0)
> > > +             goto out;
> > > +
> > > +     /* Set the offset pointers to the correct place. */
> > > +     if (inoffp =3D=3D NULL)
> > > +             inoffp =3D &infp->f_offset;
> > > +     if (outoffp =3D=3D NULL)
> > > +             outoffp =3D &outfp->f_offset;
> > > +     savinoff =3D *inoffp;
> > > +     savoutoff =3D *outoffp;
> >
> > Should these two lines, saving the old inoffp and outoffp, be moved
> > before the two conditionals above?
> >
>=20
> Dereferencing potentially NULL pointers like that seems like a scary
> proposition; I think this reads most correctly given the context.

Ah, good catch! I missed reading the dereference.

Thanks,

--=20
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal:    +1 443-546-8752
Tor+XMPP+OTR:        lattera@is.a.hacker.sx
GPG Key ID:          0xFF2E67A277F8E1FA
GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9  3633 C85B 0AF8 AB23 0FB2

--sugm4n5urmhr6ogw
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl053i4ACgkQ/y5nonf4
4fpiOg//dxMJ0ROFTnnFFQCmdqNGOAxxV6YOi5MToKu4lE2+eVY6uyoXok1IZ0ay
60orsBakPvo38CrtRNjLo1fgkm14GyHo4c4THGY7T9sPPcYL88hBNSuHs1eMMzOW
qSlcwLcpBvRE4xOuAMuHWXF7xLj7iUEnhVYmhaZDYH+sWszElLZ8I7DRctSaF22e
S6tkwoQRq1yj0rY/4mTlYxeBvnAyQCWijLxyjllKjVT+aXUI4m01pDRIMFd8O4Me
D7cbVHRXyTiwC+SkrfKK0QWbPr+GFdaF7IHyI/Nka/LRCNJatctV0Dw6L6WzUmZP
2w9cBFTSDSfQb2yAesbgHtnotVeHJJNilJQn8QAoZLUd7gl5aPhkHgWhijQcXeA6
G32kpwt80DCEji0UxGK9q5tFEjVe3Jz8BKzPo+QS3+BX5Cv2K+2XLJi+LAbfIN8H
E4x7nvmIg/UY7oU/yP2tEB6yKlZjDJr4i1tF8ss7YhXaBAYxFXcv/gm4RJ++141U
sxYJsOGmTG/hOqCHh/BADx3sgjVCUn12fnNMBCDfooSpfVbNxII3DN9Fi6N7zPCG
2No8Q0AczdECvA2XYprXGCkODs8Qy05gyf5pxjMIXIky4tiKV4OiQodjuLIXem+M
Dywx3w3aHIkAeQ7Grp+I6RYRAk/9rah4b3tid+rNpadeVIMzC1Y=
=NzFq
-----END PGP SIGNATURE-----

--sugm4n5urmhr6ogw--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190725165159.y3zhaafdkrundew2>