From: Helg Subject: fuse: readlink shouldn't silently truncate To: tech@openbsd.org Date: Mon, 8 Sep 2025 20:25:39 +0200 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)