Date: Thu, 11 Aug 2011 15:12:52 +0400 (MSK) From: Gleb Smirnoff <glebius@FreeBSD.org> To: FreeBSD-gnats-submit@FreeBSD.org Cc: Roland Olbricht <roland.olbricht@gmx.de> Subject: standards/159679: [patch] [standards] fchmod(2) fails on descriptor referencing shared memory Message-ID: <201108111112.p7BBCqqU001339@behemoth.ramtel.ru> Resent-Message-ID: <201108111130.p7BBU99K010419@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 159679 >Category: standards >Synopsis: [patch] [standards] fchmod(2) fails on descriptor referencing shared memory >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-standards >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Aug 11 11:30:08 UTC 2011 >Closed-Date: >Last-Modified: >Originator: Gleb Smirnoff <glebius@FreeBSD.org> >Release: FreeBSD 9.0-BETA1 amd64 >Organization: Rambler Internet Holding >Environment: System: FreeBSD behemoth.ramtel.ru 9.0-BETA1 FreeBSD 9.0-BETA1 #69 r224757M: Thu Aug 11 14:52:27 MSK 2011 glebius@behemoth.ramtel.ru:/usr/obj/usr/src/newcarp/sys/BEHEMOTH amd64 >Description: According to POSIX fchmod() syscall should be capable to deal with file descriptors referencing shared memory segments, with certain mask put on mode. http://pubs.opengroup.org/onlinepubs/9699919799/functions/fchmod.html In FreeBSD it returns EINVAL and doesn't change access mode. This is due to fchmod() requiring only vnode-backed filedescriptors. The attached patch fixes the problem. It makes some namespace pollution to vfs_syscalls.c, so probably a cleaner approach would be to move fchmod() to some not vfs-related file, may be kern_descrip.c? After applying this patch we also need to switch from ACCESSPERMS mask to a new mask consisting only flags mentioned in above URL. Probably mask can be called RWPERMS, defined in sys/stat.h and used in fchmod() as well as in shm_open(). #define RWPERMS (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH) /* 0666 */ >How-To-Repeat: #include <sys/mman.h> #include <sys/stat.h> #include <fcntl.h> #include <err.h> #include <stdio.h> int main(int argc, char* args[]) { int fd; if ((fd = shm_open("/foobar", O_RDWR|O_CREAT|O_TRUNC|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) err(1, "shm_open"); /* * http://pubs.opengroup.org/onlinepubs/9699919799/functions/fchmod.html * * [SHM] If fildes references a shared memory object, the fchmod() * function need only affect the * S_IRUSR, S_IWUSR, S_IRGRP, S_IWGRP, S_IROTH, and S_IWOTH * file permission bits. */ if (fchmod(fd, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH) != 0) { shm_unlink("/foobar"); err(1, "fchmod"); } printf("test successful\n"); shm_unlink("/foobar"); } >Fix: Index: vfs_syscalls.c =================================================================== --- vfs_syscalls.c (revision 224757) +++ vfs_syscalls.c (working copy) @@ -59,6 +59,7 @@ #include <sys/filio.h> #include <sys/limits.h> #include <sys/linker.h> +#include <sys/mman.h> #include <sys/sdt.h> #include <sys/stat.h> #include <sys/sx.h> @@ -101,6 +102,7 @@ const struct timespec *, int, int); static int vn_access(struct vnode *vp, int user_flags, struct ucred *cred, struct thread *td); +static int getfile(struct filedesc *fdp, int fd, struct file **fpp); /* * The module initialization routine for POSIX asynchronous I/O will @@ -2931,21 +2933,40 @@ } */ *uap; { struct file *fp; - int vfslocked; int error; AUDIT_ARG_FD(uap->fd); AUDIT_ARG_MODE(uap->mode); - if ((error = getvnode(td->td_proc->p_fd, uap->fd, &fp)) != 0) + if ((error = getfile(td->td_proc->p_fd, uap->fd, &fp)) != 0) return (error); - vfslocked = VFS_LOCK_GIANT(fp->f_vnode->v_mount); + switch (fp->f_type) { + case DTYPE_VNODE: + { + int vfslocked; + + vfslocked = 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); + vn_lock(fp->f_vnode, LK_SHARED | LK_RETRY); + AUDIT_ARG_VNODE1(fp->f_vnode); + VOP_UNLOCK(fp->f_vnode, 0); #endif - error = setfmode(td, fp->f_vnode, uap->mode); - VFS_UNLOCK_GIANT(vfslocked); + error = setfmode(td, fp->f_vnode, uap->mode); + VFS_UNLOCK_GIANT(vfslocked); + break; + } + case DTYPE_SHM: + { + struct shmfd *shmfd = fp->f_data; + + shmfd->shm_mode = uap->mode & ACCESSPERMS; + error = 0; + + break; + } + default: + error = EINVAL; + } + fdrop(fp, td); return (error); } @@ -4251,11 +4272,8 @@ * Convert a user file descriptor to a kernel file entry. * A reference on the file entry is held upon returning. */ -int -getvnode(fdp, fd, fpp) - struct filedesc *fdp; - int fd; - struct file **fpp; +static int +getfile(struct filedesc *fdp, int fd, struct file **fpp) { int error; struct file *fp; @@ -4264,11 +4282,24 @@ fp = NULL; if (fdp == NULL || (fp = fget_unlocked(fdp, fd)) == NULL) error = EBADF; - else if (fp->f_vnode == NULL) { + *fpp = fp; + return (error); +} + +/* + * Convert a user file descriptor to a kernel file entry, + * failing on a file that hasn't a vnode. + */ +int +getvnode(struct filedesc *fdp, int fd, struct file **fpp) +{ + int error; + + error = getfile(fdp, fd, fpp); + if (error == 0 && (*fpp)->f_vnode == NULL) { error = EINVAL; - fdrop(fp, curthread); + fdrop(*fpp, curthread); } - *fpp = fp; return (error); } >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108111112.p7BBCqqU001339>