Index | Thread | Search

From:
Helg <helg-openbsd@gmx.de>
Subject:
fuse: readlink shouldn't silently truncate
To:
tech@openbsd.org
Date:
Mon, 8 Sep 2025 20:25:39 +0200

Download raw body.

Thread
The fuse vfs syscall readlink doesn't check the size of the buffer
passed to it. This patch now uses checks the buffer size without making
any assumptions. It also adds a few other related checks.

Note that Linux libfuse uses a buffer of PATH_MAX + 1 but in my testing
this can sometimes be too small. For example, if I pass a larger buffer
to readlink(2) then the syscall uses this size. I think that libfuse
should not make any assumptions and use fb_io_len as passed by the
kernel. It's up to the file system if it wants to use the POSIX PATH_MAX
limit.

I've added a sanity check in fuse_devic.c to check that the buffer
returned from userland is not larger than fb_io_len. The buffer would
simply be truncated by uiomove in fuse_vnops.c later but silently
truncating is not ideal. This will now also protect read and readdir.

The NUL check in fusefs_readlink was inspired by FreeBSD.

Lastly, I noticed that readdir was not mentioned in the man page so
added it.

OK?


Index: lib/libfuse/fuse_new.3
===================================================================
RCS file: /cvs/src/lib/libfuse/fuse_new.3,v
diff -u -p -r1.8 fuse_new.3
--- lib/libfuse/fuse_new.3	10 Jun 2025 12:55:33 -0000	1.8
+++ lib/libfuse/fuse_new.3	8 Sep 2025 18:00:07 -0000
@@ -179,6 +179,9 @@ The O_CREAT and O_TRUNC flags are never 
 the mknod and truncate operations are invoked before open instead.
 .It opendir
 Called when a directory is opened for reading.
+.It readlink
+Called to read the target of a symbolic link.
+The path must be NUL-terminated.
 .It release
 Called when there are no more references to the file.
 .It releasedir
Index: lib/libfuse/fuse_ops.c
===================================================================
RCS file: /cvs/src/lib/libfuse/fuse_ops.c,v
diff -u -p -r1.38 fuse_ops.c
--- lib/libfuse/fuse_ops.c	6 Sep 2025 06:15:52 -0000	1.38
+++ lib/libfuse/fuse_ops.c	8 Sep 2025 18:00:07 -0000
@@ -25,6 +25,7 @@
 #define CHECK_OPT(opname)	DPRINTF("Opcode: %s\t", #opname);	\
 				DPRINTF("Inode: %llu\t",		\
 				    (unsigned long long)fbuf->fb_ino);	\
