diff --git a/sys/fs/fuse/fuse_ipc.h b/sys/fs/fuse/fuse_ipc.h --- a/sys/fs/fuse/fuse_ipc.h +++ b/sys/fs/fuse/fuse_ipc.h @@ -239,6 +239,7 @@ #define FSESS_WARN_CACHE_INCOHERENT 0x200000 /* Read cache incoherent */ #define FSESS_WARN_WB_CACHE_INCOHERENT 0x400000 /* WB cache incoherent */ #define FSESS_WARN_ILLEGAL_INODE 0x800000 /* Illegal inode for new file */ +#define FSESS_WARN_READLINK_EMBEDDED_NUL 0x1000000 /* corrupt READLINK output */ #define FSESS_MNTOPTS_MASK ( \ FSESS_DAEMON_CAN_SPY | FSESS_PUSH_SYMLINKS_IN | \ FSESS_DEFAULT_PERMISSIONS | FSESS_INTR) diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -2007,6 +2007,11 @@ if (err) { goto out; } + if (strlen(fdi.answ) + 1 < fdi.iosize) { + struct fuse_data *data = fuse_get_mpdata(vnode_mount(vp)); + fuse_warn(data, FSESS_WARN_READLINK_EMBEDDED_NUL, + "Returned an embedded NUL from FUSE_READLINK"); + } if (((char *)fdi.answ)[0] == '/' && fuse_get_mpdata(vnode_mount(vp))->dataflags & FSESS_PUSH_SYMLINKS_IN) { char *mpth = vnode_mount(vp)->mnt_stat.f_mntonname; diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c --- a/sys/kern/vfs_lookup.c +++ b/sys/kern/vfs_lookup.c @@ -990,7 +990,6 @@ { char *cp; /* pointer into pathname argument */ char *prev_ni_next; /* saved ndp->ni_next */ - char *nulchar; /* location of '\0' in cn_pnbuf */ char *lastchar; /* location of the last character */ struct vnode *dp = NULL; /* the directory we are searching */ struct vnode *tdp; /* saved dp */ @@ -1086,23 +1085,11 @@ * * The last component of the filename is left accessible via * cnp->cn_nameptr. It has to be freed with a call to NDFREE*. - * - * Store / as a temporary sentinel so that we only have one character - * to test for. Pathnames tend to be short so this should not be - * resulting in cache misses. */ - nulchar = &cnp->cn_nameptr[ndp->ni_pathlen - 1]; - KASSERT(*nulchar == '\0', - ("%s: expected nul at %p; string [%s]\n", __func__, nulchar, + KASSERT(cnp->cn_nameptr[ndp->ni_pathlen - 1] == '\0', + ("%s: expected nul at end of string; string [%s]\n", __func__, cnp->cn_pnbuf)); - *nulchar = '/'; - for (cp = cnp->cn_nameptr; *cp != '/'; cp++) { - KASSERT(*cp != '\0', - ("%s: encountered unexpected nul; string [%s]\n", __func__, - cnp->cn_nameptr)); - continue; - } - *nulchar = '\0'; + cp = strchrnul(cnp->cn_nameptr, '/'); cnp->cn_namelen = cp - cnp->cn_nameptr; if (__predict_false(cnp->cn_namelen > NAME_MAX)) { error = ENAMETOOLONG; diff --git a/tests/sys/fs/fusefs/readlink.cc b/tests/sys/fs/fusefs/readlink.cc --- a/tests/sys/fs/fusefs/readlink.cc +++ b/tests/sys/fs/fusefs/readlink.cc @@ -79,6 +79,42 @@ EXPECT_EQ(ELOOP, errno); } +// If a malicious or buggy server returns a NUL in the FUSE_READLINK result, it +// should be handled gracefully. +// https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=274268 +TEST_F(Readlink, embedded_nul) +{ + const char FULLPATH[] = "mountpoint/src"; + const char RELPATH[] = "src"; + const char dst[] = "dst\0stuff"; + const uint64_t ino = 42; + + EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFLNK | 0777; + out.body.entry.nodeid = ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_LOOKUP(FUSE_ROOT_ID, dst) + .WillOnce(Invoke(ReturnErrno(ENOENT))); + + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_READLINK && + in.header.nodeid == ino); + }, Eq(true)), + _) + ).WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + memcpy(out.body.str, dst, sizeof(dst)); + out.header.len = sizeof(out.header) + sizeof(dst) + 1; + }))); + + EXPECT_EQ(-1, access(FULLPATH, R_OK)); + EXPECT_EQ(ENOENT, errno); +} + TEST_F(Readlink, ok) { const char FULLPATH[] = "mountpoint/src";