standards/159679: [patch] [standards] fchmod(2) fails on descriptor
referencing shared memory
Gleb Smirnoff
glebius at FreeBSD.org
Thu Aug 11 11:30:09 UTC 2011
>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 at 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 at 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:
More information about the freebsd-standards
mailing list