+				DPRINTF("Size: %lu\t", fbuf->fb_io_len);\
 				if (!f->op.opname) {			\
 					fbuf->fb_err = -ENOSYS;		\
 					return (0);			\
@@ -678,13 +679,11 @@ static int
 ifuse_ops_readlink(struct fuse *f, struct fusebuf *fbuf)
 {
 	struct fuse_vnode *vn;
+	size_t bufsize;
+	char *name;
 	char *realname;
-	char name[PATH_MAX + 1];
-	int len, ret;
-
-	DPRINTF("Opcode: readlink\t");
-	DPRINTF("Inode: %llu\t", (unsigned long long)fbuf->fb_ino);
 
+	CHECK_OPT(readlink);
 	vn = tree_get(&f->vnode_tree, fbuf->fb_ino);
 	if (vn == NULL) {
 		fbuf->fb_err = -errno;
@@ -697,19 +696,30 @@ ifuse_ops_readlink(struct fuse *f, struc
 		return (0);
 	}
 
-	if (f->op.readlink)
-		ret = f->op.readlink(realname, name, sizeof(name));
-	else
-		ret = -ENOSYS;
-	free(realname);
+	bufsize = fbuf->fb_io_len + 1;
+	name = calloc(bufsize, sizeof(*name));
+	if (name == NULL) {
+		fbuf->fb_err = -errno;
+		free(realname);
+		return (0);
+	}
+
+	fbuf->fb_err = f->op.readlink(realname, name, bufsize);
+
+	if (!fbuf->fb_err) {
+		/* file system should return a NUL-terminated string */
+		fbuf->fb_len = strnlen(name, bufsize);
 
-	fbuf->fb_err = ret;
-	if (!ret) {
-		len = strnlen(name, PATH_MAX);
-		fbuf->fb_len = len;
-		memcpy(fbuf->fb_dat, name, len);
+		/* string was not NUL-terminated, assume it was too long */
+		if (fbuf->fb_len == bufsize)
+			fbuf->fb_err = -ENAMETOOLONG;
+		else
+			memcpy(fbuf->fb_dat, name, fbuf->fb_len);
 	} else
 		fbuf->fb_len = 0;
+
+	free(realname);
+	free(name);
 
 	return (0);
 }
Index: sys/miscfs/fuse/fuse_device.c
===================================================================
RCS file: /cvs/src/sys/miscfs/fuse/fuse_device.c,v
diff -u -p -r1.44 fuse_device.c
--- sys/miscfs/fuse/fuse_device.c	8 Sep 2025 17:25:46 -0000	1.44
+++ sys/miscfs/fuse/fuse_device.c	8 Sep 2025 18:00:18 -0000
@@ -380,7 +380,7 @@ fusewrite(dev_t dev, struct uio *uio, in
 	fbuf->fb_ino = hdr.fh_ino;
 
 	/* Check for corrupted fbufs */
-	if ((fbuf->fb_len && fbuf->fb_err) ||
+	if ((fbuf->fb_len && fbuf->fb_err) || fbuf->fb_len > fbuf->fb_io_len ||
 	    SIMPLEQ_EMPTY(&fd->fd_fbufs_wait)) {
 		printf("fuse: dropping corrupted fusebuf\n");
 		error = EINVAL;
Index: sys/miscfs/fuse/fuse_vnops.c
===================================================================
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
diff -u -p -r1.72 fuse_vnops.c
--- sys/miscfs/fuse/fuse_vnops.c	31 Oct 2024 13:55:21 -0000	1.72
+++ sys/miscfs/fuse/fuse_vnops.c	8 Sep 2025 18:00:18 -0000
@@ -909,7 +909,7 @@ fusefs_readlink(void *v)
 	struct fusebuf *fbuf;
 	struct uio *uio;
 	struct proc *p;
-	int error = 0;
+	int error;
 
 	ip = VTOI(vp);
 	fmp = (struct fusefs_mnt *)ip->i_ump;
@@ -918,12 +918,19 @@ fusefs_readlink(void *v)
 
 	if (!fmp->sess_init)
 		return (ENXIO);
+        if (uio->uio_resid == 0)
+                return (0);
+        if (uio->uio_offset < 0)
+                return (EINVAL);
 
 	if (fmp->undef_op & UNDEF_READLINK)
 		return (ENOSYS);
 
 	fbuf = fb_setup(0, ip->i_number, FBT_READLINK, p);
 
+	fbuf->fb_io_off = uio->uio_offset;
+	fbuf->fb_io_len = MIN(uio->uio_resid, fmp->max_read);
+
 	error = fb_queue(fmp->dev, fbuf);
 
 	if (error) {
@@ -934,6 +941,14 @@ fusefs_readlink(void *v)
 		return (error);
 	}
 
+	if (strnlen(fbuf->fb_dat, fbuf->fb_len) != fbuf->fb_len) {
+		printf("fusefs: symbolic link contains embedded NUL: %s\n",
+		   fbuf->fb_dat);
+
+		fb_delete(fbuf);
+		return (EIO);
+	}
+
 	error = uiomove(fbuf->fb_dat, fbuf->fb_len, uio);
 	fb_delete(fbuf);
 
@@ -1140,7 +1155,7 @@ fusefs_read(void *v)
 	struct fusefs_mnt *fmp;
 	struct fusebuf *fbuf = NULL;
 	size_t size;
-	int error=0;
+	int error;
 
 	ip = VTOI(vp);
 	fmp = (struct fusefs_mnt *)ip->i_ump;
@@ -1148,7 +1163,7 @@ fusefs_read(void *v)
 	if (!fmp->sess_init)
 		return (ENXIO);
 	if (uio->uio_resid == 0)
-		return (error);
+		return (0);
 	if (uio->uio_offset < 0)
 		return (EINVAL);
 
@@ -1194,7 +1209,7 @@ fusefs_write(void *v)
 	struct fusefs_mnt *fmp;
 	struct fusebuf *fbuf = NULL;
 	size_t len, diff;
-	int error=0;
+	int error;
 
 	ip = VTOI(vp);
 	fmp = (struct fusefs_mnt *)ip->i_ump;
@@ -1202,7 +1217,7 @@ fusefs_write(void *v)
 	if (!fmp->sess_init)
 		return (ENXIO);
 	if (uio->uio_resid == 0)
-		return (error);
+		return (0);
 
 	if (ioflag & IO_APPEND) {
 		if ((error = VOP_GETATTR(vp, &vattr, cred, p)) != 0)