Date: Wed, 29 Feb 2012 14:21:23 +0300 From: Sergey Kandaurov <pluknet@gmail.com> To: d@delphij.net Cc: Jilles Tjoelker <jilles@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: RFC: futimens(2) and utimensat(2) Message-ID: <CAE-mSOJU=hm8%2B-AC_oQmx%2Bh2grv7PGaH7kNYKoT3GMePDPXsYg@mail.gmail.com> In-Reply-To: <4F4DC876.3010809@delphij.net> References: <4F4DC876.3010809@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On 29 February 2012 10:40, Xin Li <delphij@delphij.net> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Hi, > > These are required by IEEE Std 1003.1-2008. =A0Patchset at: > > http://people.freebsd.org/~delphij/for_review/utimens.diff > First, thank you very much for doing this. ERRORS section for utimes(2) is still not updated (not exists). Funny but that was the most difficult part to implement these syscalls a year ago with the great help from jilles@. He could further comment on your patchset. Otherwise looks good and pretty similar to my work, though I didn't use a "const" modifier in my version for both functions and syscall definitions in syscall.master for some reasons. Further I wrote a test to see how properly implementation detects EACCES/EPERM with different UTIME_OMIT/UTIME_NOW passed. It shall pass all tests as shown in the table (stolen somewhere from austingroup): [a] [b] [c] times file file arg. UID is NULL owner writable Result !NULL !owner !writable N o w success N o !w success N ! w success N !o !w EACCES [1] !N o w success !N o !w success !N !o w EPERM [2] !N !o !w EPERM [3] Here NULL also covers cases when: - both fields are UTIME_NULL - both fields are UTIME_OMIT. Ok, lets see how it does: 1) Given: UTIME_NOW UTIME_NOW o w gives: success expected: success 2) Given: UTIME_NOW UTIME_NOW o !w gives: success expected: success 3) Given: UTIME_NOW UTIME_NOW !o w gives: EPERM expected: success 4) Given: UTIME_NOW UTIME_NOW !o !w gives: EPERM expected: EACCES 5) Given: (NULL) (NULL) o w gives: success expected: success 6) Given: (NULL) (NULL) o !w gives: success expected: success 7) Given: (NULL) (NULL) !o w gives: success expected: success 8) Given: (NULL) (NULL) !o !w gives: EACCES expected: EACCES 9) Given: (number) (number) o w gives: success expected: success 10) Given: (number) (number) o !w gives: success expected: success 11) Given: (number) (number) !o w gives: EPERM expected: EPERM 12) Gives: (number) (number) !o !w gives: EPERM expected: EPERM So, your version doesn't differentiate the case with both UTIME_NULL as a special case when you need to grant caller more privileges as if this was the case with both NULL pointers. My version handles this. Your version uses two calls to vfs_timestamp() in different condition branches. It could be done just once. My version of getutimens() is more complicated but it handles the case with both UTIME_NOW. This is the older version last time discussed with jilles. It misses man page update and compat32 parts (both were done since then except missing ERROR section in utimes(2). e.g. my compat32 version is just as yours :)). I started to commit my version (you can see r227447) but failed due to missing ERROR section, my lack of english to rewrite utimes(2) man page, and too complicated and wrong ERROR section in the existing utimes(2). http://plukky.net/~pluknet/patches/utimes.2008.3.diff It is pretty similar to your except I done getutimens() a bit different. I had to introduce such complication to pass all tests. Take note on private flags UTIMENS_NULL and UTIMENS_EXIT. Index: 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 --- sys/kern/vfs_syscalls.c (revision 220831) +++ sys/kern/vfs_syscalls.c (working copy) @@ -94,6 +94,8 @@ static int chroot_refuse_vdir_fds(struct filedesc *fdp); static int getutimes(const struct timeval *, enum uio_seg, struct timespec= *); +static int getutimens(const struct timespec *, enum uio_seg, + struct timespec *, int *); static int setfown(struct thread *td, struct vnode *, uid_t, gid_t); static int setfmode(struct thread *td, struct vnode *, int); static int setfflags(struct thread *td, struct vnode *, int); @@ -3162,9 +3164,61 @@ } /* - * Common implementation code for utimes(), lutimes(), and futimes(). + * Common implementation code for futimens(), utimensat(). */ +#define UTIMENS_NULL 0x1 +#define UTIMENS_EXIT 0x2 static int +getutimens(usrtsp, tspseg, tsp, retflags) + const struct timespec *usrtsp; + enum uio_seg tspseg; + struct timespec *tsp; + int *retflags; +{ + int error; + struct timespec tsnow; + + vfs_timestamp(&tsnow); + *retflags =3D 0; + if (usrtsp =3D=3D NULL) { + tsp[0] =3D tsnow; + tsp[1] =3D tsnow; + *retflags |=3D UTIMENS_NULL; + return (0); + } + if (tspseg =3D=3D UIO_SYSSPACE) { + tsp[0] =3D usrtsp[0]; + tsp[1] =3D usrtsp[1]; + } else if ((error =3D copyin(usrtsp, tsp, sizeof(*tsp) * 2)) !=3D 0) + return (error); + + if (tsp[0].tv_nsec =3D=3D UTIME_OMIT && tsp[1].tv_nsec =3D=3D UTIME_OMIT) + *retflags |=3D UTIMENS_EXIT; + if (tsp[0].tv_nsec =3D=3D UTIME_NOW && tsp[1].tv_nsec =3D=3D UTIME_NOW) + *retflags |=3D UTIMENS_NULL; + + if (tsp[0].tv_nsec =3D=3D UTIME_OMIT) + tsp[0].tv_sec =3D VNOVAL; + else if (tsp[0].tv_nsec =3D=3D UTIME_NOW) + tsp[0] =3D tsnow; + else if (tsp[0].tv_nsec < 0 || tsp[0].tv_nsec >=3D 1000000000L) + return (EINVAL); + + if (tsp[1].tv_nsec =3D=3D UTIME_OMIT) + tsp[1].tv_sec =3D VNOVAL; + else if (tsp[1].tv_nsec =3D=3D UTIME_NOW) + tsp[1] =3D tsnow; + else if (tsp[1].tv_nsec < 0 || tsp[1].tv_nsec >=3D 1000000000L) + return (EINVAL); + + return (0); +} + +/* + * Common implementation code for utimes(), lutimes(), futimes(), futimens= (), + * and utimensat(). + */ +static int setutimes(td, vp, ts, numtimes, nullflag) struct thread *td; struct vnode *vp; @@ -3362,6 +3416,94 @@ return (error); } +#ifndef _SYS_SYSPROTO_H_ +struct futimens_args { + int fd; + struct timespec *times; +}; +#endif +int +futimens(struct thread *td, struct futimens_args *uap) +{ + + return (kern_futimens(td, uap->fd, uap->times, UIO_USERSPACE)); +} + +int +kern_futimens(struct thread *td, int fd, struct timespec *tptr, + enum uio_seg tptrseg) +{ + struct timespec ts[2]; + struct file *fp; + int error, flags, vfslocked; + + AUDIT_ARG_FD(fd); + if ((error =3D getutimens(tptr, tptrseg, ts, &flags)) !=3D 0) + return (error); + if (flags & UTIMENS_EXIT) + return (0); + if ((error =3D getvnode(td->td_proc->p_fd, fd, &fp)) !=3D 0) + return (error); + vfslocked =3D VFS_LOCK_GIANT(fp->f_vnode->v_mount); +#ifdef AUDIT + vn_lock(fp->f_vnode, LK_SHARED | LK_RETRY); + AUDIT_ARG_VNODE1(fp->f_vnode); + VOP_UNLOCK(fp->f_vnode, 0); +#endif + error =3D setutimes(td, fp->f_vnode, ts, 2, flags & UTIMENS_NULL); + VFS_UNLOCK_GIANT(vfslocked); + fdrop(fp, td); + return (error); +} + +#ifndef _SYS_SYSPROTO_H_ +struct utimensat_args { + int fd; + const char *path; + const struct timespec *times; + int flag; +}; +#endif +int +utimensat(struct thread *td, struct utimensat_args *uap) +{ + + return (kern_utimensat(td, uap->fd, uap->path, UIO_USERSPACE, + uap->times, UIO_USERSPACE, uap->flag)); +} + +int +kern_utimensat(struct thread *td, int fd, char *path, enum uio_seg pathseg= , + struct timespec *tptr, enum uio_seg tptrseg, int flag) +{ + struct nameidata nd; + struct timespec ts[2]; + int error, flags, vfslocked; + + if (flag & ~AT_SYMLINK_NOFOLLOW) + return (EINVAL); + + if ((error =3D getutimens(tptr, tptrseg, ts, &flags)) !=3D 0) + return (error); + NDINIT_AT(&nd, LOOKUP, ((flag & AT_SYMLINK_NOFOLLOW) ? NOFOLLOW : + FOLLOW) | MPSAFE | AUDITVNODE1, pathseg, path, fd, td); + if ((error =3D namei(&nd)) !=3D 0) + return (error); + /* + * We are allowed to call namei() regardless of 2xUTIME_OMIT. + * POSIX states: + * "If both tv_nsec fields are UTIME_OMIT... EACCESS may be detected." + * "Search permission is denied by a component of the path prefix." + */ + vfslocked =3D NDHASGIANT(&nd); + NDFREE(&nd, NDF_ONLY_PNBUF); + if ((flags & UTIMENS_EXIT) =3D=3D 0) + error =3D setutimes(td, nd.ni_vp, ts, 2, flags & UTIMENS_NULL); + vrele(nd.ni_vp); + VFS_UNLOCK_GIANT(vfslocked); + return (error); +} + /* * Truncate a file given its path name. */ --=20 wbr, pluknet
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAE-mSOJU=hm8%2B-AC_oQmx%2Bh2grv7PGaH7kNYKoT3GMePDPXsYg>