From patchwork Thu Sep 16 08:40:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498285 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5CE99C433EF for ; Thu, 16 Sep 2021 08:45:54 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1ED7D61209 for ; Thu, 16 Sep 2021 08:45:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 1ED7D61209 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:49526 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn1k-00032D-1W for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:45:52 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48092) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmx8-000483-Sr for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:06 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:51770) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmx7-00043v-8u for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781664; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2Aa3s0DVFlM7JNUvl5FXoaHREIoYI6Yg29qn6jJ1Zg4=; b=V3pO26KQPTiiTTvjB8jkmXxWTWsSz7UkJYp6I4UPkeJiMx6HK/amKyPKpvSFCs97TKbBV6 0v8b/JHoPgkfwLxSFUiQEsv6NS3GBz2YEHtDorHtljtvIPk1etGIQUHupp2RofvbP4dVvH XxRB8uAMVTXGO/uUDSusRHzxwztlc9o= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-331-Gv9Tw8RDMZKh__W4DyV9SA-1; Thu, 16 Sep 2021 04:41:03 -0400 X-MC-Unique: Gv9Tw8RDMZKh__W4DyV9SA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 46F89835DE3 for ; Thu, 16 Sep 2021 08:41:02 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6F3AF6A91B; Thu, 16 Sep 2021 08:40:56 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 01/12] virtiofsd: Keep /proc/self/mountinfo open Date: Thu, 16 Sep 2021 10:40:34 +0200 Message-Id: <20210916084045.31684-2-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" File handles are specific to mounts, and so name_to_handle_at() returns the respective mount ID. However, open_by_handle_at() is not content with an ID, it wants a file descriptor for some inode on the mount, which we have to open. We want to use /proc/self/mountinfo to find the mounts' root directories so we can open them and pass the respective FDs to open_by_handle_at(). (We need to use the root directory, because we want the inode belonging to every mount FD be deletable. Before the root directory can be deleted, all entries within must have been closed, and so when it is deleted, there should not be any file handles left that need its FD as their mount FD. Thus, we can then close that FD and the inode can be deleted.[1]) That is why we need to open /proc/self/mountinfo so that we can use it to translate mount IDs into root directory paths. We have to open it after setup_mounts() was called, because if we try to open it before, it will appear as an empty file after setup_mounts(). [1] Note that in practice, you still cannot delete the mount root directory. It is a mount point on the host, after all, and mount points cannot be deleted. But by using the mount point as the mount FD, we will at least not hog any actually deletable inodes. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 38b2af8599..6511a6acb4 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -172,6 +172,8 @@ struct lo_data { /* An O_PATH file descriptor to /proc/self/fd/ */ int proc_self_fd; + /* A read-only FILE pointer for /proc/self/mountinfo */ + FILE *mountinfo_fp; int user_killpriv_v2, killpriv_v2; /* If set, virtiofsd is responsible for setting umask during creation */ bool change_umask; @@ -3718,6 +3720,19 @@ static void setup_chroot(struct lo_data *lo) static void setup_sandbox(struct lo_data *lo, struct fuse_session *se, bool enable_syslog) { + int proc_self, mountinfo_fd; + int saverr; + + /* + * Open /proc/self before we pivot to the new root so we can still + * open /proc/self/mountinfo afterwards + */ + proc_self = open("/proc/self", O_PATH); + if (proc_self < 0) { + fuse_log(FUSE_LOG_WARNING, "Failed to open /proc/self: %m; " + "will not be able to use file handles\n"); + } + if (lo->sandbox == SANDBOX_NAMESPACE) { setup_namespaces(lo, se); setup_mounts(lo->source); @@ -3725,6 +3740,31 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se, setup_chroot(lo); } + /* + * Opening /proc/self/mountinfo before the umount2() call in + * setup_mounts() leads to the file appearing empty. That is why + * we defer opening it until here. + */ + lo->mountinfo_fp = NULL; + if (proc_self >= 0) { + mountinfo_fd = openat(proc_self, "mountinfo", O_RDONLY); + if (mountinfo_fd < 0) { + saverr = errno; + } else if (mountinfo_fd >= 0) { + lo->mountinfo_fp = fdopen(mountinfo_fd, "r"); + if (!lo->mountinfo_fp) { + saverr = errno; + close(mountinfo_fd); + } + } + if (!lo->mountinfo_fp) { + fuse_log(FUSE_LOG_WARNING, "Failed to open /proc/self/mountinfo: " + "%s; will not be able to use file handles\n", + strerror(saverr)); + } + close(proc_self); + } + setup_seccomp(enable_syslog); setup_capabilities(g_strdup(lo->modcaps)); } From patchwork Thu Sep 16 08:40:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498283 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 43D2AC433EF for ; Thu, 16 Sep 2021 08:44:58 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0F05961209 for ; Thu, 16 Sep 2021 08:44:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0F05961209 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:46974 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn0r-0001JU-1r for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:44:57 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48132) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxI-0004R2-UN for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:16 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:40801) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxG-0004BU-0u for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781673; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/LNdYWdfE7B711WCrLYvyDud7WHfFEqV3dUEWIbBA+E=; b=PjPV9i318SagWAZUs2jMKq0AwDgVXOHmqidGgUc0E4E3c2EP4b9FB42SNE4X8fqWvWCxYP xERrfmbutx6zviiTjVuYNqUbUs6c4Ta7pJ7qLTcN2XwommDsfr23uNG1NCMqyR2UqD7InR ihkqzxkwWugaYUwcOhD4ybJwGKhtmwg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-371-rXVcPl1FOM6oBR-XnebC4Q-1; Thu, 16 Sep 2021 04:41:12 -0400 X-MC-Unique: rXVcPl1FOM6oBR-XnebC4Q-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7D79E81451E for ; Thu, 16 Sep 2021 08:41:11 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0462819736; Thu, 16 Sep 2021 08:41:03 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 02/12] virtiofsd: Limit setxattr()'s creds-dropped region Date: Thu, 16 Sep 2021 10:40:35 +0200 Message-Id: <20210916084045.31684-3-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" We only need to drop/switch our credentials for the (f)setxattr() call alone, not for the openat() or fchdir() around it. (Right now, this may not be that big of a problem, but with inodes being identified by file handles instead of an O_PATH fd, we will need open_by_handle_at() calls here, which is really fickle when it comes to credentials being dropped.) Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 34 +++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 6511a6acb4..b43afdfbd3 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -3123,6 +3123,7 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, bool switched_creds = false; bool cap_fsetid_dropped = false; struct lo_cred old = {}; + bool changed_cwd = false; if (block_xattr(lo, in_name)) { fuse_reply_err(req, EOPNOTSUPP); @@ -3158,6 +3159,24 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, ", name=%s value=%s size=%zd)\n", ino, name, value, size); sprintf(procname, "%i", inode->fd); + /* + * We can only open regular files or directories. If the inode is + * something else, we have to enter /proc/self/fd and use + * setxattr() on the link's filename there. + */ + if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { + fd = openat(lo->proc_self_fd, procname, O_RDONLY); + if (fd < 0) { + saverr = errno; + goto out; + } + } else { + /* fchdir should not fail here */ + FCHDIR_NOFAIL(lo->proc_self_fd); + /* Set flag so the clean-up path will chdir back */ + changed_cwd = true; + } + /* * If we are setting posix access acl and if SGID needs to be * cleared, then switch to caller's gid and drop CAP_FSETID @@ -3178,20 +3197,12 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, } switched_creds = true; } - if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = openat(lo->proc_self_fd, procname, O_RDONLY); - if (fd < 0) { - saverr = errno; - goto out; - } + if (fd >= 0) { ret = fsetxattr(fd, name, value, size, flags); saverr = ret == -1 ? errno : 0; } else { - /* fchdir should not fail here */ - FCHDIR_NOFAIL(lo->proc_self_fd); ret = setxattr(procname, name, value, size, flags); saverr = ret == -1 ? errno : 0; - FCHDIR_NOFAIL(lo->root.fd); } if (switched_creds) { if (cap_fsetid_dropped) @@ -3201,6 +3212,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, } out: + if (changed_cwd) { + /* Change CWD back, fchdir should not fail here */ + FCHDIR_NOFAIL(lo->root.fd); + } + if (fd >= 0) { close(fd); } From patchwork Thu Sep 16 08:40:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498291 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19B46C433EF for ; Thu, 16 Sep 2021 08:47:57 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B492A61178 for ; Thu, 16 Sep 2021 08:47:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B492A61178 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:55396 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn3j-00072H-UI for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:47:55 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48168) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxQ-0004jU-8j for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:24 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:24753) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxO-0004Gy-Mq for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781681; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3XOocYzAQPSU4w9oDdGDlaI8wcduwC0/uQDUpjlWy54=; b=PcZiqASW1IFt9gJBreJ/ucjuoGGrYorhB1LWdtJXXN5J9pYLfYaZFaZeBb/o1k0F6hOAgH h5K3cAfn59J58Y/swzRS1K2i955IaG7obOnPHXFEZkc36t1gnihgCZ7l0kepKgHQm+xAed vuGAmqNwktLmbsr267v13oWg+thSYEI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-506-lQ9XCvOXNa2XCG3YHGwXyQ-1; Thu, 16 Sep 2021 04:41:20 -0400 X-MC-Unique: lQ9XCvOXNa2XCG3YHGwXyQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C9FE819200C0 for ; Thu, 16 Sep 2021 08:41:19 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 52EB460BD8; Thu, 16 Sep 2021 08:41:13 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 03/12] virtiofsd: Add TempFd structure Date: Thu, 16 Sep 2021 10:40:36 +0200 Message-Id: <20210916084045.31684-4-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" We are planning to add file handles to lo_inode objects as an alternative to lo_inode.fd. That means that everywhere where we currently reference lo_inode.fd, we will have to open a temporary file descriptor that needs to be closed after use. So instead of directly accessing lo_inode.fd, there will be a helper function (lo_inode_fd()) that either returns lo_inode.fd, or opens a new file descriptor with open_by_handle_at(). It encapsulates this result in a TempFd structure to let the caller know whether the FD needs to be closed after use (opened from the handle) or not (copied from lo_inode.fd). By using g_auto(TempFd) to store this result, callers will not even have to care about closing a temporary FD after use. It will be done automatically once the object goes out of scope. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 63 ++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index b43afdfbd3..88318a4ba9 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -180,6 +180,28 @@ struct lo_data { int user_posix_acl, posix_acl; }; +/** + * Represents a file descriptor that may either be owned by this + * TempFd, or only referenced (i.e. the ownership belongs to some + * other object, and the value has just been copied into this TempFd). + * + * The purpose of this encapsulation is to be used as g_auto(TempFd) + * to automatically clean up owned file descriptors when this object + * goes out of scope. + * + * Use temp_fd_steal() to get an owned file descriptor that will not + * be closed when the TempFd goes out of scope. + */ +typedef struct { + int fd; + bool owned; /* fd owned by this object? */ +} TempFd; + +#define TEMP_FD_INIT ((TempFd) { .fd = -1, .owned = false }) + +static void temp_fd_clear(TempFd *temp_fd); +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(TempFd, temp_fd_clear); + static const struct fuse_opt lo_opts[] = { { "sandbox=namespace", offsetof(struct lo_data, sandbox), @@ -257,6 +279,47 @@ static struct lo_data *lo_data(fuse_req_t req) return (struct lo_data *)fuse_req_userdata(req); } +/** + * Clean-up function for TempFds + */ +static void temp_fd_clear(TempFd *temp_fd) +{ + if (temp_fd->owned) { + close(temp_fd->fd); + *temp_fd = TEMP_FD_INIT; + } +} + +/** + * Return an owned fd from *temp_fd that will not be closed when + * *temp_fd goes out of scope. + * + * (TODO: Remove __attribute__ once this is used.) + */ +static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd) +{ + if (temp_fd->owned) { + temp_fd->owned = false; + return temp_fd->fd; + } else { + return dup(temp_fd->fd); + } +} + +/** + * Create a borrowing copy of an existing TempFd. Note that *to is + * only valid as long as *from is valid. + * + * (TODO: Remove __attribute__ once this is used.) + */ +static __attribute__((unused)) void temp_fd_copy(const TempFd *from, TempFd *to) +{ + *to = (TempFd) { + .fd = from->fd, + .owned = false, + }; +} + /* * Load capng's state from our saved state if the current thread * hadn't previously been loaded. From patchwork Thu Sep 16 08:40:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498263 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0A781C433EF for ; Thu, 16 Sep 2021 08:43:46 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9B4C661209 for ; Thu, 16 Sep 2021 08:43:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9B4C661209 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:43412 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQmzg-0007Nq-Oo for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:43:44 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48194) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxV-0004mm-Ug for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:30 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:37199) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxU-0004Mt-6a for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:29 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781687; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=b8JCe2/MxdzLsWsSz1R0KzEI7jC387lZ5OvEsfnLoEs=; b=L+EPephdEbQpSLjPbU5zjoHhuMcRELB8AqOnkph1HRfo7c6l+GKArJbU8Pj+Ntr1PhGTBw rPPdmX1Jk5O4NEFSfWVdYdlVkn4R7+YthFzh1Bz8uXrSEh5xd9yy2TGSBCoLGUD/2V12Pv 2+XKZ8SxfkmFvEf3Rjy2/MYzBmBnqrU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-427-Erqfs7dPOSW7LPrOC0ZgmA-1; Thu, 16 Sep 2021 04:41:23 -0400 X-MC-Unique: Erqfs7dPOSW7LPrOC0ZgmA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3A6031006AA1 for ; Thu, 16 Sep 2021 08:41:22 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AA3145B4BC; Thu, 16 Sep 2021 08:41:21 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 04/12] virtiofsd: Use lo_inode_open() instead of openat() Date: Thu, 16 Sep 2021 10:40:37 +0200 Message-Id: <20210916084045.31684-5-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" The xattr functions want a non-O_PATH FD, so they reopen the lo_inode.fd with the flags they need through /proc/self/fd. Similarly, lo_opendir() needs an O_RDONLY FD. Instead of the /proc/self/fd trick, it just uses openat(fd, "."), because the FD is guaranteed to be a directory, so this works. All cases have one problem in common, though: In the future, when we may have a file handle in the lo_inode instead of an FD, querying an lo_inode FD may incur an open_by_handle_at() call. It does not make sense to then reopen that FD with custom flags, those should have been passed to open_by_handle_at() instead. Use lo_inode_open() instead of openat(). As part of the file handle change, lo_inode_open() will be made to invoke openat() only if lo_inode.fd is valid. Otherwise, it will invoke open_by_handle_at() with the right flags from the start. Consequently, after this patch, lo_inode_open() is the only place to invoke openat() to reopen an existing FD with different flags. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 47 ++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 88318a4ba9..d76283d080 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1745,18 +1745,26 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino, { int error = ENOMEM; struct lo_data *lo = lo_data(req); - struct lo_dirp *d; + struct lo_inode *inode; + struct lo_dirp *d = NULL; int fd; ssize_t fh; + inode = lo_inode(req, ino); + if (!inode) { + error = EBADF; + goto out_err; + } + d = calloc(1, sizeof(struct lo_dirp)); if (d == NULL) { goto out_err; } - fd = openat(lo_fd(req, ino), ".", O_RDONLY); - if (fd == -1) { - goto out_errno; + fd = lo_inode_open(lo, inode, O_RDONLY); + if (fd < 0) { + error = -fd; + goto out_err; } d->dp = fdopendir(fd); @@ -1780,11 +1788,13 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino, fi->cache_readdir = 1; } fuse_reply_open(req, fi); + lo_inode_put(lo, &inode); return; out_errno: error = errno; out_err: + lo_inode_put(lo, &inode); if (d) { if (d->dp) { closedir(d->dp); @@ -2989,7 +2999,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, } } - sprintf(procname, "%i", inode->fd); /* * It is not safe to open() non-regular/non-dir files in file server * unless O_PATH is used, so use that method for regular files/dir @@ -2997,13 +3006,15 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, * Otherwise, call fchdir() to avoid open(). */ if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = openat(lo->proc_self_fd, procname, O_RDONLY); + fd = lo_inode_open(lo, inode, O_RDONLY); if (fd < 0) { - goto out_err; + saverr = -fd; + goto out; } ret = fgetxattr(fd, name, value, size); saverr = ret == -1 ? errno : 0; } else { + sprintf(procname, "%i", inode->fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = getxattr(procname, name, value, size); @@ -3070,15 +3081,16 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) } } - sprintf(procname, "%i", inode->fd); if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = openat(lo->proc_self_fd, procname, O_RDONLY); + fd = lo_inode_open(lo, inode, O_RDONLY); if (fd < 0) { - goto out_err; + saverr = -fd; + goto out; } ret = flistxattr(fd, value, size); saverr = ret == -1 ? errno : 0; } else { + sprintf(procname, "%i", inode->fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = listxattr(procname, value, size); @@ -3175,7 +3187,6 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, const char *value, size_t size, int flags, uint32_t extra_flags) { - char procname[64]; const char *name; char *mapped_name; struct lo_data *lo = lo_data(req); @@ -3221,16 +3232,15 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, fuse_log(FUSE_LOG_DEBUG, "lo_setxattr(ino=%" PRIu64 ", name=%s value=%s size=%zd)\n", ino, name, value, size); - sprintf(procname, "%i", inode->fd); /* * We can only open regular files or directories. If the inode is * something else, we have to enter /proc/self/fd and use * setxattr() on the link's filename there. */ if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = openat(lo->proc_self_fd, procname, O_RDONLY); + fd = lo_inode_open(lo, inode, O_RDONLY); if (fd < 0) { - saverr = errno; + saverr = -fd; goto out; } } else { @@ -3264,6 +3274,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, ret = fsetxattr(fd, name, value, size, flags); saverr = ret == -1 ? errno : 0; } else { + char procname[64]; + + sprintf(procname, "%i", inode->fd); ret = setxattr(procname, name, value, size, flags); saverr = ret == -1 ? errno : 0; } @@ -3333,16 +3346,16 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) fuse_log(FUSE_LOG_DEBUG, "lo_removexattr(ino=%" PRIu64 ", name=%s)\n", ino, name); - sprintf(procname, "%i", inode->fd); if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = openat(lo->proc_self_fd, procname, O_RDONLY); + fd = lo_inode_open(lo, inode, O_RDONLY); if (fd < 0) { - saverr = errno; + saverr = -fd; goto out; } ret = fremovexattr(fd, name); saverr = ret == -1 ? errno : 0; } else { + sprintf(procname, "%i", inode->fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = removexattr(procname, name); From patchwork Thu Sep 16 08:40:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498287 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 09F85C433EF for ; Thu, 16 Sep 2021 08:46:41 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9083060E90 for ; Thu, 16 Sep 2021 08:46:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9083060E90 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:52056 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn2V-0004nq-LJ for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:46:39 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48216) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxf-0004of-5s for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:39 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:59014) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxZ-0004V9-K9 for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781693; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KNa5+OFXRM0dhhLk4K19fDaQ19QxKdz1U0WGF9RJei0=; b=bLFaZlOTsRHrWW9NKv51jvDMBbHlyAR2MoDXpfTof6U5eyfn0uhPT5iQ1kcRVBxj4N5y82 7fs0xTaUkBMKTnWQM9fGj5l9Ikr5AxKD0nuExtdGtQsh9fAUsbdwqxfaFpKoAPJk3iB1xx M3tS/M6D16VxtCAb4bJJqs4v0CT/upc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-347-BYa3lhkJM3-PoO-zyb62lw-1; Thu, 16 Sep 2021 04:41:31 -0400 X-MC-Unique: BYa3lhkJM3-PoO-zyb62lw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 19FFF1084684 for ; Thu, 16 Sep 2021 08:41:31 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D608B6A908; Thu, 16 Sep 2021 08:41:23 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 05/12] virtiofsd: Add lo_inode_fd() helper Date: Thu, 16 Sep 2021 10:40:38 +0200 Message-Id: <20210916084045.31684-6-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Once we let lo_inode.fd be optional, we will need its users to open the file handle stored in lo_inode instead. This function will do that. For now, it just returns lo_inode.fd, though. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 159 +++++++++++++++++++++++++------ 1 file changed, 132 insertions(+), 27 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index d76283d080..c5baa752e4 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -651,6 +651,16 @@ static struct lo_inode *lo_inode(fuse_req_t req, fuse_ino_t ino) return elem->inode; } +static int lo_inode_fd(const struct lo_inode *inode, TempFd *tfd) +{ + *tfd = (TempFd) { + .fd = inode->fd, + .owned = false, + }; + + return 0; +} + /* * TODO Remove this helper and force callers to hold an inode refcount until * they are done with the fd. This will be done in a later patch to make @@ -838,11 +848,11 @@ static int lo_fi_fd(fuse_req_t req, struct fuse_file_info *fi) static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, int valid, struct fuse_file_info *fi) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; int saverr; char procname[64]; struct lo_data *lo = lo_data(req); struct lo_inode *inode; - int ifd; int res; int fd = -1; @@ -852,7 +862,11 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, return; } - ifd = inode->fd; + res = lo_inode_fd(inode, &path_fd); + if (res < 0) { + saverr = -res; + goto out_err; + } /* If fi->fh is invalid we'll report EBADF later */ if (fi) { @@ -863,7 +877,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, if (fi) { res = fchmod(fd, attr->st_mode); } else { - sprintf(procname, "%i", ifd); + sprintf(procname, "%i", path_fd.fd); res = fchmodat(lo->proc_self_fd, procname, attr->st_mode, 0); } if (res == -1) { @@ -875,12 +889,13 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, uid_t uid = (valid & FUSE_SET_ATTR_UID) ? attr->st_uid : (uid_t)-1; gid_t gid = (valid & FUSE_SET_ATTR_GID) ? attr->st_gid : (gid_t)-1; - saverr = drop_security_capability(lo, ifd); + saverr = drop_security_capability(lo, path_fd.fd); if (saverr) { goto out_err; } - res = fchownat(ifd, "", uid, gid, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); + res = fchownat(path_fd.fd, "", uid, gid, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); if (res == -1) { saverr = errno; goto out_err; @@ -959,7 +974,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, if (fi) { res = futimens(fd, tv); } else { - sprintf(procname, "%i", inode->fd); + sprintf(procname, "%i", path_fd.fd); res = utimensat(lo->proc_self_fd, procname, tv, 0); } if (res == -1) { @@ -1074,7 +1089,8 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, struct fuse_entry_param *e, struct lo_inode **inodep) { - int newfd; + g_auto(TempFd) dir_path_fd = TEMP_FD_INIT; + int newfd = -1; int res; int saverr; uint64_t mnt_id; @@ -1104,7 +1120,13 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, name = "."; } - newfd = openat(dir->fd, name, O_PATH | O_NOFOLLOW); + res = lo_inode_fd(dir, &dir_path_fd); + if (res < 0) { + saverr = -res; + goto out; + } + + newfd = openat(dir_path_fd.fd, name, O_PATH | O_NOFOLLOW); if (newfd == -1) { goto out_err; } @@ -1171,6 +1193,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, out_err: saverr = errno; +out: if (newfd != -1) { close(newfd); } @@ -1328,6 +1351,7 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent, const char *name, mode_t mode, dev_t rdev, const char *link) { + g_auto(TempFd) dir_path_fd = TEMP_FD_INIT; int res; int saverr; struct lo_data *lo = lo_data(req); @@ -1351,12 +1375,18 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent, return; } + res = lo_inode_fd(dir, &dir_path_fd); + if (res < 0) { + saverr = -res; + goto out; + } + saverr = lo_change_cred(req, &old, lo->change_umask && !S_ISLNK(mode)); if (saverr) { goto out; } - res = mknod_wrapper(dir->fd, name, link, mode, rdev); + res = mknod_wrapper(dir_path_fd.fd, name, link, mode, rdev); saverr = errno; @@ -1404,6 +1434,8 @@ static void lo_symlink(fuse_req_t req, const char *link, fuse_ino_t parent, static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, const char *name) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; + g_auto(TempFd) parent_path_fd = TEMP_FD_INIT; int res; struct lo_data *lo = lo_data(req); struct lo_inode *parent_inode; @@ -1425,22 +1457,35 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, parent_inode = lo_inode(req, parent); inode = lo_inode(req, ino); if (!parent_inode || !inode) { - errno = EBADF; - goto out_err; + saverr = EBADF; + goto out; + } + + res = lo_inode_fd(inode, &path_fd); + if (res < 0) { + saverr = -res; + goto out; + } + + res = lo_inode_fd(parent_inode, &parent_path_fd); + if (res < 0) { + saverr = -res; + goto out; } memset(&e, 0, sizeof(struct fuse_entry_param)); e.attr_timeout = lo->timeout; e.entry_timeout = lo->timeout; - sprintf(procname, "%i", inode->fd); - res = linkat(lo->proc_self_fd, procname, parent_inode->fd, name, + sprintf(procname, "%i", path_fd.fd); + res = linkat(lo->proc_self_fd, procname, parent_path_fd.fd, name, AT_SYMLINK_FOLLOW); if (res == -1) { goto out_err; } - res = fstatat(inode->fd, "", &e.attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); + res = fstatat(path_fd.fd, "", &e.attr, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); if (res == -1) { goto out_err; } @@ -1460,6 +1505,7 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, out_err: saverr = errno; +out: lo_inode_put(lo, &parent_inode); lo_inode_put(lo, &inode); fuse_reply_err(req, saverr); @@ -1469,23 +1515,34 @@ out_err: static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, const char *name) { + g_auto(TempFd) dir_path_fd = TEMP_FD_INIT; int res; uint64_t mnt_id; struct stat attr; struct lo_data *lo = lo_data(req); struct lo_inode *dir = lo_inode(req, parent); + struct lo_inode *inode = NULL; if (!dir) { - return NULL; + goto out; } - res = do_statx(lo, dir->fd, name, &attr, AT_SYMLINK_NOFOLLOW, &mnt_id); - lo_inode_put(lo, &dir); + res = lo_inode_fd(dir, &dir_path_fd); + if (res < 0) { + goto out; + } + + res = do_statx(lo, dir_path_fd.fd, name, &attr, AT_SYMLINK_NOFOLLOW, + &mnt_id); if (res == -1) { - return NULL; + goto out; } - return lo_find(lo, &attr, mnt_id); + inode = lo_find(lo, &attr, mnt_id); + +out: + lo_inode_put(lo, &dir); + return inode; } static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name) @@ -1521,6 +1578,8 @@ static void lo_rename(fuse_req_t req, fuse_ino_t parent, const char *name, fuse_ino_t newparent, const char *newname, unsigned int flags) { + g_auto(TempFd) parent_path_fd = TEMP_FD_INIT; + g_auto(TempFd) newparent_path_fd = TEMP_FD_INIT; int res; struct lo_inode *parent_inode; struct lo_inode *newparent_inode; @@ -1553,12 +1612,24 @@ static void lo_rename(fuse_req_t req, fuse_ino_t parent, const char *name, goto out; } + res = lo_inode_fd(parent_inode, &parent_path_fd); + if (res < 0) { + fuse_reply_err(req, -res); + goto out; + } + + res = lo_inode_fd(newparent_inode, &newparent_path_fd); + if (res < 0) { + fuse_reply_err(req, -res); + goto out; + } + if (flags) { #ifndef SYS_renameat2 fuse_reply_err(req, EINVAL); #else - res = syscall(SYS_renameat2, parent_inode->fd, name, - newparent_inode->fd, newname, flags); + res = syscall(SYS_renameat2, parent_path_fd.fd, name, + newparent_path_fd.fd, newname, flags); if (res == -1 && errno == ENOSYS) { fuse_reply_err(req, EINVAL); } else { @@ -1568,7 +1639,7 @@ static void lo_rename(fuse_req_t req, fuse_ino_t parent, const char *name, goto out; } - res = renameat(parent_inode->fd, name, newparent_inode->fd, newname); + res = renameat(parent_path_fd.fd, name, newparent_path_fd.fd, newname); fuse_reply_err(req, res == -1 ? errno : 0); out: @@ -2054,6 +2125,7 @@ static int lo_do_open(struct lo_data *lo, struct lo_inode *inode, static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name, mode_t mode, struct fuse_file_info *fi) { + g_auto(TempFd) parent_path_fd = TEMP_FD_INIT; int fd = -1; struct lo_data *lo = lo_data(req); struct lo_inode *parent_inode; @@ -2076,6 +2148,12 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name, return; } + err = lo_inode_fd(parent_inode, &parent_path_fd); + if (err < 0) { + err = -err; + goto out; + } + err = lo_change_cred(req, &old, lo->change_umask); if (err) { goto out; @@ -2084,7 +2162,7 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name, update_open_flags(lo->writeback, lo->allow_direct_io, fi); /* Try to create a new file but don't open existing files */ - fd = openat(parent_inode->fd, name, fi->flags | O_CREAT | O_EXCL, mode); + fd = openat(parent_path_fd.fd, name, fi->flags | O_CREAT | O_EXCL, mode); err = fd == -1 ? errno : 0; lo_restore_cred(&old, lo->change_umask); @@ -3014,7 +3092,14 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, ret = fgetxattr(fd, name, value, size); saverr = ret == -1 ? errno : 0; } else { - sprintf(procname, "%i", inode->fd); + g_auto(TempFd) path_fd = TEMP_FD_INIT; + + ret = lo_inode_fd(inode, &path_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", path_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = getxattr(procname, name, value, size); @@ -3090,7 +3175,14 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) ret = flistxattr(fd, value, size); saverr = ret == -1 ? errno : 0; } else { - sprintf(procname, "%i", inode->fd); + g_auto(TempFd) path_fd = TEMP_FD_INIT; + + ret = lo_inode_fd(inode, &path_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", path_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = listxattr(procname, value, size); @@ -3187,6 +3279,7 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, const char *value, size_t size, int flags, uint32_t extra_flags) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; const char *name; char *mapped_name; struct lo_data *lo = lo_data(req); @@ -3244,6 +3337,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, goto out; } } else { + ret = lo_inode_fd(inode, &path_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); /* Set flag so the clean-up path will chdir back */ @@ -3276,7 +3374,7 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, } else { char procname[64]; - sprintf(procname, "%i", inode->fd); + sprintf(procname, "%i", path_fd.fd); ret = setxattr(procname, name, value, size, flags); saverr = ret == -1 ? errno : 0; } @@ -3355,7 +3453,14 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) ret = fremovexattr(fd, name); saverr = ret == -1 ? errno : 0; } else { - sprintf(procname, "%i", inode->fd); + g_auto(TempFd) path_fd = TEMP_FD_INIT; + + ret = lo_inode_fd(inode, &path_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", path_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = removexattr(procname, name); From patchwork Thu Sep 16 08:40:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498307 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF0AFC433EF for ; Thu, 16 Sep 2021 08:50:07 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 548CC61178 for ; Thu, 16 Sep 2021 08:50:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 548CC61178 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:60082 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn5q-0001nh-41 for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:50:06 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48264) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxs-0004ve-1d for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:52 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:43791) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxq-0004e0-7r for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781705; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GQq7PVfPt12HO8B/MZpLQYAZnvGez48Zdk7HM3HgHyQ=; b=QBVugky3j1ipUaocAhPjhkzcxOiMC4Fkvs6q1Xnz8w/+Yjkda5gMMMB5s0tlsfi5W773KK JcgsWrpZRJ7KjtwxjGpGivHwGdg5IDvmtrHkqrHofPOiyK4w7ucWHQxkul7m2n8/+3NIcF cJUbjB7pIQuLL+ZyjRK0YUEDxyd7/Jw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-146-5930drqdNxqDgstSVRwiRw-1; Thu, 16 Sep 2021 04:41:42 -0400 X-MC-Unique: 5930drqdNxqDgstSVRwiRw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A7BF4108468C for ; Thu, 16 Sep 2021 08:41:41 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BC9296D981; Thu, 16 Sep 2021 08:41:32 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 06/12] virtiofsd: Let lo_fd() return a TempFd Date: Thu, 16 Sep 2021 10:40:39 +0200 Message-Id: <20210916084045.31684-7-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Accessing lo_inode.fd must generally happen through lo_inode_fd(), and lo_fd() is no exception; and then it must pass on the TempFd it has received from lo_inode_fd(). (Note that all lo_fd() calls now use proper error handling, where all of them were in-line before; i.e. they were used in place of the fd argument of some function call. This only worked because the only error that could occur was that lo_inode() failed to find the inode ID: Then -1 would be passed as the fd, which would result in an EBADF error, which is precisely what we would want to return to the guest for an invalid inode ID. Now, though, lo_inode_fd() might potentially invoke open_by_handle_at(), which can return many different errors, and they should be properly handled and returned to the guest. So we can no longer allow lo_fd() to be used in-line, and instead need to do proper error handling for it.) Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 55 +++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index c5baa752e4..3bf20b8659 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -666,18 +666,19 @@ static int lo_inode_fd(const struct lo_inode *inode, TempFd *tfd) * they are done with the fd. This will be done in a later patch to make * review easier. */ -static int lo_fd(fuse_req_t req, fuse_ino_t ino) +static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd *tfd) { struct lo_inode *inode = lo_inode(req, ino); - int fd; + int res; if (!inode) { - return -1; + return -EBADF; } - fd = inode->fd; + res = lo_inode_fd(inode, tfd); + lo_inode_put(lo_data(req), &inode); - return fd; + return res; } /* @@ -814,14 +815,19 @@ static void lo_init(void *userdata, struct fuse_conn_info *conn) static void lo_getattr(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; int res; struct stat buf; struct lo_data *lo = lo_data(req); (void)fi; - res = - fstatat(lo_fd(req, ino), "", &buf, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); + res = lo_fd(req, ino, &path_fd); + if (res < 0) { + return (void)fuse_reply_err(req, -res); + } + + res = fstatat(path_fd.fd, "", &buf, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); if (res == -1) { return (void)fuse_reply_err(req, errno); } @@ -1547,6 +1553,7 @@ out: static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name) { + g_auto(TempFd) parent_path_fd = TEMP_FD_INIT; int res; struct lo_inode *inode; struct lo_data *lo = lo_data(req); @@ -1561,13 +1568,19 @@ static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name) return; } + res = lo_fd(req, parent, &parent_path_fd); + if (res < 0) { + fuse_reply_err(req, -res); + return; + } + inode = lookup_name(req, parent, name); if (!inode) { fuse_reply_err(req, EIO); return; } - res = unlinkat(lo_fd(req, parent), name, AT_REMOVEDIR); + res = unlinkat(parent_path_fd.fd, name, AT_REMOVEDIR); fuse_reply_err(req, res == -1 ? errno : 0); unref_inode_lolocked(lo, inode, 1); @@ -1653,6 +1666,7 @@ out: static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name) { + g_auto(TempFd) parent_path_fd = TEMP_FD_INIT; int res; struct lo_inode *inode; struct lo_data *lo = lo_data(req); @@ -1667,13 +1681,19 @@ static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name) return; } + res = lo_fd(req, parent, &parent_path_fd); + if (res < 0) { + fuse_reply_err(req, -res); + return; + } + inode = lookup_name(req, parent, name); if (!inode) { fuse_reply_err(req, EIO); return; } - res = unlinkat(lo_fd(req, parent), name, 0); + res = unlinkat(parent_path_fd.fd, name, 0); fuse_reply_err(req, res == -1 ? errno : 0); unref_inode_lolocked(lo, inode, 1); @@ -1753,10 +1773,16 @@ static void lo_forget_multi(fuse_req_t req, size_t count, static void lo_readlink(fuse_req_t req, fuse_ino_t ino) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; char buf[PATH_MAX + 1]; int res; - res = readlinkat(lo_fd(req, ino), "", buf, sizeof(buf)); + res = lo_fd(req, ino, &path_fd); + if (res < 0) { + return (void)fuse_reply_err(req, -res); + } + + res = readlinkat(path_fd.fd, "", buf, sizeof(buf)); if (res == -1) { return (void)fuse_reply_err(req, errno); } @@ -2554,10 +2580,17 @@ static void lo_write_buf(fuse_req_t req, fuse_ino_t ino, static void lo_statfs(fuse_req_t req, fuse_ino_t ino) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; int res; struct statvfs stbuf; - res = fstatvfs(lo_fd(req, ino), &stbuf); + res = lo_fd(req, ino, &path_fd); + if (res < 0) { + fuse_reply_err(req, -res); + return; + } + + res = fstatvfs(path_fd.fd, &stbuf); if (res == -1) { fuse_reply_err(req, errno); } else { From patchwork Thu Sep 16 08:40:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498281 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A312C433F5 for ; Thu, 16 Sep 2021 08:44:17 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0344C6120C for ; Thu, 16 Sep 2021 08:44:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0344C6120C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:44486 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn0C-00085B-6U for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:44:16 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48296) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxy-0005Cb-5A for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:58 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:51317) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmxv-0004mw-RM for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:41:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781715; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QHb/m7LNWzjjVHgHsiAfXIOJoC+uV/r0UT9ieBKPj4Y=; b=ZYFppFhnITM2TBti1KgjltOWwdBRvZYDwApwephYBHaEqTJ0ahSPiYk5FQNeUG7QzCShU5 4MyniR69He43V5H6gw0LD+ila7kWiKWquU2C+wKPpT66z8debqNNRR5oWZLb/l5JteHYO9 k4IwwNYbtOMuYsspWgQixlHIvz9e6Dk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-221-XvnP1lmYMoiiSNwPXhSa_w-1; Thu, 16 Sep 2021 04:41:51 -0400 X-MC-Unique: XvnP1lmYMoiiSNwPXhSa_w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 11027835DE1 for ; Thu, 16 Sep 2021 08:41:51 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 706085D9F4; Thu, 16 Sep 2021 08:41:43 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 07/12] virtiofsd: Let lo_inode_open() return a TempFd Date: Thu, 16 Sep 2021 10:40:40 +0200 Message-Id: <20210916084045.31684-8-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Strictly speaking, this is not necessary, because lo_inode_open() will always return a new FD owned by the caller, so TempFd.owned will always be true. The auto-cleanup is nice, though. Also, we get a more unified interface where you always get a TempFd when you need an FD for an lo_inode (regardless of whether it is an O_PATH FD or a non-O_PATH FD). Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 156 +++++++++++++++---------------- 1 file changed, 75 insertions(+), 81 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 3bf20b8659..d257eda129 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -293,10 +293,8 @@ static void temp_fd_clear(TempFd *temp_fd) /** * Return an owned fd from *temp_fd that will not be closed when * *temp_fd goes out of scope. - * - * (TODO: Remove __attribute__ once this is used.) */ -static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd) +static int temp_fd_steal(TempFd *temp_fd) { if (temp_fd->owned) { temp_fd->owned = false; @@ -309,10 +307,8 @@ static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd) /** * Create a borrowing copy of an existing TempFd. Note that *to is * only valid as long as *from is valid. - * - * (TODO: Remove __attribute__ once this is used.) */ -static __attribute__((unused)) void temp_fd_copy(const TempFd *from, TempFd *to) +static void temp_fd_copy(const TempFd *from, TempFd *to) { *to = (TempFd) { .fd = from->fd, @@ -689,9 +685,12 @@ static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd *tfd) * when a malicious client opens special files such as block device nodes. * Symlink inodes are also rejected since symlinks must already have been * traversed on the client side. + * + * The fd is returned in tfd->fd. The return value is 0 on success and -errno + * otherwise. */ static int lo_inode_open(struct lo_data *lo, struct lo_inode *inode, - int open_flags) + int open_flags, TempFd *tfd) { g_autofree char *fd_str = g_strdup_printf("%d", inode->fd); int fd; @@ -710,7 +709,13 @@ static int lo_inode_open(struct lo_data *lo, struct lo_inode *inode, if (fd < 0) { return -errno; } - return fd; + + *tfd = (TempFd) { + .fd = fd, + .owned = true, + }; + + return 0; } static void lo_init(void *userdata, struct fuse_conn_info *conn) @@ -854,7 +859,8 @@ static int lo_fi_fd(fuse_req_t req, struct fuse_file_info *fi) static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, int valid, struct fuse_file_info *fi) { - g_auto(TempFd) path_fd = TEMP_FD_INIT; + g_auto(TempFd) path_fd = TEMP_FD_INIT; /* at least an O_PATH fd */ + g_auto(TempFd) rw_fd = TEMP_FD_INIT; /* O_RDWR fd */ int saverr; char procname[64]; struct lo_data *lo = lo_data(req); @@ -868,7 +874,15 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, return; } - res = lo_inode_fd(inode, &path_fd); + if (!fi && (valid & FUSE_SET_ATTR_SIZE)) { + /* We need an O_RDWR FD for ftruncate() */ + res = lo_inode_open(lo, inode, O_RDWR, &rw_fd); + if (res >= 0) { + temp_fd_copy(&rw_fd, &path_fd); + } + } else { + res = lo_inode_fd(inode, &path_fd); + } if (res < 0) { saverr = -res; goto out_err; @@ -916,18 +930,12 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, if (fi) { truncfd = fd; } else { - truncfd = lo_inode_open(lo, inode, O_RDWR); - if (truncfd < 0) { - saverr = -truncfd; - goto out_err; - } + assert(rw_fd.fd >= 0); + truncfd = rw_fd.fd; } saverr = drop_security_capability(lo, truncfd); if (saverr) { - if (!fi) { - close(truncfd); - } goto out_err; } @@ -935,9 +943,6 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, res = drop_effective_cap("FSETID", &cap_fsetid_dropped); if (res != 0) { saverr = res; - if (!fi) { - close(truncfd); - } goto out_err; } } @@ -950,9 +955,6 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, fuse_log(FUSE_LOG_ERR, "Failed to gain CAP_FSETID\n"); } } - if (!fi) { - close(truncfd); - } if (res == -1) { goto out_err; } @@ -1840,11 +1842,13 @@ static struct lo_dirp *lo_dirp(fuse_req_t req, struct fuse_file_info *fi) static void lo_opendir(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) { + g_auto(TempFd) rd_fd = TEMP_FD_INIT; int error = ENOMEM; struct lo_data *lo = lo_data(req); struct lo_inode *inode; struct lo_dirp *d = NULL; int fd; + int res; ssize_t fh; inode = lo_inode(req, ino); @@ -1858,14 +1862,16 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino, goto out_err; } - fd = lo_inode_open(lo, inode, O_RDONLY); - if (fd < 0) { - error = -fd; + res = lo_inode_open(lo, inode, O_RDONLY, &rd_fd); + if (res < 0) { + error = -res; goto out_err; } + fd = temp_fd_steal(&rd_fd); d->dp = fdopendir(fd); if (d->dp == NULL) { + close(fd); goto out_errno; } @@ -1895,8 +1901,6 @@ out_err: if (d) { if (d->dp) { closedir(d->dp); - } else if (fd != -1) { - close(fd); } free(d); } @@ -2096,6 +2100,7 @@ static void update_open_flags(int writeback, int allow_direct_io, static int lo_do_open(struct lo_data *lo, struct lo_inode *inode, int existing_fd, struct fuse_file_info *fi) { + g_auto(TempFd) opened_fd = TEMP_FD_INIT; ssize_t fh; int fd = existing_fd; int err; @@ -2112,16 +2117,18 @@ static int lo_do_open(struct lo_data *lo, struct lo_inode *inode, } } - fd = lo_inode_open(lo, inode, fi->flags); + err = lo_inode_open(lo, inode, fi->flags, &opened_fd); if (cap_fsetid_dropped) { if (gain_effective_cap("FSETID")) { fuse_log(FUSE_LOG_ERR, "Failed to gain CAP_FSETID\n"); } } - if (fd < 0) { - return -fd; + if (err < 0) { + return -err; } + fd = temp_fd_steal(&opened_fd); + if (fi->flags & (O_TRUNC)) { int err = drop_security_capability(lo, fd); if (err) { @@ -2231,8 +2238,9 @@ static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo, uint64_t lock_owner, pid_t pid, int *err) { + g_auto(TempFd) rw_fd = TEMP_FD_INIT; struct lo_inode_plock *plock; - int fd; + int res; plock = g_hash_table_lookup(inode->posix_locks, GUINT_TO_POINTER(lock_owner)); @@ -2249,15 +2257,15 @@ static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo, /* Open another instance of file which can be used for ofd locks. */ /* TODO: What if file is not writable? */ - fd = lo_inode_open(lo, inode, O_RDWR); - if (fd < 0) { - *err = -fd; + res = lo_inode_open(lo, inode, O_RDWR, &rw_fd); + if (res < 0) { + *err = -res; free(plock); return NULL; } plock->lock_owner = lock_owner; - plock->fd = fd; + plock->fd = temp_fd_steal(&rw_fd); g_hash_table_insert(inode->posix_locks, GUINT_TO_POINTER(plock->lock_owner), plock); return plock; @@ -2473,6 +2481,7 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, struct fuse_file_info *fi) { + g_auto(TempFd) rw_fd = TEMP_FD_INIT; struct lo_inode *inode = lo_inode(req, ino); struct lo_data *lo = lo_data(req); int res; @@ -2487,11 +2496,12 @@ static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, } if (!fi) { - fd = lo_inode_open(lo, inode, O_RDWR); - if (fd < 0) { - res = -fd; + res = lo_inode_open(lo, inode, O_RDWR, &rw_fd); + if (res < 0) { + res = -res; goto out; } + fd = rw_fd.fd; } else { fd = lo_fi_fd(req, fi); } @@ -2501,9 +2511,6 @@ static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, } else { res = fsync(fd) == -1 ? errno : 0; } - if (!fi) { - close(fd); - } out: lo_inode_put(lo, &inode); fuse_reply_err(req, res); @@ -3065,7 +3072,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, struct lo_inode *inode; ssize_t ret; int saverr; - int fd = -1; if (block_xattr(lo, in_name)) { fuse_reply_err(req, EOPNOTSUPP); @@ -3117,12 +3123,14 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, * Otherwise, call fchdir() to avoid open(). */ if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = lo_inode_open(lo, inode, O_RDONLY); - if (fd < 0) { - saverr = -fd; + g_auto(TempFd) rd_fd = TEMP_FD_INIT; + + ret = lo_inode_open(lo, inode, O_RDONLY, &rd_fd); + if (ret < 0) { + saverr = -ret; goto out; } - ret = fgetxattr(fd, name, value, size); + ret = fgetxattr(rd_fd.fd, name, value, size); saverr = ret == -1 ? errno : 0; } else { g_auto(TempFd) path_fd = TEMP_FD_INIT; @@ -3153,10 +3161,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, fuse_reply_xattr(req, ret); } out_free: - if (fd >= 0) { - close(fd); - } - lo_inode_put(lo, &inode); return; @@ -3176,7 +3180,6 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) struct lo_inode *inode; ssize_t ret; int saverr; - int fd = -1; inode = lo_inode(req, ino); if (!inode) { @@ -3200,12 +3203,14 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) } if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = lo_inode_open(lo, inode, O_RDONLY); - if (fd < 0) { - saverr = -fd; + g_auto(TempFd) rd_fd = TEMP_FD_INIT; + + ret = lo_inode_open(lo, inode, O_RDONLY, &rd_fd); + if (ret < 0) { + saverr = -ret; goto out; } - ret = flistxattr(fd, value, size); + ret = flistxattr(rd_fd.fd, value, size); saverr = ret == -1 ? errno : 0; } else { g_auto(TempFd) path_fd = TEMP_FD_INIT; @@ -3294,10 +3299,6 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) fuse_reply_xattr(req, ret); } out_free: - if (fd >= 0) { - close(fd); - } - lo_inode_put(lo, &inode); return; @@ -3312,14 +3313,14 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, const char *value, size_t size, int flags, uint32_t extra_flags) { - g_auto(TempFd) path_fd = TEMP_FD_INIT; + g_auto(TempFd) path_fd = TEMP_FD_INIT; /* O_PATH fd */ + g_auto(TempFd) rd_fd = TEMP_FD_INIT; /* O_RDONLY fd */ const char *name; char *mapped_name; struct lo_data *lo = lo_data(req); struct lo_inode *inode; ssize_t ret; int saverr; - int fd = -1; bool switched_creds = false; bool cap_fsetid_dropped = false; struct lo_cred old = {}; @@ -3364,9 +3365,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, * setxattr() on the link's filename there. */ if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = lo_inode_open(lo, inode, O_RDONLY); - if (fd < 0) { - saverr = -fd; + ret = lo_inode_open(lo, inode, O_RDONLY, &rd_fd); + if (ret < 0) { + saverr = -ret; goto out; } } else { @@ -3401,8 +3402,8 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, } switched_creds = true; } - if (fd >= 0) { - ret = fsetxattr(fd, name, value, size, flags); + if (rd_fd.fd >= 0) { + ret = fsetxattr(rd_fd.fd, name, value, size, flags); saverr = ret == -1 ? errno : 0; } else { char procname[64]; @@ -3424,10 +3425,6 @@ out: FCHDIR_NOFAIL(lo->root.fd); } - if (fd >= 0) { - close(fd); - } - lo_inode_put(lo, &inode); g_free(mapped_name); fuse_reply_err(req, saverr); @@ -3442,7 +3439,6 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) struct lo_inode *inode; ssize_t ret; int saverr; - int fd = -1; if (block_xattr(lo, in_name)) { fuse_reply_err(req, EOPNOTSUPP); @@ -3478,12 +3474,14 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) name); if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) { - fd = lo_inode_open(lo, inode, O_RDONLY); - if (fd < 0) { - saverr = -fd; + g_auto(TempFd) rd_fd = TEMP_FD_INIT; + + ret = lo_inode_open(lo, inode, O_RDONLY, &rd_fd); + if (ret < 0) { + saverr = -ret; goto out; } - ret = fremovexattr(fd, name); + ret = fremovexattr(rd_fd.fd, name); saverr = ret == -1 ? errno : 0; } else { g_auto(TempFd) path_fd = TEMP_FD_INIT; @@ -3502,10 +3500,6 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) } out: - if (fd >= 0) { - close(fd); - } - lo_inode_put(lo, &inode); g_free(mapped_name); fuse_reply_err(req, saverr); From patchwork Thu Sep 16 08:40:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498311 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B17C1C433EF for ; Thu, 16 Sep 2021 08:51:54 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 40A2460F25 for ; Thu, 16 Sep 2021 08:51:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 40A2460F25 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:34444 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn7Z-0003YB-Fq for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:51:53 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48360) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmy6-0005Vn-3M for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:06 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:23962) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmy4-0004vK-7F for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781723; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QK4Ko1a0HV1/11qX46Shrs3wSZtvo4iD4h0aQ3Iaf/s=; b=ImQPIazp6C0hn4ZCOr2vHKyr3BbwbN6UysB92LWauTsX0XXo1+YpVRlHcAV06KpEDXLjn6 p8z1R7lpH7HS7WjuMepWN0tMOJierwOKMv4w+rVvTeZ0mUeCbr5YjeDStbTd7hwCc13oZM rLPPBEecFSYABCoNmgs5lvPmeJsChfU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-463--YpCZLupM4-abStfmzQusg-1; Thu, 16 Sep 2021 04:42:02 -0400 X-MC-Unique: -YpCZLupM4-abStfmzQusg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2276E835DE0 for ; Thu, 16 Sep 2021 08:42:01 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CD4CA10027C4; Thu, 16 Sep 2021 08:41:52 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 08/12] virtiofsd: Pass lo_data to lo_inode_{fd,open}() Date: Thu, 16 Sep 2021 10:40:41 +0200 Message-Id: <20210916084045.31684-9-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" In order to be able to use file handles for identifying lo_inode objects, we will add some global state to lo_data, which we will need in a future function to be called from lo_inode_fd() and lo_inode_open(). To prepare for this, pass a (non-const) lo_data pointer to lo_inode_fd() and lo_inode_open(). Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 34 +++++++++++++++++--------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index d257eda129..bc3b803d46 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -647,7 +647,8 @@ static struct lo_inode *lo_inode(fuse_req_t req, fuse_ino_t ino) return elem->inode; } -static int lo_inode_fd(const struct lo_inode *inode, TempFd *tfd) +static int lo_inode_fd(struct lo_data *lo, const struct lo_inode *inode, + TempFd *tfd) { *tfd = (TempFd) { .fd = inode->fd, @@ -665,15 +666,16 @@ static int lo_inode_fd(const struct lo_inode *inode, TempFd *tfd) static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd *tfd) { struct lo_inode *inode = lo_inode(req, ino); + struct lo_data *lo = lo_data(req); int res; if (!inode) { return -EBADF; } - res = lo_inode_fd(inode, tfd); + res = lo_inode_fd(lo, inode, tfd); - lo_inode_put(lo_data(req), &inode); + lo_inode_put(lo, &inode); return res; } @@ -881,7 +883,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, temp_fd_copy(&rw_fd, &path_fd); } } else { - res = lo_inode_fd(inode, &path_fd); + res = lo_inode_fd(lo, inode, &path_fd); } if (res < 0) { saverr = -res; @@ -1128,7 +1130,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, name = "."; } - res = lo_inode_fd(dir, &dir_path_fd); + res = lo_inode_fd(lo, dir, &dir_path_fd); if (res < 0) { saverr = -res; goto out; @@ -1383,7 +1385,7 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent, return; } - res = lo_inode_fd(dir, &dir_path_fd); + res = lo_inode_fd(lo, dir, &dir_path_fd); if (res < 0) { saverr = -res; goto out; @@ -1469,13 +1471,13 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, goto out; } - res = lo_inode_fd(inode, &path_fd); + res = lo_inode_fd(lo, inode, &path_fd); if (res < 0) { saverr = -res; goto out; } - res = lo_inode_fd(parent_inode, &parent_path_fd); + res = lo_inode_fd(lo, parent_inode, &parent_path_fd); if (res < 0) { saverr = -res; goto out; @@ -1535,7 +1537,7 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, goto out; } - res = lo_inode_fd(dir, &dir_path_fd); + res = lo_inode_fd(lo, dir, &dir_path_fd); if (res < 0) { goto out; } @@ -1627,13 +1629,13 @@ static void lo_rename(fuse_req_t req, fuse_ino_t parent, const char *name, goto out; } - res = lo_inode_fd(parent_inode, &parent_path_fd); + res = lo_inode_fd(lo, parent_inode, &parent_path_fd); if (res < 0) { fuse_reply_err(req, -res); goto out; } - res = lo_inode_fd(newparent_inode, &newparent_path_fd); + res = lo_inode_fd(lo, newparent_inode, &newparent_path_fd); if (res < 0) { fuse_reply_err(req, -res); goto out; @@ -2181,7 +2183,7 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name, return; } - err = lo_inode_fd(parent_inode, &parent_path_fd); + err = lo_inode_fd(lo, parent_inode, &parent_path_fd); if (err < 0) { err = -err; goto out; @@ -3135,7 +3137,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, } else { g_auto(TempFd) path_fd = TEMP_FD_INIT; - ret = lo_inode_fd(inode, &path_fd); + ret = lo_inode_fd(lo, inode, &path_fd); if (ret < 0) { saverr = -ret; goto out; @@ -3215,7 +3217,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) } else { g_auto(TempFd) path_fd = TEMP_FD_INIT; - ret = lo_inode_fd(inode, &path_fd); + ret = lo_inode_fd(lo, inode, &path_fd); if (ret < 0) { saverr = -ret; goto out; @@ -3371,7 +3373,7 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, goto out; } } else { - ret = lo_inode_fd(inode, &path_fd); + ret = lo_inode_fd(lo, inode, &path_fd); if (ret < 0) { saverr = -ret; goto out; @@ -3486,7 +3488,7 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) } else { g_auto(TempFd) path_fd = TEMP_FD_INIT; - ret = lo_inode_fd(inode, &path_fd); + ret = lo_inode_fd(lo, inode, &path_fd); if (ret < 0) { saverr = -ret; goto out; From patchwork Thu Sep 16 08:40:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498289 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7B7EC433FE for ; Thu, 16 Sep 2021 08:47:13 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 459F1611C4 for ; Thu, 16 Sep 2021 08:47:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 459F1611C4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:53150 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn32-0005WJ-FX for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:47:12 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48422) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyG-0005uO-CF for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:16 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:45403) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyE-00056H-9C for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:16 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781732; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Bl3JgclFZHjurrEhyZznusHprYGoPh2uJ+Fx7Cp7dBI=; b=MsOgcB9QvZ8YyBdS6FWGdoZ/ZiKLarDWjW44v59EwgtqXDQMNVraZXznx8Rh5oqWNsao9i Op8DES5biVRUjgg9HHn5jM9MrvWMDJd/agsL8Zy2VzXB1kA06/cX3qzGcYmovm4vV+1aa9 0Lno/Ch7bBRYIXE65RudzUr+d475Hdc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-357-RP9iPUOKNx-0RDotcaCd8w-1; Thu, 16 Sep 2021 04:42:09 -0400 X-MC-Unique: RP9iPUOKNx-0RDotcaCd8w-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DE5A269723 for ; Thu, 16 Sep 2021 08:42:08 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D2EB35F707; Thu, 16 Sep 2021 08:42:02 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 09/12] virtiofsd: Add lo_inode.fhandle Date: Thu, 16 Sep 2021 10:40:42 +0200 Message-Id: <20210916084045.31684-10-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" This new field is an alternative to lo_inode.fd: Either of the two must be set. In case an O_PATH FD is needed for some lo_inode, it is either taken from lo_inode.fd, if valid, or a temporary FD is opened with open_by_handle_at(). Using a file handle instead of an FD has the advantage of keeping the number of open file descriptors low. Because open_by_handle_at() requires a mount FD (i.e. a non-O_PATH FD opened on the filesystem to which the file handle refers), but every lo_fhandle only has a mount ID (as returned by name_to_handle_at()), we keep a hash map of such FDs in mount_fds (mapping ID to FD). get_file_handle(), which is added by a later patch, will ensure that every mount ID for which we have generated a handle has a corresponding entry in mount_fds. Every handle holds a strong reference to its mount FD (lo_mount_fd.refcount) so we can clean up mount FDs when they are no longer needed. release_file_handle()'s drop_mount_fd_ref parameter may look a bit strange at this point, because we always pass true for it, but it will make more sense when we start generating file handles: At that point, we will also use this function to clean up lo_fhandle object that do not yet have a strong reference to an lo_mount_fd object, and then we will need to be able to pass false for drop_mount_fd_ref. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 175 +++++++++++++++++++++++--- tools/virtiofsd/passthrough_seccomp.c | 1 + 2 files changed, 161 insertions(+), 15 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index bc3b803d46..bd8fc922ea 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -88,8 +88,18 @@ struct lo_key { uint64_t mnt_id; }; +struct lo_fhandle { + union { + struct file_handle handle; + char padding[sizeof(struct file_handle) + MAX_HANDLE_SZ]; + }; + int mount_id; +}; + struct lo_inode { + /* fd or fhandle must be set (i.e. >= 0 or non-NULL, respectively) */ int fd; + struct lo_fhandle *fhandle; /* * Atomic reference count for this object. The nlookup field holds a @@ -142,6 +152,19 @@ typedef struct xattr_map_entry { unsigned int flags; } XattrMapEntry; +/* + * An O_RDONLY FD representing the mount it is on. We need this for + * open_by_handle_at(). + * + * The refcount is increased every time we store a file handle for + * this mount, and it is decreased every time we release such a stored + * file handle. + */ +struct lo_mount_fd { + int fd; + gint refcount; +}; + struct lo_data { pthread_mutex_t mutex; int sandbox; @@ -178,6 +201,10 @@ struct lo_data { /* If set, virtiofsd is responsible for setting umask during creation */ bool change_umask; int user_posix_acl, posix_acl; + + /* Maps (integer) mount IDs to lo_mount_fd objects */ + GHashTable *mount_fds; + pthread_rwlock_t mount_fds_lock; }; /** @@ -316,6 +343,93 @@ static void temp_fd_copy(const TempFd *from, TempFd *to) }; } +static void free_lo_mount_fd(gpointer data) +{ + struct lo_mount_fd *mfd = data; + + close(mfd->fd); + g_free(mfd); +} + +/** + * Frees a file handle and optionally removes its reference to the + * associated mount FD. (Passing NULL as @fh is OK.) + * + * Pass @drop_mount_fd_ref == true if and only if this handle has a + * strong reference to an lo_mount_fd object in the mount_fds hash + * table. That is always the case for file handles stored in lo_inode + * objects. + */ +static void release_file_handle(struct lo_data *lo, struct lo_fhandle *fh, + bool drop_mount_fd_ref) +{ + if (!fh) { + return; + } + + if (drop_mount_fd_ref) { + struct lo_mount_fd *mfd; + + if (pthread_rwlock_rdlock(&lo->mount_fds_lock)) { + fuse_log(FUSE_LOG_ERR, "%s(): Dropping mount FD reference failed " + "(mount ID: %i)\n", __func__, fh->mount_id); + } else { + mfd = g_hash_table_lookup(lo->mount_fds, + GINT_TO_POINTER(fh->mount_id)); + assert(mfd != NULL); + + pthread_rwlock_unlock(&lo->mount_fds_lock); + + if (g_atomic_int_dec_and_test(&mfd->refcount)) { + if (pthread_rwlock_wrlock(&lo->mount_fds_lock)) { + fuse_log(FUSE_LOG_ERR, "%s(): Dropping mount FD reference " + "failed (mount ID: %i)\n", __func__, fh->mount_id); + } else { + /* Auto-closes the FD and frees mfd */ + g_hash_table_remove(lo->mount_fds, + GINT_TO_POINTER(fh->mount_id)); + pthread_rwlock_unlock(&lo->mount_fds_lock); + } + } + } + } + + g_free(fh); +} + +/** + * Open the given file handle with the given flags. + * + * The mount FD to pass to open_by_handle_at() is taken from the + * mount_fds hash map. + * + * On error, return -errno. + */ +static int open_file_handle(struct lo_data *lo, const struct lo_fhandle *fh, + int flags) +{ + struct lo_mount_fd *mfd; + int ret; + + ret = pthread_rwlock_rdlock(&lo->mount_fds_lock); + if (ret) { + return -ret; + } + + mfd = g_hash_table_lookup(lo->mount_fds, GINT_TO_POINTER(fh->mount_id)); + pthread_rwlock_unlock(&lo->mount_fds_lock); + if (!mfd) { + return -EINVAL; + } + + ret = open_by_handle_at(mfd->fd, (struct file_handle *)&fh->handle, flags); + if (ret < 0) { + return -errno; + } + + return ret; +} + /* * Load capng's state from our saved state if the current thread * hadn't previously been loaded. @@ -622,7 +736,10 @@ static void lo_inode_put(struct lo_data *lo, struct lo_inode **inodep) *inodep = NULL; if (g_atomic_int_dec_and_test(&inode->refcount)) { - close(inode->fd); + if (inode->fd >= 0) { + close(inode->fd); + } + release_file_handle(lo, inode->fhandle, true); free(inode); } } @@ -650,10 +767,25 @@ static struct lo_inode *lo_inode(fuse_req_t req, fuse_ino_t ino) static int lo_inode_fd(struct lo_data *lo, const struct lo_inode *inode, TempFd *tfd) { - *tfd = (TempFd) { - .fd = inode->fd, - .owned = false, - }; + if (inode->fd >= 0) { + *tfd = (TempFd) { + .fd = inode->fd, + .owned = false, + }; + } else { + int fd; + + assert(inode->fhandle != NULL); + fd = open_file_handle(lo, inode->fhandle, O_PATH); + if (fd < 0) { + return -errno; + } + + *tfd = (TempFd) { + .fd = fd, + .owned = true, + }; + } return 0; } @@ -694,22 +826,32 @@ static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd *tfd) static int lo_inode_open(struct lo_data *lo, struct lo_inode *inode, int open_flags, TempFd *tfd) { - g_autofree char *fd_str = g_strdup_printf("%d", inode->fd); + g_autofree char *fd_str = NULL; int fd; if (!S_ISREG(inode->filetype) && !S_ISDIR(inode->filetype)) { return -EBADF; } - /* - * The file is a symlink so O_NOFOLLOW must be ignored. We checked earlier - * that the inode is not a special file but if an external process races - * with us then symlinks are traversed here. It is not possible to escape - * the shared directory since it is mounted as "/" though. - */ - fd = openat(lo->proc_self_fd, fd_str, open_flags & ~O_NOFOLLOW); - if (fd < 0) { - return -errno; + if (inode->fd >= 0) { + /* + * The file is a symlink so O_NOFOLLOW must be ignored. We checked + * earlier that the inode is not a special file but if an external + * process races with us then symlinks are traversed here. It is not + * possible to escape the shared directory since it is mounted as "/" + * though. + */ + fd_str = g_strdup_printf("%d", inode->fd); + fd = openat(lo->proc_self_fd, fd_str, open_flags & ~O_NOFOLLOW); + if (fd < 0) { + return -errno; + } + } else { + assert(inode->fhandle != NULL); + fd = open_file_handle(lo, inode->fhandle, open_flags); + if (fd < 0) { + return fd; + } } *tfd = (TempFd) { @@ -4187,6 +4329,9 @@ int main(int argc, char *argv[]) lo.root.fuse_ino = FUSE_ROOT_ID; lo.cache = CACHE_AUTO; + pthread_rwlock_init(&lo.mount_fds_lock, NULL); + lo.mount_fds = g_hash_table_new_full(NULL, NULL, NULL, free_lo_mount_fd); + /* * Set up the ino map like this: * [0] Reserved (will not be used) diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c index f49ed94b5e..af04c638cb 100644 --- a/tools/virtiofsd/passthrough_seccomp.c +++ b/tools/virtiofsd/passthrough_seccomp.c @@ -77,6 +77,7 @@ static const int syscall_allowlist[] = { SCMP_SYS(statx), SCMP_SYS(open), SCMP_SYS(openat), + SCMP_SYS(open_by_handle_at), SCMP_SYS(ppoll), SCMP_SYS(prctl), /* TODO restrict to just PR_SET_NAME? */ SCMP_SYS(preadv), From patchwork Thu Sep 16 08:40:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498309 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7FF3C4332F for ; Thu, 16 Sep 2021 08:50:48 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3675D6120C for ; Thu, 16 Sep 2021 08:50:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3675D6120C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:60820 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn6V-0002IF-DK for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:50:47 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48434) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyN-0006Hp-24 for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:23 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:41781) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyK-0005Dj-VM for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781740; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KR5b2ReeX11MFXGPtz6M0FxMB5Scw4Jcz/8eoKEkbKA=; b=g5h4PPZZ9zOw7yQOgO77eHiJ298sOGjitUh92Y3G/BpxDf6/W4EYkKFvxnf1l7IUICd68h 97FDKAepP7552GymDVA+j0rmEyzRr/B99fgRtldhCgg2d1EZyoU8FS/iAksYrNPxHuZ6DR weSgWFIAv/HcgpSqxr+w3drQpvElhs0= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-85-4Vxk5OxdMuirUGMIIqySow-1; Thu, 16 Sep 2021 04:42:19 -0400 X-MC-Unique: 4Vxk5OxdMuirUGMIIqySow-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2E3DC802934 for ; Thu, 16 Sep 2021 08:42:18 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B51895D9CA; Thu, 16 Sep 2021 08:42:10 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 10/12] virtiofsd: Add inodes_by_handle hash table Date: Thu, 16 Sep 2021 10:40:43 +0200 Message-Id: <20210916084045.31684-11-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Currently, lo_inode.fhandle is always NULL and so always keep an O_PATH FD in lo_inode.fd. Therefore, when the respective inode is unlinked, its inode ID will remain in use until we drop our lo_inode (and lo_inode_put() thus closes the FD). Therefore, lo_find() can safely use the inode ID as an lo_inode key, because any inode with an inode ID we find in lo_data.inodes (on the same filesystem) must be the exact same file. This will change when we start setting lo_inode.fhandle so we do not have to keep an O_PATH FD open. Then, unlinking such an inode will immediately remove it, so its ID can then be reused by newly created files, even while the lo_inode object is still there[1]. So creating a new file can then reuse the old file's inode ID, and looking up the new file would lead to us finding the old file's lo_inode, which is not ideal. Luckily, just as file handles cause this problem, they also solve it: A file handle contains a generation ID, which changes when an inode ID is reused, so the new file can be distinguished from the old one. So all we need to do is to add a second map besides lo_data.inodes that maps file handles to lo_inodes, namely lo_data.inodes_by_handle. For clarity, lo_data.inodes is renamed to lo_data.inodes_by_ids. Unfortunately, we cannot rely on being able to generate file handles every time. Therefore, we still enter every lo_inode object into inodes_by_ids, but having an entry in inodes_by_handle is optional. A potential inodes_by_handle entry then has precedence, the inodes_by_ids entry is just a fallback. Note that we do not generate lo_fhandle objects yet, and so we also do not enter anything into the inodes_by_handle map yet. Also, all lookups skip that map. We might manually create file handles with some code that is immediately removed by the next patch again, but that would break the assumption in lo_find() that every lo_inode with a non-NULL .fhandle must have an entry in inodes_by_handle and vice versa. So we leave actually using the inodes_by_handle map for the next patch. [1] If some application in the guest still has the file open, there is going to be a corresponding FD mapping in lo_data.fd_map. In such a case, the inode will only go away once every application in the guest has closed it. The problem described only applies to cases where the guest does not have the file open, and it is just in the dentry cache, basically. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 81 +++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 16 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index bd8fc922ea..b7d6aa7f9d 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -186,7 +186,8 @@ struct lo_data { int announce_submounts; bool use_statx; struct lo_inode root; - GHashTable *inodes; /* protected by lo->mutex */ + GHashTable *inodes_by_ids; /* protected by lo->mutex */ + GHashTable *inodes_by_handle; /* protected by lo->mutex */ struct lo_map ino_map; /* protected by lo->mutex */ struct lo_map dirp_map; /* protected by lo->mutex */ struct lo_map fd_map; /* protected by lo->mutex */ @@ -275,8 +276,9 @@ static struct { /* That we loaded cap-ng in the current thread from the saved */ static __thread bool cap_loaded = 0; -static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st, - uint64_t mnt_id); +static struct lo_inode *lo_find(struct lo_data *lo, + const struct lo_fhandle *fhandle, + struct stat *st, uint64_t mnt_id); static int xattr_map_client(const struct lo_data *lo, const char *client_name, char **out_name); @@ -1143,18 +1145,40 @@ out_err: fuse_reply_err(req, saverr); } -static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st, - uint64_t mnt_id) +static struct lo_inode *lo_find(struct lo_data *lo, + const struct lo_fhandle *fhandle, + struct stat *st, uint64_t mnt_id) { - struct lo_inode *p; - struct lo_key key = { + struct lo_inode *p = NULL; + struct lo_key ids_key = { .ino = st->st_ino, .dev = st->st_dev, .mnt_id = mnt_id, }; pthread_mutex_lock(&lo->mutex); - p = g_hash_table_lookup(lo->inodes, &key); + if (fhandle) { + p = g_hash_table_lookup(lo->inodes_by_handle, fhandle); + } + if (!p) { + p = g_hash_table_lookup(lo->inodes_by_ids, &ids_key); + /* + * When we had to fall back to looking up an inode by its + * inode ID, ensure that we hit an entry that has a valid file + * descriptor. Having an FD open means that the inode cannot + * really be deleted until the FD is closed, so that the inode + * ID remains valid until we evict our lo_inode. + * With no FD open (and just a file handle), the inode can be + * deleted while we still have our lo_inode, and so the inode + * ID may be reused by a completely different new inode. We + * then must look up the lo_inode by file handle, because this + * handle contains a generation ID to differentiate between + * the old and the new inode. + */ + if (p && p->fd == -1) { + p = NULL; + } + } if (p) { assert(p->nlookup > 0); p->nlookup++; @@ -1294,7 +1318,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, e->attr_flags |= FUSE_ATTR_SUBMOUNT; } - inode = lo_find(lo, &e->attr, mnt_id); + inode = lo_find(lo, NULL, &e->attr, mnt_id); if (inode) { close(newfd); } else { @@ -1324,7 +1348,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, } pthread_mutex_lock(&lo->mutex); inode->fuse_ino = lo_add_inode_mapping(req, inode); - g_hash_table_insert(lo->inodes, &inode->key, inode); + g_hash_table_insert(lo->inodes_by_ids, &inode->key, inode); pthread_mutex_unlock(&lo->mutex); } e->ino = inode->fuse_ino; @@ -1690,7 +1714,7 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, goto out; } - inode = lo_find(lo, &attr, mnt_id); + inode = lo_find(lo, NULL, &attr, mnt_id); out: lo_inode_put(lo, &dir); @@ -1857,7 +1881,7 @@ static void unref_inode(struct lo_data *lo, struct lo_inode *inode, uint64_t n) inode->nlookup -= n; if (!inode->nlookup) { lo_map_remove(&lo->ino_map, inode->fuse_ino); - g_hash_table_remove(lo->inodes, &inode->key); + g_hash_table_remove(lo->inodes_by_ids, &inode->key); if (lo->posix_lock) { if (g_hash_table_size(inode->posix_locks)) { fuse_log(FUSE_LOG_WARNING, "Hash table is not empty\n"); @@ -3700,7 +3724,7 @@ static void lo_destroy(void *userdata) GHashTableIter iter; gpointer key, value; - g_hash_table_iter_init(&iter, lo->inodes); + g_hash_table_iter_init(&iter, lo->inodes_by_ids); if (!g_hash_table_iter_next(&iter, &key, &value)) { break; } @@ -4264,10 +4288,34 @@ static gboolean lo_key_equal(gconstpointer a, gconstpointer b) return la->ino == lb->ino && la->dev == lb->dev && la->mnt_id == lb->mnt_id; } +static guint lo_fhandle_hash(gconstpointer key) +{ + const struct lo_fhandle *fh = key; + guint hash; + size_t i; + + /* Basically g_str_hash() */ + hash = 5381; + for (i = 0; i < sizeof(fh->padding); i++) { + hash += hash * 33 + (unsigned char)fh->padding[i]; + } + hash += hash * 33 + fh->mount_id; + + return hash; +} + +static gboolean lo_fhandle_equal(gconstpointer a, gconstpointer b) +{ + return !memcmp(a, b, sizeof(struct lo_fhandle)); +} + static void fuse_lo_data_cleanup(struct lo_data *lo) { - if (lo->inodes) { - g_hash_table_destroy(lo->inodes); + if (lo->inodes_by_ids) { + g_hash_table_destroy(lo->inodes_by_ids); + } + if (lo->inodes_by_handle) { + g_hash_table_destroy(lo->inodes_by_handle); } if (lo->root.posix_locks) { @@ -4324,7 +4372,8 @@ int main(int argc, char *argv[]) qemu_init_exec_dir(argv[0]); pthread_mutex_init(&lo.mutex, NULL); - lo.inodes = g_hash_table_new(lo_key_hash, lo_key_equal); + lo.inodes_by_ids = g_hash_table_new(lo_key_hash, lo_key_equal); + lo.inodes_by_handle = g_hash_table_new(lo_fhandle_hash, lo_fhandle_equal); lo.root.fd = -1; lo.root.fuse_ino = FUSE_ROOT_ID; lo.cache = CACHE_AUTO; From patchwork Thu Sep 16 08:40:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498343 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 23CD6C433F5 for ; Thu, 16 Sep 2021 08:54:05 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AC4FE60F4C for ; Thu, 16 Sep 2021 08:54:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AC4FE60F4C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:40756 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn9f-0007t4-Sw for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:54:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48458) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyb-0006hU-Bl for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:39 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:47870) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyY-0005Tt-Lp for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781754; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hDvhcmVyAqA1oQZ0PugnVezcuViXdQ1YqjgnvniwOXA=; b=D29qtmqzGtV3j1F2X+D0/Rr7wz5qEpvzm2n2HtzpE5M3UkVXTK4AD5LX6FQZvlO4LKhVEu 7WHMqPT1FugFyOwln/7EoBSF3/hxeCtersa3wx0elsd25exAGjwJKucqumRfWHIJic+ld0 mxraMhkMUbt89wXGONaODpFUVbvGBUs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-586-koSKlKh_Py273u1qxvDw4g-1; Thu, 16 Sep 2021 04:42:30 -0400 X-MC-Unique: koSKlKh_Py273u1qxvDw4g-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7384679EDD for ; Thu, 16 Sep 2021 08:42:29 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0978A10027C4; Thu, 16 Sep 2021 08:42:19 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 11/12] virtiofsd: Optionally fill lo_inode.fhandle Date: Thu, 16 Sep 2021 10:40:44 +0200 Message-Id: <20210916084045.31684-12-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" When the inode_file_handles option is set, try to generate a file handle for new inodes instead of opening an O_PATH FD. Being able to open these again will require CAP_DAC_READ_SEARCH, so setting this option will result in us taking that capability. Generating a file handle returns the mount ID it is valid for. Opening it will require an FD instead. We have mount_fds to map an ID to an FD. get_file_handle() scans /proc/self/mountinfo to map mount IDs to their mount points, which we open to get the mount FD we need. To verify that the resulting FD indeed represents the handle's mount ID, we use statx(). Therefore, using file handles requires statx() support. Signed-off-by: Hanna Reitz --- tools/virtiofsd/helper.c | 3 + tools/virtiofsd/passthrough_ll.c | 297 ++++++++++++++++++++++++-- tools/virtiofsd/passthrough_seccomp.c | 1 + 3 files changed, 289 insertions(+), 12 deletions(-) diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c index a8295d975a..311f05c7ee 100644 --- a/tools/virtiofsd/helper.c +++ b/tools/virtiofsd/helper.c @@ -187,6 +187,9 @@ void fuse_cmdline_help(void) " default: no_allow_direct_io\n" " -o announce_submounts Announce sub-mount points to the guest\n" " -o posix_acl/no_posix_acl Enable/Disable posix_acl. (default: disabled)\n" + " -o inode_file_handles Use file handles to reference inodes\n" + " instead of O_PATH file descriptors\n" + " (adds +dac_read_search to modcaps)\n" ); } diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index b7d6aa7f9d..e86fad8b2f 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -206,6 +206,8 @@ struct lo_data { /* Maps (integer) mount IDs to lo_mount_fd objects */ GHashTable *mount_fds; pthread_rwlock_t mount_fds_lock; + + int inode_file_handles; }; /** @@ -262,6 +264,10 @@ static const struct fuse_opt lo_opts[] = { { "no_killpriv_v2", offsetof(struct lo_data, user_killpriv_v2), 0 }, { "posix_acl", offsetof(struct lo_data, user_posix_acl), 1 }, { "no_posix_acl", offsetof(struct lo_data, user_posix_acl), 0 }, + { "inode_file_handles", offsetof(struct lo_data, inode_file_handles), 1 }, + { "no_inode_file_handles", + offsetof(struct lo_data, inode_file_handles), + 0 }, FUSE_OPT_END }; static bool use_syslog = false; @@ -359,8 +365,15 @@ static void free_lo_mount_fd(gpointer data) * * Pass @drop_mount_fd_ref == true if and only if this handle has a * strong reference to an lo_mount_fd object in the mount_fds hash - * table. That is always the case for file handles stored in lo_inode - * objects. + * table, i.e. if this file handle has been returned by a + * get_file_handle() call where *can_open_handle was returned to be + * true. (That is always the case for file handles stored in lo_inode + * objects, because those file handles must be open-able.) + * + * Conversely, pass @drop_mount_fd_ref == false if and only if this + * file handle has been returned by a get_file_handle() call where + * either NULL was passed for @can_open_handle, or where + * *can_open_handle was returned to be false. */ static void release_file_handle(struct lo_data *lo, struct lo_fhandle *fh, bool drop_mount_fd_ref) @@ -399,6 +412,196 @@ static void release_file_handle(struct lo_data *lo, struct lo_fhandle *fh, g_free(fh); } +/** + * Generate a file handle for the given dirfd/name combination. + * + * If mount_fds does not yet contain an entry for the handle's mount + * ID, (re)open dirfd/name in O_RDONLY mode and add it to mount_fds + * as the FD for that mount ID. (That is the file that we have + * generated a handle for, so it should be representative for the + * mount ID. However, to be sure (and to rule out races), we use + * statx() to verify that our assumption is correct.) + * + * Opening a mount FD can fail in various ways, and independently of + * whether generating a file handle was possible. Many callers only + * care about getting a file handle for a lookup, though, and so do + * not necessarily need it to be usable. (You need a valid mount FD + * for the handle to be usable.) + * *can_open_handle will be set to true if the file handle can be + * opened (i.e., we have a mount FD for it), and to false otherwise. + * By passing NULL for @can_open_handle, the caller indicates that + * they do not care about the handle being open-able, and so + * generating a mount FD will be skipped altogether. + * + * File handles must be freed with release_file_handle(). + */ +static struct lo_fhandle *get_file_handle(struct lo_data *lo, + int dirfd, const char *name, + bool *can_open_handle) +{ + /* We need statx() to verify the mount ID */ +#if defined(CONFIG_STATX) && defined(STATX_MNT_ID) + int root_path_fd = -1; + int mount_fd = -1; + struct lo_fhandle *fh = NULL; + struct lo_mount_fd *mfd; + int ret; + + if (!lo->use_statx || !lo->inode_file_handles || !lo->mountinfo_fp) { + goto fail_handle; + } + + fh = g_new0(struct lo_fhandle, 1); + + fh->handle.handle_bytes = sizeof(fh->padding) - sizeof(fh->handle); + ret = name_to_handle_at(dirfd, name, &fh->handle, &fh->mount_id, + AT_EMPTY_PATH); + if (ret < 0) { + goto fail_handle; + } + + if (!can_open_handle) { + /* No need to generate a mount FD if the caller does not care */ + return fh; + } + + if (pthread_rwlock_rdlock(&lo->mount_fds_lock)) { + goto fail_mount_fd; + } + + mfd = g_hash_table_lookup(lo->mount_fds, GINT_TO_POINTER(fh->mount_id)); + if (mfd) { + g_atomic_int_inc(&mfd->refcount); + } else { + char *mi_line = NULL; + size_t mi_line_len = 0; + char *mount_root = NULL; + struct statx stx; + char procname[64]; + + pthread_rwlock_unlock(&lo->mount_fds_lock); + + rewind(lo->mountinfo_fp); + while (!mount_root) { + ssize_t read_count; + int scan_count; + int mount_id; + + read_count = getline(&mi_line, &mi_line_len, lo->mountinfo_fp); + if (read_count < 0) { + break; + } + + scan_count = sscanf(mi_line, "%d %*d %*d:%*d %*s %ms", + &mount_id, &mount_root); + if (scan_count != 2 || mount_id != fh->mount_id) { + free(mount_root); + mount_root = NULL; + } + } + free(mi_line); + + if (!mount_root) { + goto fail_mount_fd; + } + + root_path_fd = open(mount_root, O_PATH); + free(mount_root); + if (root_path_fd < 0) { + goto fail_mount_fd; + } + + ret = statx(root_path_fd, "", AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, + STATX_TYPE | STATX_MNT_ID, &stx); + if (ret < 0) { + if (errno == ENOSYS) { + lo->use_statx = false; + fuse_log(FUSE_LOG_WARNING, + "statx() does not work: Will not be able to use file " + "handles for inodes\n"); + } + goto fail_mount_fd; + } + if (!(stx.stx_mask & STATX_MNT_ID) || stx.stx_mnt_id != fh->mount_id) { + /* + * Perhaps a TOCTTOU problem. Just return a non-openable file + * handle this time and retry for the next handle that we want to + * generate for this mount. + */ + goto fail_mount_fd; + } + if (!(stx.stx_mask & STATX_TYPE) || + !(S_ISREG(stx.stx_mode) || S_ISDIR(stx.stx_mode))) + { + /* + * We must not open special files with anything but O_PATH, so we + * cannot use this file for mount_fds. (Note that filesystems can + * have special files as their root node, so this case can happen.) + * Just return a failure in such a case and let the lo_inode have + * an O_PATH fd instead of a file handle. + */ + goto fail_mount_fd; + } + + /* Now that we know this fd is safe to open, do it */ + snprintf(procname, sizeof(procname), "%i", root_path_fd); + mount_fd = openat(lo->proc_self_fd, procname, O_RDONLY); + if (mount_fd < 0) { + goto fail_mount_fd; + } + + if (pthread_rwlock_wrlock(&lo->mount_fds_lock)) { + goto fail_mount_fd; + } + + /* Check again, might have changed */ + if (!g_hash_table_contains(lo->mount_fds, + GINT_TO_POINTER(fh->mount_id))) { + mfd = g_new(struct lo_mount_fd, 1); + + *mfd = (struct lo_mount_fd) { + .fd = mount_fd, + .refcount = 1, + }; + mount_fd = -1; /* reference moved to *mfd */ + + g_hash_table_insert(lo->mount_fds, + GINT_TO_POINTER(fh->mount_id), + mfd); + } + } + pthread_rwlock_unlock(&lo->mount_fds_lock); + + assert(can_open_handle != NULL); + *can_open_handle = true; + + goto out; + +fail_handle: + release_file_handle(lo, fh, false); + fh = NULL; + +fail_mount_fd: + if (can_open_handle) { + *can_open_handle = false; + } + +out: + if (root_path_fd >= 0) { + close(root_path_fd); + } + if (mount_fd >= 0) { + close(mount_fd); + } + return fh; +#else /* defined(CONFIG_STATX) && defined(STATX_MNT_ID) */ + if (can_open_handle) { + *can_open_handle = false; + } + return NULL; +#endif +} + /** * Open the given file handle with the given flags. * @@ -1244,6 +1447,11 @@ static int do_statx(struct lo_data *lo, int dirfd, const char *pathname, return -1; } lo->use_statx = false; + if (lo->inode_file_handles) { + fuse_log(FUSE_LOG_WARNING, + "statx() does not work: Will not be able to use file " + "handles for inodes\n"); + } /* fallback */ } #endif @@ -1273,6 +1481,8 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, struct lo_data *lo = lo_data(req); struct lo_inode *inode = NULL; struct lo_inode *dir = lo_inode(req, parent); + struct lo_fhandle *fh = NULL; + bool can_open_handle = false; if (inodep) { *inodep = NULL; /* in case there is an error */ @@ -1302,13 +1512,26 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, goto out; } - newfd = openat(dir_path_fd.fd, name, O_PATH | O_NOFOLLOW); - if (newfd == -1) { - goto out_err; + fh = get_file_handle(lo, dir_path_fd.fd, name, &can_open_handle); + if (!fh || !can_open_handle) { + /* + * If we will not be able to open the file handle again + * (can_open_handle is false), open an FD that we can put into + * lo_inode (in case we need to create a new lo_inode). + */ + newfd = openat(dir_path_fd.fd, name, O_PATH | O_NOFOLLOW); + if (newfd == -1) { + goto out_err; + } } - res = do_statx(lo, newfd, "", &e->attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, - &mnt_id); + if (newfd >= 0) { + res = do_statx(lo, newfd, "", &e->attr, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, &mnt_id); + } else { + res = do_statx(lo, dir_path_fd.fd, name, &e->attr, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, &mnt_id); + } if (res == -1) { goto out_err; } @@ -1318,9 +1541,19 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, e->attr_flags |= FUSE_ATTR_SUBMOUNT; } - inode = lo_find(lo, NULL, &e->attr, mnt_id); + /* + * Note that fh is always NULL if lo->inode_file_handles is false, + * and so we will never do a lookup by file handle here, and + * lo->inodes_by_handle will always remain empty. We only need + * this map when we do not have an O_PATH fd open for every + * lo_inode, though, so if inode_file_handles is false, we do not + * need that map anyway. + */ + inode = lo_find(lo, fh, &e->attr, mnt_id); if (inode) { - close(newfd); + if (newfd != -1) { + close(newfd); + } } else { inode = calloc(1, sizeof(struct lo_inode)); if (!inode) { @@ -1338,6 +1571,10 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, inode->nlookup = 1; inode->fd = newfd; + if (can_open_handle) { + inode->fhandle = fh; + fh = NULL; /* owned by the lo_inode now */ + } inode->key.ino = e->attr.st_ino; inode->key.dev = e->attr.st_dev; inode->key.mnt_id = mnt_id; @@ -1349,6 +1586,9 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, pthread_mutex_lock(&lo->mutex); inode->fuse_ino = lo_add_inode_mapping(req, inode); g_hash_table_insert(lo->inodes_by_ids, &inode->key, inode); + if (inode->fhandle) { + g_hash_table_insert(lo->inodes_by_handle, inode->fhandle, inode); + } pthread_mutex_unlock(&lo->mutex); } e->ino = inode->fuse_ino; @@ -1362,6 +1602,8 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, lo_inode_put(lo, &dir); + release_file_handle(lo, fh, can_open_handle); + fuse_log(FUSE_LOG_DEBUG, " %lli/%s -> %lli\n", (unsigned long long)parent, name, (unsigned long long)e->ino); @@ -1373,6 +1615,7 @@ out: if (newfd != -1) { close(newfd); } + release_file_handle(lo, fh, can_open_handle); lo_inode_put(lo, &inode); lo_inode_put(lo, &dir); return saverr; @@ -1695,6 +1938,7 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, int res; uint64_t mnt_id; struct stat attr; + struct lo_fhandle *fh; struct lo_data *lo = lo_data(req); struct lo_inode *dir = lo_inode(req, parent); struct lo_inode *inode = NULL; @@ -1708,13 +1952,17 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, goto out; } + fh = get_file_handle(lo, dir_path_fd.fd, name, NULL); + /* Ignore errors, this is just an optional key for the lookup */ + res = do_statx(lo, dir_path_fd.fd, name, &attr, AT_SYMLINK_NOFOLLOW, &mnt_id); if (res == -1) { goto out; } - inode = lo_find(lo, NULL, &attr, mnt_id); + inode = lo_find(lo, fh, &attr, mnt_id); + release_file_handle(lo, fh, false); out: lo_inode_put(lo, &dir); @@ -1882,6 +2130,9 @@ static void unref_inode(struct lo_data *lo, struct lo_inode *inode, uint64_t n) if (!inode->nlookup) { lo_map_remove(&lo->ino_map, inode->fuse_ino); g_hash_table_remove(lo->inodes_by_ids, &inode->key); + if (inode->fhandle) { + g_hash_table_remove(lo->inodes_by_handle, inode->fhandle); + } if (lo->posix_lock) { if (g_hash_table_size(inode->posix_locks)) { fuse_log(FUSE_LOG_WARNING, "Hash table is not empty\n"); @@ -3978,8 +4229,11 @@ static void setup_mounts(const char *source) /* * Only keep capabilities in allowlist that are needed for file system operation * The (possibly NULL) modcaps_in string passed in is free'd before exit. + * + * Passing true for cap_dac_read_search adds CAP_DAC_READ_SEARCH to the + * allowlist. */ -static void setup_capabilities(char *modcaps_in) +static void setup_capabilities(char *modcaps_in, bool cap_dac_read_search) { char *modcaps = modcaps_in; pthread_mutex_lock(&cap.mutex); @@ -4012,6 +4266,17 @@ static void setup_capabilities(char *modcaps_in) exit(1); } + /* + * If we need CAP_DAC_READ_SEARCH (for file handles), add that, too. + */ + if (cap_dac_read_search && + capng_update(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE, + CAP_DAC_READ_SEARCH)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for " + "CAP_DAC_READ_SEARCH\n", __func__); + exit(1); + } + /* * The modcaps option is a colon separated list of caps, * each preceded by either + or -. @@ -4158,7 +4423,7 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se, } setup_seccomp(enable_syslog); - setup_capabilities(g_strdup(lo->modcaps)); + setup_capabilities(g_strdup(lo->modcaps), lo->inode_file_handles); } /* Set the maximum number of open file descriptors */ @@ -4498,6 +4763,14 @@ int main(int argc, char *argv[]) lo.use_statx = true; +#if !defined(CONFIG_STATX) || !defined(STATX_MNT_ID) + if (lo.inode_file_handles) { + fuse_log(FUSE_LOG_WARNING, + "No statx() or mount ID support: Will not be able to use file " + "handles for inodes\n"); + } +#endif + se = fuse_session_new(&args, &lo_oper, sizeof(lo_oper), &lo); if (se == NULL) { goto err_out1; diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c index af04c638cb..ab4dc07e3f 100644 --- a/tools/virtiofsd/passthrough_seccomp.c +++ b/tools/virtiofsd/passthrough_seccomp.c @@ -73,6 +73,7 @@ static const int syscall_allowlist[] = { SCMP_SYS(mprotect), SCMP_SYS(mremap), SCMP_SYS(munmap), + SCMP_SYS(name_to_handle_at), SCMP_SYS(newfstatat), SCMP_SYS(statx), SCMP_SYS(open), From patchwork Thu Sep 16 08:40:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12498313 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7FEEEC433EF for ; Thu, 16 Sep 2021 08:53:48 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F3AD260F4C for ; Thu, 16 Sep 2021 08:53:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F3AD260F4C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:39582 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQn9P-000756-0p for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 04:53:47 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48470) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyh-0006w5-1r for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:43 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:47124) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQmyf-0005aY-0P for qemu-devel@nongnu.org; Thu, 16 Sep 2021 04:42:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631781760; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=a7L6wJecFIS2hzEromfrddFGo+PgXoIROWmqA13TApg=; b=gszSbPU/d5NZ41wuLJxe0mL6xHCJLdypNBkXMsLc0r/QMFmng3mUkXkl2HBTD8HU/jiEbJ fWgnbUnUEmDkqi06Oni9Nt2HtVxoJntCtCqzPF2NsLtrVDZHfTso/k2y+NMdV+mRI8aZ0o PMGDl3mp3yThJVTkyrkeWOJmS9gwGS4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-258-mkbcOAUsO7m6oEqyI_-Wow-1; Thu, 16 Sep 2021 04:42:39 -0400 X-MC-Unique: mkbcOAUsO7m6oEqyI_-Wow-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4EE161005E46 for ; Thu, 16 Sep 2021 08:42:38 +0000 (UTC) Received: from localhost (unknown [10.39.192.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 24B295D9CA; Thu, 16 Sep 2021 08:42:30 +0000 (UTC) From: Hanna Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v4 12/12] virtiofsd: Add lazy lo_do_find() Date: Thu, 16 Sep 2021 10:40:45 +0200 Message-Id: <20210916084045.31684-13-hreitz@redhat.com> In-Reply-To: <20210916084045.31684-1-hreitz@redhat.com> References: <20210916084045.31684-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.39, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hanna Reitz , Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" lo_find() right now takes two lookup keys for two maps, namely the file handle for inodes_by_handle and the statx information for inodes_by_ids. However, we only need the statx information if looking up the inode by the file handle failed. There are two callers of lo_find(): The first one, lo_do_lookup(), has both keys anyway, so passing them does not incur any additional cost. The second one, lookup_name(), though, needs to explicitly invoke name_to_handle_at() (through get_file_handle()) and statx() (through do_statx()). We need to try to get a file handle as the primary key, so we cannot get rid of get_file_handle(), but we only need the statx information if looking up an inode by handle failed; so we can defer that until the lookup has indeed failed. To this end, replace lo_find()'s st/mnt_id parameters by a get_ids() closure that is invoked to fill the lo_key struct if necessary. Also, lo_find() is renamed to lo_do_find(), so we can add a new lo_find() wrapper whose closure just initializes the lo_key from the st/mnt_id parameters, just like the old lo_find() did. lookup_name() directly calls lo_do_find() now and passes its own closure, which performs the do_statx() call. Signed-off-by: Hanna Reitz --- tools/virtiofsd/passthrough_ll.c | 93 +++++++++++++++++++++++++------- 1 file changed, 75 insertions(+), 18 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index e86fad8b2f..368bad17c7 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1348,22 +1348,23 @@ out_err: fuse_reply_err(req, saverr); } -static struct lo_inode *lo_find(struct lo_data *lo, - const struct lo_fhandle *fhandle, - struct stat *st, uint64_t mnt_id) +/* + * get_ids() will be called to get the key for lo->inodes_by_ids if + * the lookup by file handle has failed. + */ +static struct lo_inode *lo_do_find(struct lo_data *lo, + const struct lo_fhandle *fhandle, + int (*get_ids)(struct lo_key *, const void *), + const void *get_ids_opaque) { struct lo_inode *p = NULL; - struct lo_key ids_key = { - .ino = st->st_ino, - .dev = st->st_dev, - .mnt_id = mnt_id, - }; + struct lo_key ids_key; pthread_mutex_lock(&lo->mutex); if (fhandle) { p = g_hash_table_lookup(lo->inodes_by_handle, fhandle); } - if (!p) { + if (!p && get_ids(&ids_key, get_ids_opaque) == 0) { p = g_hash_table_lookup(lo->inodes_by_ids, &ids_key); /* * When we had to fall back to looking up an inode by its @@ -1392,6 +1393,36 @@ static struct lo_inode *lo_find(struct lo_data *lo, return p; } +struct lo_find_get_ids_key_opaque { + const struct stat *st; + uint64_t mnt_id; +}; + +static int lo_find_get_ids_key(struct lo_key *ids_key, const void *opaque) +{ + const struct lo_find_get_ids_key_opaque *stat_info = opaque; + + *ids_key = (struct lo_key){ + .ino = stat_info->st->st_ino, + .dev = stat_info->st->st_dev, + .mnt_id = stat_info->mnt_id, + }; + + return 0; +} + +static struct lo_inode *lo_find(struct lo_data *lo, + const struct lo_fhandle *fhandle, + struct stat *st, uint64_t mnt_id) +{ + const struct lo_find_get_ids_key_opaque stat_info = { + .st = st, + .mnt_id = mnt_id, + }; + + return lo_do_find(lo, fhandle, lo_find_get_ids_key, &stat_info); +} + /* value_destroy_func for posix_locks GHashTable */ static void posix_locks_value_destroy(gpointer data) { @@ -1930,14 +1961,41 @@ out: fuse_reply_err(req, saverr); } +struct lookup_name_get_ids_key_opaque { + struct lo_data *lo; + int parent_fd; + const char *name; +}; + +static int lookup_name_get_ids_key(struct lo_key *ids_key, const void *opaque) +{ + const struct lookup_name_get_ids_key_opaque *stat_params = opaque; + uint64_t mnt_id; + struct stat attr; + int res; + + res = do_statx(stat_params->lo, stat_params->parent_fd, stat_params->name, + &attr, AT_SYMLINK_NOFOLLOW, &mnt_id); + if (res < 0) { + return -errno; + } + + *ids_key = (struct lo_key){ + .ino = attr.st_ino, + .dev = attr.st_dev, + .mnt_id = mnt_id, + }; + + return 0; +} + /* Increments nlookup and caller must release refcount using lo_inode_put() */ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, const char *name) { g_auto(TempFd) dir_path_fd = TEMP_FD_INIT; int res; - uint64_t mnt_id; - struct stat attr; + struct lookup_name_get_ids_key_opaque stat_params; struct lo_fhandle *fh; struct lo_data *lo = lo_data(req); struct lo_inode *dir = lo_inode(req, parent); @@ -1955,13 +2013,12 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, fh = get_file_handle(lo, dir_path_fd.fd, name, NULL); /* Ignore errors, this is just an optional key for the lookup */ - res = do_statx(lo, dir_path_fd.fd, name, &attr, AT_SYMLINK_NOFOLLOW, - &mnt_id); - if (res == -1) { - goto out; - } - - inode = lo_find(lo, fh, &attr, mnt_id); + stat_params = (struct lookup_name_get_ids_key_opaque){ + .lo = lo, + .parent_fd = dir_path_fd.fd, + .name = name, + }; + inode = lo_do_find(lo, fh, lookup_name_get_ids_key, &stat_params); release_file_handle(lo, fh, false); out: