From: Miklos Szeredi Date: Thu, 14 May 2020 19:17:50 +0000 (+0200) Subject: passthrough_ll: remove symlink fallbacks (#514) X-Git-Tag: fuse-3.9.2~3 X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=ded868455aecc883ecc53b1fac7356dba761b240;p=qemu-gpiodev%2Flibfuse.git passthrough_ll: remove symlink fallbacks (#514) * passthrough_ll/hp: remove symlink fallbacks Path lookup in the kernel has special rules for looking up magic symlinks under /proc. If a filesystem operation is instructed to follow symlinks (e.g. via AT_SYMLINK_FOLLOW or lack of AT_SYMLINK_NOFOLLOW), and the final component is such a proc symlink, then the target of the magic symlink is used for the operation, even if the target itself is a symlink. I.e. path lookup is always terminated after following a final magic symlink. I was erronously assuming that in the above case the target symlink would also be followed, and so workarounds were added for a couple of operations to handle the symlink case. Since the symlink can be handled simply by following the proc symlink, these workardouds are not needed. Signed-off-by: Miklos Szeredi Co-authored-by: Miklos Szeredi --- diff --git a/example/passthrough_hp.cc b/example/passthrough_hp.cc index ddede52..3e11266 100644 --- a/example/passthrough_hp.cc +++ b/example/passthrough_hp.cc @@ -121,7 +121,6 @@ typedef std::unordered_map InodeMap; struct Inode { int fd {-1}; - bool is_symlink {false}; dev_t src_dev {0}; ino_t src_ino {0}; uint64_t nlookup {0}; @@ -217,28 +216,6 @@ static void sfs_getattr(fuse_req_t req, fuse_ino_t ino, fuse_file_info *fi) { } -#ifdef HAVE_UTIMENSAT -static int utimensat_empty_nofollow(Inode& inode, - const struct timespec *tv) { - if (inode.is_symlink) { - /* Does not work on current kernels, but may in the future: - https://marc.info/?l=linux-kernel&m=154158217810354&w=2 */ - auto res = utimensat(inode.fd, "", tv, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); - if (res == -1 && errno == EINVAL) { - /* Sorry, no race free way to set times on symlink. */ - errno = EPERM; - } - return res; - } - - char procname[64]; - sprintf(procname, "/proc/self/fd/%i", inode.fd); - - return utimensat(AT_FDCWD, procname, tv, 0); -} -#endif - - static void do_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, int valid, struct fuse_file_info* fi) { Inode& inode = get_inode(ino); @@ -297,7 +274,9 @@ static void do_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, res = futimens(fi->fh, tv); else { #ifdef HAVE_UTIMENSAT - res = utimensat_empty_nofollow(inode, tv); + char procname[64]; + sprintf(procname, "/proc/self/fd/%i", ifd); + res = utimensat(AT_FDCWD, procname, tv, 0); #else res = -1; errno = EOPNOTSUPP; @@ -378,7 +357,6 @@ static int do_lookup(fuse_ino_t parent, const char *name, lock_guard g {inode.m}; inode.src_ino = e->attr.st_ino; inode.src_dev = e->attr.st_dev; - inode.is_symlink = S_ISLNK(e->attr.st_mode); inode.nlookup = 1; inode.fd = newfd; fs_lock.unlock(); @@ -460,22 +438,6 @@ static void sfs_symlink(fuse_req_t req, const char *link, fuse_ino_t parent, } -static int linkat_empty_nofollow(Inode& inode, int dfd, const char *name) { - if (inode.is_symlink) { - auto res = linkat(inode.fd, "", dfd, name, AT_EMPTY_PATH); - if (res == -1 && (errno == ENOENT || errno == EINVAL)) { - /* Sorry, no race free way to hard-link a symlink. */ - errno = EOPNOTSUPP; - } - return res; - } - - char procname[64]; - sprintf(procname, "/proc/self/fd/%i", inode.fd); - return linkat(AT_FDCWD, procname, dfd, name, AT_SYMLINK_FOLLOW); -} - - static void sfs_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, const char *name) { Inode& inode = get_inode(ino); @@ -485,7 +447,9 @@ static void sfs_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, e.attr_timeout = fs.timeout; e.entry_timeout = fs.timeout; - auto res = linkat_empty_nofollow(inode, inode_p.fd, name); + char procname[64]; + sprintf(procname, "/proc/self/fd/%i", inode.fd); + auto res = linkat(AT_FDCWD, procname, inode_p.fd, name, AT_SYMLINK_FOLLOW); if (res == -1) { fuse_reply_err(req, errno); return; @@ -965,12 +929,6 @@ static void sfs_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, ssize_t ret; int saverr; - if (inode.is_symlink) { - /* Sorry, no race free way to getxattr on symlink. */ - saverr = ENOTSUP; - goto out; - } - char procname[64]; sprintf(procname, "/proc/self/fd/%i", inode.fd); @@ -1014,12 +972,6 @@ static void sfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) { ssize_t ret; int saverr; - if (inode.is_symlink) { - /* Sorry, no race free way to listxattr on symlink. */ - saverr = ENOTSUP; - goto out; - } - char procname[64]; sprintf(procname, "/proc/self/fd/%i", inode.fd); @@ -1062,19 +1014,12 @@ static void sfs_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, ssize_t ret; int saverr; - if (inode.is_symlink) { - /* Sorry, no race free way to setxattr on symlink. */ - saverr = ENOTSUP; - goto out; - } - char procname[64]; sprintf(procname, "/proc/self/fd/%i", inode.fd); ret = setxattr(procname, name, value, size, flags); saverr = ret == -1 ? errno : 0; -out: fuse_reply_err(req, saverr); } @@ -1085,17 +1030,10 @@ static void sfs_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) { ssize_t ret; int saverr; - if (inode.is_symlink) { - /* Sorry, no race free way to setxattr on symlink. */ - saverr = ENOTSUP; - goto out; - } - sprintf(procname, "/proc/self/fd/%i", inode.fd); ret = removexattr(procname, name); saverr = ret == -1 ? errno : 0; -out: fuse_reply_err(req, saverr); } #endif @@ -1221,7 +1159,6 @@ int main(int argc, char *argv[]) { // Initialize filesystem root fs.root.fd = -1; fs.root.nlookup = 9999; - fs.root.is_symlink = false; fs.timeout = options.count("nocache") ? 0 : 86400.0; struct stat stat; diff --git a/example/passthrough_ll.c b/example/passthrough_ll.c index 5667223..06bb50f 100644 --- a/example/passthrough_ll.c +++ b/example/passthrough_ll.c @@ -74,7 +74,6 @@ struct lo_inode { struct lo_inode *next; /* protected by lo->mutex */ struct lo_inode *prev; /* protected by lo->mutex */ int fd; - bool is_symlink; ino_t ino; dev_t dev; uint64_t refcount; /* protected by lo->mutex */ @@ -188,26 +187,6 @@ static void lo_getattr(fuse_req_t req, fuse_ino_t ino, fuse_reply_attr(req, &buf, lo->timeout); } -static int utimensat_empty_nofollow(struct lo_inode *inode, - const struct timespec *tv) -{ - int res; - char procname[64]; - - if (inode->is_symlink) { - res = utimensat(inode->fd, "", tv, - AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); - if (res == -1 && errno == EINVAL) { - /* Sorry, no race free way to set times on symlink. */ - errno = EPERM; - } - return res; - } - sprintf(procname, "/proc/self/fd/%i", inode->fd); - - return utimensat(AT_FDCWD, procname, tv, 0); -} - static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, int valid, struct fuse_file_info *fi) { @@ -268,8 +247,10 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, if (fi) res = futimens(fi->fh, tv); - else - res = utimensat_empty_nofollow(inode, tv); + else { + sprintf(procname, "/proc/self/fd/%i", ifd); + res = utimensat(AT_FDCWD, procname, tv, 0); + } if (res == -1) goto out_err; } @@ -332,7 +313,6 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, if (!inode) goto out_err; - inode->is_symlink = S_ISLNK(e->attr.st_mode); inode->refcount = 1; inode->fd = newfd; inode->ino = e->attr.st_ino; @@ -426,26 +406,6 @@ static void lo_symlink(fuse_req_t req, const char *link, lo_mknod_symlink(req, parent, name, S_IFLNK, 0, link); } -static int linkat_empty_nofollow(struct lo_inode *inode, int dfd, - const char *name) -{ - int res; - char procname[64]; - - if (inode->is_symlink) { - res = linkat(inode->fd, "", dfd, name, AT_EMPTY_PATH); - if (res == -1 && (errno == ENOENT || errno == EINVAL)) { - /* Sorry, no race free way to hard-link a symlink. */ - errno = EPERM; - } - return res; - } - - sprintf(procname, "/proc/self/fd/%i", inode->fd); - - return linkat(AT_FDCWD, procname, dfd, name, AT_SYMLINK_FOLLOW); -} - static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, const char *name) { @@ -453,13 +413,16 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, struct lo_data *lo = lo_data(req); struct lo_inode *inode = lo_inode(req, ino); struct fuse_entry_param e; + char procname[64]; int saverr; memset(&e, 0, sizeof(struct fuse_entry_param)); e.attr_timeout = lo->timeout; e.entry_timeout = lo->timeout; - res = linkat_empty_nofollow(inode, lo_fd(req, parent), name); + sprintf(procname, "/proc/self/fd/%i", inode->fd); + res = linkat(AT_FDCWD, procname, lo_fd(req, parent), name, + AT_SYMLINK_FOLLOW); if (res == -1) goto out_err; @@ -976,12 +939,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, ino, name, size); } - if (inode->is_symlink) { - /* Sorry, no race free way to getxattr on symlink. */ - saverr = EPERM; - goto out; - } - sprintf(procname, "/proc/self/fd/%i", inode->fd); if (size) { @@ -1032,12 +989,6 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) ino, size); } - if (inode->is_symlink) { - /* Sorry, no race free way to listxattr on symlink. */ - saverr = EPERM; - goto out; - } - sprintf(procname, "/proc/self/fd/%i", inode->fd); if (size) { @@ -1088,12 +1039,6 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, ino, name, value, size); } - if (inode->is_symlink) { - /* Sorry, no race free way to setxattr on symlink. */ - saverr = EPERM; - goto out; - } - sprintf(procname, "/proc/self/fd/%i", inode->fd); ret = setxattr(procname, name, value, size, flags); @@ -1119,12 +1064,6 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) ino, name); } - if (inode->is_symlink) { - /* Sorry, no race free way to setxattr on symlink. */ - saverr = EPERM; - goto out; - } - sprintf(procname, "/proc/self/fd/%i", inode->fd); ret = removexattr(procname, name); @@ -1274,7 +1213,6 @@ int main(int argc, char *argv[]) } else { lo.source = "/"; } - lo.root.is_symlink = false; if (!lo.timeout_set) { switch (lo.cache) { case CACHE_NEVER: