svn commit: r345808 - projects/fuse2/sys/fs/fuse

Alan Somers asomers at FreeBSD.org
Tue Sep 3 14:06:08 UTC 2019


Author: asomers
Date: Tue Apr  2 18:09:40 2019
New Revision: 345808
URL: https://svnweb.freebsd.org/changeset/base/345808

Log:
  fusefs: cleanup and refactor some recent commits
  
  This commit cleans up after recent commits, especially 345766, 345768, and
  345781.  There is no functional change.  The most important change is to add
  comments documenting why we can't send flags like O_APPEND in
  FUSE_WRITE_OPEN.
  
  PR:		236340
  Sponsored by:	The FreeBSD Foundation

Modified:
  projects/fuse2/sys/fs/fuse/fuse_file.c
  projects/fuse2/sys/fs/fuse/fuse_file.h
  projects/fuse2/sys/fs/fuse/fuse_vnops.c

Modified: projects/fuse2/sys/fs/fuse/fuse_file.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_file.c	Tue Apr  2 18:01:54 2019	(r345807)
+++ projects/fuse2/sys/fs/fuse/fuse_file.c	Tue Apr  2 18:09:40 2019	(r345808)
@@ -97,22 +97,75 @@ static int fuse_fh_count = 0;
 SYSCTL_INT(_vfs_fusefs, OID_AUTO, filehandle_count, CTLFLAG_RD,
     &fuse_fh_count, 0, "number of open FUSE filehandles");
 
+/* Get the FUFH type for a particular access mode */
+static inline fufh_type_t
+fflags_2_fufh_type(int fflags)
+{
+	if ((fflags & FREAD) && (fflags & FWRITE))
+		return FUFH_RDWR;
+	else if (fflags & (FWRITE))
+		return FUFH_WRONLY;
+	else if (fflags & (FREAD))
+		return FUFH_RDONLY;
+	else if (fflags & (FEXEC))
+		return FUFH_EXEC;
+	else
+		panic("FUSE: What kind of a flag is this (%x)?", fflags);
+}
+
+/*
+ * Get the flags to use for FUSE_CREATE, FUSE_OPEN and FUSE_RELEASE
+ *
+ * These are supposed to be the same as the flags argument to open(2).
+ * However, since we can't reliably associate a fuse_filehandle with a specific
+ * file descriptor it would would be dangerous to include anything more than
+ * the access mode flags.  For example, suppose we open a file twice, once with
+ * O_APPEND and once without.  Then the user pwrite(2)s to offset using the
+ * second file descriptor.  If fusefs uses the first file handle, then the
+ * server may append the write to the end of the file rather than at offset 0.
+ * To prevent problems like this, we only ever send the portion of flags
+ * related to access mode.
+ *
+ * It's essential to send that portion, because FUSE uses it for server-side
+ * authorization.
+ *
+ * TODO: consider sending O_APPEND after upgrading to protocol 7.9, which
+ * includes flags in fuse_write_in.
+ */
+static inline int
+fufh_type_2_fflags(fufh_type_t type)
+{
+	int oflags = -1;
+
+	switch (type) {
+	case FUFH_RDONLY:
+	case FUFH_WRONLY:
+	case FUFH_RDWR:
+	case FUFH_EXEC:
+		oflags = type;
+		break;
+	default:
+		break;
+	}
+
+	return oflags;
+}
+
 int
-fuse_filehandle_open(struct vnode *vp, fufh_type_t fufh_type,
+fuse_filehandle_open(struct vnode *vp, int a_mode,
     struct fuse_filehandle **fufhp, struct thread *td, struct ucred *cred)
 {
 	struct fuse_dispatcher fdi;
 	struct fuse_open_in *foi;
 	struct fuse_open_out *foo;
+	fufh_type_t fufh_type;
 
 	int err = 0;
 	int oflags = 0;
 	int op = FUSE_OPEN;
 
-	/*
-	 * Note that this means we are effectively FILTERING OUT open() flags.
-	 */
-	oflags = fuse_filehandle_xlate_to_oflags(fufh_type);
+	fufh_type = fflags_2_fufh_type(a_mode);
+	oflags = fufh_type_2_fflags(fufh_type);
 
 	if (vnode_isdir(vp)) {
 		op = FUSE_OPENDIR;
@@ -136,7 +189,7 @@ fuse_filehandle_open(struct vnode *vp, fufh_type_t fuf
 	foo = fdi.answ;
 
 	fuse_filehandle_init(vp, fufh_type, fufhp, td->td_proc->p_pid, cred,
-		foo->fh);
+		foo);
 
 	fuse_vnode_open(vp, foo->open_flags, td);
 
@@ -164,7 +217,7 @@ fuse_filehandle_close(struct vnode *vp, struct fuse_fi
 	fdisp_make_vp(&fdi, op, vp, td, cred);
 	fri = fdi.indata;
 	fri->fh = fufh->fh_id;
-	fri->flags = fufh->flags;
+	fri->flags = fufh_type_2_fflags(fufh->fufh_type);
 
 	err = fdisp_wait_answ(&fdi);
 	fdisp_destroy(&fdi);
@@ -185,18 +238,19 @@ out:
  * A pid of 0 means "don't care"
  */
 bool
-fuse_filehandle_validrw(struct vnode *vp, fufh_type_t fufh_type,
+fuse_filehandle_validrw(struct vnode *vp, int mode,
 	struct ucred *cred, pid_t pid)
 {
 	struct fuse_vnode_data *fvdat = VTOFUD(vp);
 	struct fuse_filehandle *fufh;
+	fufh_type_t fufh_type = fflags_2_fufh_type(mode);
 
 	/* 
 	 * Unlike fuse_filehandle_get, we want to search for a filehandle with
 	 * the exact cred, and no fallback
 	 */
 	LIST_FOREACH(fufh, &fvdat->handles, next) {
-		if (fufh->flags == fufh_type &&
+		if (fufh->fufh_type == fufh_type &&
 		    fufh->uid == cred->cr_uid &&
 		    fufh->gid == cred->cr_rgid &&
 		    (pid == 0 || fufh->pid == pid))
@@ -208,7 +262,7 @@ fuse_filehandle_validrw(struct vnode *vp, fufh_type_t 
 
 	/* Fallback: find a RDWR list entry with the right cred */
 	LIST_FOREACH(fufh, &fvdat->handles, next) {
-		if (fufh->flags == FUFH_RDWR &&
+		if (fufh->fufh_type == FUFH_RDWR &&
 		    fufh->uid == cred->cr_uid &&
 		    fufh->gid == cred->cr_rgid &&
 		    (pid == 0 || fufh->pid == pid))
@@ -229,7 +283,7 @@ fuse_filehandle_get(struct vnode *vp, fufh_type_t fufh
 		goto fallback;
 
 	LIST_FOREACH(fufh, &fvdat->handles, next) {
-		if (fufh->flags == fufh_type &&
+		if (fufh->fufh_type == fufh_type &&
 		    fufh->uid == cred->cr_uid &&
 		    fufh->gid == cred->cr_rgid &&
 		    (pid == 0 || fufh->pid == pid))
@@ -239,7 +293,7 @@ fuse_filehandle_get(struct vnode *vp, fufh_type_t fufh
 fallback:
 	/* Fallback: find a list entry with the right flags */
 	LIST_FOREACH(fufh, &fvdat->handles, next) {
-		if (fufh->flags == fufh_type)
+		if (fufh->fufh_type == fufh_type)
 			break;
 	}
 
@@ -267,7 +321,7 @@ fuse_filehandle_getrw(struct vnode *vp, fufh_type_t fu
 void
 fuse_filehandle_init(struct vnode *vp, fufh_type_t fufh_type,
     struct fuse_filehandle **fufhp, pid_t pid, struct ucred *cred,
-    uint64_t fh_id)
+    struct fuse_open_out *foo)
 {
 	struct fuse_vnode_data *fvdat = VTOFUD(vp);
 	struct fuse_filehandle *fufh;
@@ -275,12 +329,12 @@ fuse_filehandle_init(struct vnode *vp, fufh_type_t fuf
 	fufh = malloc(sizeof(struct fuse_filehandle), M_FUSE_FILEHANDLE,
 		M_WAITOK);
 	MPASS(fufh != NULL);
-	fufh->fh_id = fh_id;
-	fufh->flags = fufh_type;
+	fufh->fh_id = foo->fh;
+	fufh->fufh_type = fufh_type;
 	fufh->gid = cred->cr_rgid;
 	fufh->uid = cred->cr_uid;
 	fufh->pid = pid;
-	/* TODO: initialize open flags */
+	fufh->fuse_open_flags = foo->open_flags;
 	if (!FUFH_IS_VALID(fufh)) {
 		panic("FUSE: init: invalid filehandle id (type=%d)", fufh_type);
 	}

Modified: projects/fuse2/sys/fs/fuse/fuse_file.h
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_file.h	Tue Apr  2 18:01:54 2019	(r345807)
+++ projects/fuse2/sys/fs/fuse/fuse_file.h	Tue Apr  2 18:09:40 2019	(r345808)
@@ -137,8 +137,8 @@ struct fuse_filehandle {
 	/* flags returned by FUSE_OPEN */
 	uint32_t fuse_open_flags;
 
-	/* The flags used to open(2) the file (using O_RDONLY, not FREAD) */
-	uint32_t flags;
+	/* The access mode of the file handle */
+	fufh_type_t fufh_type;
 
 	/* Credentials used to open the file */
 	gid_t gid;
@@ -146,43 +146,9 @@ struct fuse_filehandle {
 	uid_t uid;
 };
 
-#define FUFH_IS_VALID(f)  ((f)->flags != FUFH_INVALID)
+#define FUFH_IS_VALID(f)  ((f)->fufh_type != FUFH_INVALID)
 
-static inline fufh_type_t
-fuse_filehandle_xlate_from_fflags(int fflags)
-{
-	if ((fflags & FREAD) && (fflags & FWRITE))
-		return FUFH_RDWR;
-	else if (fflags & (FWRITE))
-		return FUFH_WRONLY;
-	else if (fflags & (FREAD))
-		return FUFH_RDONLY;
-	else if (fflags & (FEXEC))
-		return FUFH_EXEC;
-	else
-		panic("FUSE: What kind of a flag is this (%x)?", fflags);
-}
-
-static inline int
-fuse_filehandle_xlate_to_oflags(fufh_type_t type)
-{
-	int oflags = -1;
-
-	switch (type) {
-	case FUFH_RDONLY:
-	case FUFH_WRONLY:
-	case FUFH_RDWR:
-	case FUFH_EXEC:
-		oflags = type;
-		break;
-	default:
-		break;
-	}
-
-	return oflags;
-}
-
-bool fuse_filehandle_validrw(struct vnode *vp, fufh_type_t fufh_type,
+bool fuse_filehandle_validrw(struct vnode *vp, int mode,
 	struct ucred *cred, pid_t pid);
 int fuse_filehandle_get(struct vnode *vp, fufh_type_t fufh_type,
                         struct fuse_filehandle **fufhp, struct ucred *cred,
@@ -193,8 +159,8 @@ int fuse_filehandle_getrw(struct vnode *vp, fufh_type_
 
 void fuse_filehandle_init(struct vnode *vp, fufh_type_t fufh_type,
 		          struct fuse_filehandle **fufhp, pid_t pid,
-			  struct ucred *cred, uint64_t fh_id);
-int fuse_filehandle_open(struct vnode *vp, fufh_type_t fufh_type,
+			  struct ucred *cred, struct fuse_open_out *foo);
+int fuse_filehandle_open(struct vnode *vp, int mode,
                          struct fuse_filehandle **fufhp, struct thread *td,
                          struct ucred *cred);
 int fuse_filehandle_close(struct vnode *vp, struct fuse_filehandle *fufh,

Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_vnops.c	Tue Apr  2 18:01:54 2019	(r345807)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c	Tue Apr  2 18:09:40 2019	(r345808)
@@ -365,7 +365,15 @@ fuse_vnop_create(struct vop_create_args *ap)
 	uint64_t parentnid = VTOFUD(dvp)->nid;
 	mode_t mode = MAKEIMODE(vap->va_type, vap->va_mode);
 	enum fuse_opcode op;
+	int flags;
 
+	/* 
+	 * VOP_CREATE doesn't tell us the open(2) flags, so we guess.  Only a
+	 * writable mode makes sense, and we might as well include readability
+	 * too.
+	 */
+	flags = O_RDWR;
+
 	if (fuse_isdeadfs(dvp)) {
 		return ENXIO;
 	}
@@ -385,7 +393,7 @@ fuse_vnop_create(struct vop_create_args *ap)
 		fdisp_make(fdip, op, vnode_mount(dvp), parentnid, td, cred);
 		foi = fdip->indata;
 		foi->mode = mode;
-		foi->flags = O_CREAT | O_RDWR;
+		foi->flags = O_CREAT | flags;
 		memcpy((char *)fdip->indata + sizeof(*foi), cnp->cn_nameptr,
 		    cnp->cn_namelen);
 		((char *)fdip->indata)[sizeof(*foi) + cnp->cn_namelen] = '\0';
@@ -420,7 +428,7 @@ fuse_vnop_create(struct vop_create_args *ap)
 			cred);
 		foi = fdip2->indata;
 		foi->mode = mode;
-		foi->flags = O_RDWR;
+		foi->flags = flags;
 		err = fdisp_wait_answ(fdip2);
 		if (err)
 			goto out;
@@ -436,7 +444,7 @@ fuse_vnop_create(struct vop_create_args *ap)
 		fdisp_make(fdip, FUSE_RELEASE, mp, nodeid, td, cred);
 		fri = fdip->indata;
 		fri->fh = fh_id;
-		fri->flags = fuse_filehandle_xlate_to_oflags(FUFH_RDWR);
+		fri->flags = flags;
 		fuse_insert_callback(fdip->tick, fuse_internal_forget_callback);
 		fuse_insert_message(fdip->tick);
 		goto out;
@@ -444,7 +452,7 @@ fuse_vnop_create(struct vop_create_args *ap)
 	ASSERT_VOP_ELOCKED(*vpp, "fuse_vnop_create");
 
 	fuse_filehandle_init(*vpp, FUFH_RDWR, NULL, td->td_proc->p_pid, cred,
-		foo->fh);
+		foo);
 	fuse_vnode_open(*vpp, foo->open_flags, td);
 	cache_purge_negative(dvp);
 
@@ -1203,31 +1211,27 @@ static int
 fuse_vnop_open(struct vop_open_args *ap)
 {
 	struct vnode *vp = ap->a_vp;
-	int mode = ap->a_mode;
+	int a_mode = ap->a_mode;
 	struct thread *td = ap->a_td;
 	struct ucred *cred = ap->a_cred;
 	pid_t pid = td->td_proc->p_pid;
-
-	fufh_type_t fufh_type;
 	struct fuse_vnode_data *fvdat;
 
 	if (fuse_isdeadfs(vp))
 		return ENXIO;
 	if (vp->v_type == VCHR || vp->v_type == VBLK || vp->v_type == VFIFO)
 		return (EOPNOTSUPP);
-	if ((mode & (FREAD | FWRITE | FEXEC)) == 0)
+	if ((a_mode & (FREAD | FWRITE | FEXEC)) == 0)
 		return EINVAL;
 
 	fvdat = VTOFUD(vp);
 
-	fufh_type = fuse_filehandle_xlate_from_fflags(mode);
-
-	if (fuse_filehandle_validrw(vp, fufh_type, cred, pid)) {
+	if (fuse_filehandle_validrw(vp, a_mode, cred, pid)) {
 		fuse_vnode_open(vp, 0, td);
 		return 0;
 	}
 
-	return fuse_filehandle_open(vp, fufh_type, NULL, td, cred);
+	return fuse_filehandle_open(vp, a_mode, NULL, td, cred);
 }
 
 static int
@@ -1395,7 +1399,7 @@ fuse_vnop_reclaim(struct vop_reclaim_args *ap)
 	}
 	LIST_FOREACH_SAFE(fufh, &fvdat->handles, next, fufh_tmp) {
 		printf("FUSE: vnode being reclaimed with open fufh "
-			"(flags=%#x)", fufh->flags);
+			"(type=%#x)", fufh->fufh_type);
 		fuse_filehandle_close(vp, fufh, td, NULL);
 	}
 




More information about the svn-src-projects mailing list