From patchwork Fri Jul 30 15:01:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411495 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 9177EC4338F for ; Fri, 30 Jul 2021 15:04:32 +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 5201660F46 for ; Fri, 30 Jul 2021 15:04:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 5201660F46 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]:54848 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U3q-00085G-E5 for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:04:30 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49184) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U29-0005Xg-UE for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:45 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:41323) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U27-0007n0-QJ for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657362; 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=l/mIIgvz3twLcuTXGIRgLgYVjiXbPrPC8d+/bqE4twI=; b=cUDX+75mqJnLrEmIpOtROTGOZjhfgYIIgQhzLj7qd7LuX4dfl5L8Jai2oY74DZvHLomqQX FNm201lLJ8mXQsgOfkmGXmxVSQpToSZTdG1//jpAuhwWA6VqlMO0CiAOVKGKMkNdzrs20c jqkV/4drp7mXBRWG0yehqm+2ENVqWTw= 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-545-pDd-DK7UO_CbdOS6kgOohQ-1; Fri, 30 Jul 2021 11:02:41 -0400 X-MC-Unique: pDd-DK7UO_CbdOS6kgOohQ-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 7A48083DC1D for ; Fri, 30 Jul 2021 15:01:46 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1819A5D9CA; Fri, 30 Jul 2021 15:01:45 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 01/10] virtiofsd: Limit setxattr()'s creds-dropped region Date: Fri, 30 Jul 2021 17:01:25 +0200 Message-Id: <20210730150134.216126-2-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max 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 38b2af8599..1f27eeabc5 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -3121,6 +3121,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 open_inode; if (block_xattr(lo, in_name)) { fuse_reply_err(req, EOPNOTSUPP); @@ -3155,7 +3156,24 @@ 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); + /* + * 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. + */ + open_inode = S_ISREG(inode->filetype) || S_ISDIR(inode->filetype); sprintf(procname, "%i", inode->fd); + if (open_inode) { + 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); + } + /* * If we are setting posix access acl and if SGID needs to be * cleared, then switch to caller's gid and drop CAP_FSETID @@ -3176,20 +3194,13 @@ 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 (open_inode) { + assert(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) @@ -3198,6 +3209,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, lo_restore_cred(&old, false); } + if (!open_inode) { + /* Change CWD back, fchdir should not fail here */ + FCHDIR_NOFAIL(lo->root.fd); + } + out: if (fd >= 0) { close(fd); From patchwork Fri Jul 30 15:01:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411519 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 CD56AC4338F for ; Fri, 30 Jul 2021 15:09:24 +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 71FA960F0F for ; Fri, 30 Jul 2021 15:09:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 71FA960F0F 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]:41046 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U8Z-0000tU-DJ for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:09:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49216) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2C-0005dd-MA for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:48 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:43916) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2B-0007pR-07 for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657366; 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=K9qTr1Z60td9Uc5D9UyWAoFPjwYkXcgxNFxStzHkNK8=; b=R+Dnkh5QuOkd8/ApHl2aiuM0G++ZZbuaKmu5jjkPHGOEtPU9o95hPbLhxddbVDIHdZB6E+ 593bJz2WqyTn0jaomVzX+NdzxMTWOeFkut4k7dTziVDJW3x5zPFyeilRW6Mo+ebHRGDHaj bD/qRye4os4z9f9MNqTyT8K0QiAE1uE= 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-486-0sYFTtoVOlW0k7J7aMJNAw-1; Fri, 30 Jul 2021 11:02:45 -0400 X-MC-Unique: 0sYFTtoVOlW0k7J7aMJNAw-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 948FD1966358 for ; Fri, 30 Jul 2021 15:01:48 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2EC2A5D9CA; Fri, 30 Jul 2021 15:01:48 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 02/10] virtiofsd: Add TempFd structure Date: Fri, 30 Jul 2021 17:01:26 +0200 Message-Id: <20210730150134.216126-3-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H4=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max Reitz Reviewed-by: Connor Kuehl --- tools/virtiofsd/passthrough_ll.c | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 1f27eeabc5..fb5e073e6a 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -178,6 +178,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), @@ -255,6 +277,33 @@ 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); + } +} + /* * Load capng's state from our saved state if the current thread * hadn't previously been loaded. From patchwork Fri Jul 30 15:01:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411517 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 05148C4338F for ; Fri, 30 Jul 2021 15:07:36 +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 9FC7B60F5C for ; Fri, 30 Jul 2021 15:07:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9FC7B60F5C 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]:35022 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U6o-0005Le-NU for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:07:34 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49262) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2L-00065F-0f for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:57 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:27697) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2J-0007ut-6s for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657374; 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=r1VrGYFOcGITCaAF5P5jko2GS5Uy5z7dr+tB0o0MQMI=; b=aMqmecLj0e6o7A4VA5wz1mwlaNEwvkSvDcEKA1scRdICnIam1Y3IHl9uxDtJ+Wz67e/ltK V9N4df1h73X7ctalULmBTaZRIuiiucC3hDAVx3jVAfde1ze+isNRjkhSyE+/A3HHdsREke 29t68GjCAWp+obEo6LYiWovRZ8eDfe8= 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-106-itB2ai6hNZSMqmHIQ0ieWQ-1; Fri, 30 Jul 2021 11:02:53 -0400 X-MC-Unique: itB2ai6hNZSMqmHIQ0ieWQ-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 092EA875054 for ; Fri, 30 Jul 2021 15:01:59 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6ABCE5C1CF; Fri, 30 Jul 2021 15:01:50 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 03/10] virtiofsd: Use lo_inode_open() instead of openat() Date: Fri, 30 Jul 2021 17:01:27 +0200 Message-Id: <20210730150134.216126-4-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H4=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max Reitz --- tools/virtiofsd/passthrough_ll.c | 43 ++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index fb5e073e6a..a444c3a7e2 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1729,18 +1729,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); @@ -1769,6 +1777,7 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino, out_errno: error = errno; out_err: + lo_inode_put(lo, &inode); if (d) { if (d->dp) { closedir(d->dp); @@ -2973,7 +2982,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 @@ -2981,13 +2989,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); @@ -3054,15 +3064,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); @@ -3211,14 +3222,14 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, * setxattr() on the link's filename there. */ open_inode = S_ISREG(inode->filetype) || S_ISDIR(inode->filetype); - sprintf(procname, "%i", inode->fd); if (open_inode) { - 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 { + sprintf(procname, "%i", inode->fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); } @@ -3317,16 +3328,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 Fri Jul 30 15:01:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411493 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 DECF5C4338F for ; Fri, 30 Jul 2021 15:04:24 +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 3A61C60F0F for ; Fri, 30 Jul 2021 15:04:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3A61C60F0F 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]:54520 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U3j-0007rU-9N for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:04:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49148) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U24-0005Tw-PL for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:40 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:37311) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U20-0007hY-RF for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657354; 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=beYdEtSEak1AFqrUwAMHq2FzEzVChIPfwG0NrMmknx0=; b=jHVl8OJ7YUAr3Zm3b56oRYzmES3AYlxJ8du8KKU0tCHGX3SlLAuSB7WfT+BTTdcS5GrvSw S2kaSNO/6AmBQgTYFvXwmLrSyhtw3M+YNVABEIqrCUnSvdSjV3eia9XXeNOM5IrbYSILfs Kxvi5eTA5EwLG4kBSXQSMhDBURGAC/0= 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-155-uTs2fGvYOmyr9GgFT4W4fQ-1; Fri, 30 Jul 2021 11:02:32 -0400 X-MC-Unique: uTs2fGvYOmyr9GgFT4W4fQ-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 CA4B310B0B07 for ; Fri, 30 Jul 2021 15:02:08 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C17E618A50; Fri, 30 Jul 2021 15:02:00 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 04/10] virtiofsd: Add lo_inode_fd() helper Date: Fri, 30 Jul 2021 17:01:28 +0200 Message-Id: <20210730150134.216126-5-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max Reitz --- tools/virtiofsd/passthrough_ll.c | 150 +++++++++++++++++++++++++------ 1 file changed, 125 insertions(+), 25 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index a444c3a7e2..86b901cf19 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -635,6 +635,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 @@ -822,11 +832,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) inode_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; @@ -836,7 +846,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, &inode_fd); + if (res < 0) { + saverr = -res; + goto out_err; + } /* If fi->fh is invalid we'll report EBADF later */ if (fi) { @@ -847,7 +861,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", inode_fd.fd); res = fchmodat(lo->proc_self_fd, procname, attr->st_mode, 0); } if (res == -1) { @@ -859,12 +873,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, inode_fd.fd); if (saverr) { goto out_err; } - res = fchownat(ifd, "", uid, gid, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); + res = fchownat(inode_fd.fd, "", uid, gid, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); if (res == -1) { saverr = errno; goto out_err; @@ -943,7 +958,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", inode_fd.fd); res = utimensat(lo->proc_self_fd, procname, tv, 0); } if (res == -1) { @@ -1058,7 +1073,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_fd = TEMP_FD_INIT; + int newfd = -1; int res; int saverr; uint64_t mnt_id; @@ -1088,7 +1104,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_fd); + if (res < 0) { + saverr = -res; + goto out; + } + + newfd = openat(dir_fd.fd, name, O_PATH | O_NOFOLLOW); if (newfd == -1) { goto out_err; } @@ -1155,6 +1177,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); } @@ -1312,6 +1335,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_fd = TEMP_FD_INIT; int res; int saverr; struct lo_data *lo = lo_data(req); @@ -1335,12 +1359,18 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent, return; } + res = lo_inode_fd(dir, &dir_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_fd.fd, name, link, mode, rdev); saverr = errno; @@ -1388,6 +1418,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) inode_fd = TEMP_FD_INIT; + g_auto(TempFd) parent_fd = TEMP_FD_INIT; int res; struct lo_data *lo = lo_data(req); struct lo_inode *parent_inode; @@ -1413,18 +1445,31 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, goto out_err; } + res = lo_inode_fd(inode, &inode_fd); + if (res < 0) { + errno = -res; + goto out_err; + } + + res = lo_inode_fd(parent_inode, &parent_fd); + if (res < 0) { + errno = -res; + goto out_err; + } + 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", inode_fd.fd); + res = linkat(lo->proc_self_fd, procname, parent_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(inode_fd.fd, "", &e.attr, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); if (res == -1) { goto out_err; } @@ -1453,23 +1498,33 @@ out_err: static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, const char *name) { + g_auto(TempFd) dir_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_fd); + if (res < 0) { + goto out; + } + + res = do_statx(lo, dir_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) @@ -1505,6 +1560,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_fd = TEMP_FD_INIT; + g_auto(TempFd) newparent_fd = TEMP_FD_INIT; int res; struct lo_inode *parent_inode; struct lo_inode *newparent_inode; @@ -1537,12 +1594,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_fd); + if (res < 0) { + fuse_reply_err(req, -res); + goto out; + } + + res = lo_inode_fd(newparent_inode, &newparent_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_fd.fd, name, + newparent_fd.fd, newname, flags); if (res == -1 && errno == ENOSYS) { fuse_reply_err(req, EINVAL); } else { @@ -1552,7 +1621,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_fd.fd, name, newparent_fd.fd, newname); fuse_reply_err(req, res == -1 ? errno : 0); out: @@ -2037,6 +2106,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_fd = TEMP_FD_INIT; int fd = -1; struct lo_data *lo = lo_data(req); struct lo_inode *parent_inode; @@ -2059,6 +2129,12 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name, return; } + err = lo_inode_fd(parent_inode, &parent_fd); + if (err < 0) { + err = -err; + goto out; + } + err = lo_change_cred(req, &old, lo->change_umask); if (err) { goto out; @@ -2067,7 +2143,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_fd.fd, name, fi->flags | O_CREAT | O_EXCL, mode); err = fd == -1 ? errno : 0; lo_restore_cred(&old, lo->change_umask); @@ -2929,6 +3005,7 @@ static int remove_blocked_xattrs(struct lo_data *lo, char *xattr_list, static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, size_t size) { + g_auto(TempFd) inode_fd = TEMP_FD_INIT; struct lo_data *lo = lo_data(req); g_autofree char *value = NULL; char procname[64]; @@ -2997,7 +3074,12 @@ 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); + ret = lo_inode_fd(inode, &inode_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", inode_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = getxattr(procname, name, value, size); @@ -3035,6 +3117,7 @@ out: static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) { + g_auto(TempFd) inode_fd = TEMP_FD_INIT; struct lo_data *lo = lo_data(req); g_autofree char *value = NULL; char procname[64]; @@ -3073,7 +3156,12 @@ 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); + ret = lo_inode_fd(inode, &inode_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", inode_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = listxattr(procname, value, size); @@ -3170,6 +3258,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) inode_fd = TEMP_FD_INIT; char procname[64]; const char *name; char *mapped_name; @@ -3229,7 +3318,12 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, goto out; } } else { - sprintf(procname, "%i", inode->fd); + ret = lo_inode_fd(inode, &inode_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", inode_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); } @@ -3286,6 +3380,7 @@ out: static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) { + g_auto(TempFd) inode_fd = TEMP_FD_INIT; char procname[64]; const char *name; char *mapped_name; @@ -3337,7 +3432,12 @@ 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); + ret = lo_inode_fd(inode, &inode_fd); + if (ret < 0) { + saverr = -ret; + goto out; + } + sprintf(procname, "%i", inode_fd.fd); /* fchdir should not fail here */ FCHDIR_NOFAIL(lo->proc_self_fd); ret = removexattr(procname, name); From patchwork Fri Jul 30 15:01:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411497 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 2F0ABC4338F for ; Fri, 30 Jul 2021 15:04: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 BE96660F46 for ; Fri, 30 Jul 2021 15:04:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BE96660F46 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]:55122 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U3z-0008GN-Vm for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:04:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49200) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2B-0005bH-MV for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:47 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:40635) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U29-0007oi-TO for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657365; 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=4Ih1KdDIwl8x1l9qljSNCVjeeiBQljD/ocdwa7SAgfA=; b=Y9oOeaCBTKO3urXFClcvQqe/KZunpvG/dvJLC915XxszY892hIEGO/kWDnB5krZsyjH8rV BjcWyW5n/3ZiPTwBwon3lxTE4amAvqVzWshoTqidRkPFcByQBgau3WVuoCnPhbRtYa+1GI y4y/vkekGbasMnx4uT4dM+olKJvQkS8= 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-x0SzNmqJPIuQkRYOvvDo_Q-1; Fri, 30 Jul 2021 11:02:43 -0400 X-MC-Unique: x0SzNmqJPIuQkRYOvvDo_Q-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 BD40384F20E for ; Fri, 30 Jul 2021 15:02:18 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 79BDE5C1D1; Fri, 30 Jul 2021 15:02:11 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 05/10] virtiofsd: Let lo_fd() return a TempFd Date: Fri, 30 Jul 2021 17:01:29 +0200 Message-Id: <20210730150134.216126-6-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max Reitz Reviewed-by: Connor Kuehl --- 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 86b901cf19..9e1bc37af8 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -650,18 +650,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; } /* @@ -798,14 +799,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) ino_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, &ino_fd); + if (res < 0) { + return (void)fuse_reply_err(req, -res); + } + + res = fstatat(ino_fd.fd, "", &buf, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); if (res == -1) { return (void)fuse_reply_err(req, errno); } @@ -1529,6 +1535,7 @@ out: static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name) { + g_auto(TempFd) parent_fd = TEMP_FD_INIT; int res; struct lo_inode *inode; struct lo_data *lo = lo_data(req); @@ -1543,13 +1550,19 @@ static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name) return; } + res = lo_fd(req, parent, &parent_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_fd.fd, name, AT_REMOVEDIR); fuse_reply_err(req, res == -1 ? errno : 0); unref_inode_lolocked(lo, inode, 1); @@ -1635,6 +1648,7 @@ out: static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name) { + g_auto(TempFd) parent_fd = TEMP_FD_INIT; int res; struct lo_inode *inode; struct lo_data *lo = lo_data(req); @@ -1649,13 +1663,19 @@ static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name) return; } + res = lo_fd(req, parent, &parent_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_fd.fd, name, 0); fuse_reply_err(req, res == -1 ? errno : 0); unref_inode_lolocked(lo, inode, 1); @@ -1735,10 +1755,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) ino_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, &ino_fd); + if (res < 0) { + return (void)fuse_reply_err(req, -res); + } + + res = readlinkat(ino_fd.fd, "", buf, sizeof(buf)); if (res == -1) { return (void)fuse_reply_err(req, errno); } @@ -2535,10 +2561,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) ino_fd = TEMP_FD_INIT; int res; struct statvfs stbuf; - res = fstatvfs(lo_fd(req, ino), &stbuf); + res = lo_fd(req, ino, &ino_fd); + if (res < 0) { + fuse_reply_err(req, -res); + return; + } + + res = fstatvfs(ino_fd.fd, &stbuf); if (res == -1) { fuse_reply_err(req, errno); } else { From patchwork Fri Jul 30 15:01:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411537 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 DAA26C4338F for ; Fri, 30 Jul 2021 15:11:30 +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 38F2160F94 for ; Fri, 30 Jul 2021 15:11:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 38F2160F94 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]:48670 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9UAb-00063i-BJ for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:11:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49290) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2N-0006DN-Qj for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:59 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:55048) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2L-0007x4-Hh for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657377; 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=zj6FlfRk0Y1iH5pn3wm7uFE+2a56sGUjaPe4skrCOY8=; b=Sjv0I4INhrSMgN8vZ7z/h5OFMJZc7KCIBPNJJJww2K8aGqtH1OwVLwB9XPhSdJARBAqwgJ k0HR/ADSkBP/ZiwfRwAj0XdIiUOW/nowuhIT2jYyGrNvh8J9xbkwt6/Nep61ee5WPrnV46 lluH/ctzwyDK+rhN3VHNoEQK7KEtyQA= 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-285-R8SrVj77NBeaBDyrMtrILw-1; Fri, 30 Jul 2021 11:02:55 -0400 X-MC-Unique: R8SrVj77NBeaBDyrMtrILw-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 DF3188914F8 for ; Fri, 30 Jul 2021 15:02:29 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6EC4710074FD; Fri, 30 Jul 2021 15:02:20 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 06/10] virtiofsd: Let lo_inode_open() return a TempFd Date: Fri, 30 Jul 2021 17:01:30 +0200 Message-Id: <20210730150134.216126-7-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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. However, auto-cleanup is nice, and in some cases this plays nicely with an lo_inode_fd() call in another conditional branch (see lo_setattr()). Signed-off-by: Max Reitz --- tools/virtiofsd/passthrough_ll.c | 138 +++++++++++++------------------ 1 file changed, 59 insertions(+), 79 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 9e1bc37af8..292b7f7e27 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -291,10 +291,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; @@ -673,9 +671,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) +static int lo_inode_open(const struct lo_data *lo, const struct lo_inode *inode, + int open_flags, TempFd *tfd) { g_autofree char *fd_str = g_strdup_printf("%d", inode->fd); int fd; @@ -694,7 +695,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) @@ -852,7 +859,12 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, return; } - res = lo_inode_fd(inode, &inode_fd); + if (!fi && (valid & FUSE_SET_ATTR_SIZE)) { + /* We need an O_RDWR FD for ftruncate() */ + res = lo_inode_open(lo, inode, O_RDWR, &inode_fd); + } else { + res = lo_inode_fd(inode, &inode_fd); + } if (res < 0) { saverr = -res; goto out_err; @@ -900,18 +912,11 @@ 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; - } + truncfd = inode_fd.fd; } saverr = drop_security_capability(lo, truncfd); if (saverr) { - if (!fi) { - close(truncfd); - } goto out_err; } @@ -919,9 +924,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; } } @@ -934,9 +936,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; } @@ -1822,11 +1821,12 @@ 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) inode_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); @@ -1840,13 +1840,13 @@ 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, &inode_fd); + if (res < 0) { + error = -res; goto out_err; } - d->dp = fdopendir(fd); + d->dp = fdopendir(temp_fd_steal(&inode_fd)); if (d->dp == NULL) { goto out_errno; } @@ -1876,8 +1876,6 @@ out_err: if (d) { if (d->dp) { closedir(d->dp); - } else if (fd != -1) { - close(fd); } free(d); } @@ -2077,6 +2075,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) inode_fd = TEMP_FD_INIT; ssize_t fh; int fd = existing_fd; int err; @@ -2093,16 +2092,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, &inode_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(&inode_fd); + if (fi->flags & (O_TRUNC)) { int err = drop_security_capability(lo, fd); if (err) { @@ -2212,8 +2213,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) inode_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)); @@ -2230,15 +2232,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, &inode_fd); + if (res < 0) { + *err = -res; free(plock); return NULL; } plock->lock_owner = lock_owner; - plock->fd = fd; + plock->fd = temp_fd_steal(&inode_fd); g_hash_table_insert(inode->posix_locks, GUINT_TO_POINTER(plock->lock_owner), plock); return plock; @@ -2454,6 +2456,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) inode_fd = TEMP_FD_INIT; struct lo_inode *inode = lo_inode(req, ino); struct lo_data *lo = lo_data(req); int res; @@ -2468,11 +2471,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, &inode_fd); + if (res < 0) { + res = -res; goto out; } + fd = inode_fd.fd; } else { fd = lo_fi_fd(req, fi); } @@ -2482,9 +2486,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); @@ -3047,7 +3048,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); @@ -3099,12 +3099,12 @@ 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; + ret = lo_inode_open(lo, inode, O_RDONLY, &inode_fd); + if (ret < 0) { + saverr = -ret; goto out; } - ret = fgetxattr(fd, name, value, size); + ret = fgetxattr(inode_fd.fd, name, value, size); saverr = ret == -1 ? errno : 0; } else { ret = lo_inode_fd(inode, &inode_fd); @@ -3133,10 +3133,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; @@ -3157,7 +3153,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) { @@ -3181,12 +3176,12 @@ 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; + ret = lo_inode_open(lo, inode, O_RDONLY, &inode_fd); + if (ret < 0) { + saverr = -ret; goto out; } - ret = flistxattr(fd, value, size); + ret = flistxattr(inode_fd.fd, value, size); saverr = ret == -1 ? errno : 0; } else { ret = lo_inode_fd(inode, &inode_fd); @@ -3273,10 +3268,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; @@ -3299,7 +3290,6 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, 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 = {}; @@ -3345,9 +3335,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, */ open_inode = S_ISREG(inode->filetype) || S_ISDIR(inode->filetype); if (open_inode) { - fd = lo_inode_open(lo, inode, O_RDONLY); - if (fd < 0) { - saverr = -fd; + ret = lo_inode_open(lo, inode, O_RDONLY, &inode_fd); + if (ret < 0) { + saverr = -ret; goto out; } } else { @@ -3382,8 +3372,7 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, switched_creds = true; } if (open_inode) { - assert(fd >= 0); - ret = fsetxattr(fd, name, value, size, flags); + ret = fsetxattr(inode_fd.fd, name, value, size, flags); saverr = ret == -1 ? errno : 0; } else { ret = setxattr(procname, name, value, size, flags); @@ -3402,10 +3391,6 @@ static void lo_setxattr(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); @@ -3421,7 +3406,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); @@ -3457,12 +3441,12 @@ 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; + ret = lo_inode_open(lo, inode, O_RDONLY, &inode_fd); + if (ret < 0) { + saverr = -ret; goto out; } - ret = fremovexattr(fd, name); + ret = fremovexattr(inode_fd.fd, name); saverr = ret == -1 ? errno : 0; } else { ret = lo_inode_fd(inode, &inode_fd); @@ -3479,10 +3463,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 Fri Jul 30 15:01:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411515 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 DA9C3C4338F for ; Fri, 30 Jul 2021 15:07:21 +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 74B8160F5C for ; Fri, 30 Jul 2021 15:07:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 74B8160F5C 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]:34462 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U6W-0004xI-S3 for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:07:20 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49240) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2F-0005mg-Uy for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:51 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:39090) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2D-0007rZ-Pf for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657369; 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=T6DDetEgy+ESxxObNvCSGjm47upq+ySPVGiUWuVXFWs=; b=a4S0mjblY/ZOb0UArUdvvfhgyuFXAgK+bPJwq2bFQIJdlg562owsaWRA6ksVG8lzdqmJR3 X9YpP280frg9t24QuVX9FQeW3Mtpf5uOLoSiFsbA8LI1DoCpt7lmmVOcPigyC+6QLH9S4X idX0WJAzKVT6wYlsiw/pFzjqzGmcolc= 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-251-Yw9YiOFDOxa_sWZMjPf9AQ-1; Fri, 30 Jul 2021 11:02:48 -0400 X-MC-Unique: Yw9YiOFDOxa_sWZMjPf9AQ-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 B9D08180DE11 for ; Fri, 30 Jul 2021 15:02:38 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id ACC195C1CF; Fri, 30 Jul 2021 15:02:31 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 07/10] virtiofsd: Add lo_inode.fhandle Date: Fri, 30 Jul 2021 17:01:31 +0200 Message-Id: <20210730150134.216126-8-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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. Signed-off-by: Max Reitz Reviewed-by: Connor Kuehl --- tools/virtiofsd/passthrough_ll.c | 116 ++++++++++++++++++++++---- tools/virtiofsd/passthrough_seccomp.c | 1 + 2 files changed, 102 insertions(+), 15 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 292b7f7e27..487448d666 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -88,8 +88,25 @@ 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; +}; + +/* Maps mount IDs to an FD that we can pass to open_by_handle_at() */ +static GHashTable *mount_fds; +pthread_rwlock_t mount_fds_lock = PTHREAD_RWLOCK_INITIALIZER; + struct lo_inode { + /* + * Either of 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 @@ -302,6 +319,44 @@ static int temp_fd_steal(TempFd *temp_fd) } } +/** + * 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(const struct lo_fhandle *fh, int flags) +{ + gpointer mount_fd_ptr; + int mount_fd; + bool found; + int ret; + + ret = pthread_rwlock_rdlock(&mount_fds_lock); + if (ret) { + return -ret; + } + + /* mount_fd == 0 is valid, so we need lookup_extended */ + found = g_hash_table_lookup_extended(mount_fds, + GINT_TO_POINTER(fh->mount_id), + NULL, &mount_fd_ptr); + pthread_rwlock_unlock(&mount_fds_lock); + if (!found) { + return -EINVAL; + } + mount_fd = GPOINTER_TO_INT(mount_fd_ptr); + + ret = open_by_handle_at(mount_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. @@ -608,7 +663,11 @@ 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); + } else { + g_free(inode->fhandle); + } free(inode); } } @@ -635,10 +694,25 @@ static struct lo_inode *lo_inode(fuse_req_t req, fuse_ino_t ino) static int lo_inode_fd(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(inode->fhandle, O_PATH); + if (fd < 0) { + return -errno; + } + + *tfd = (TempFd) { + .fd = fd, + .owned = true, + }; + } return 0; } @@ -678,22 +752,32 @@ static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd *tfd) static int lo_inode_open(const struct lo_data *lo, const 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(inode->fhandle, open_flags); + if (fd < 0) { + return fd; + } } *tfd = (TempFd) { @@ -4110,6 +4194,8 @@ int main(int argc, char *argv[]) lo.root.fuse_ino = FUSE_ROOT_ID; lo.cache = CACHE_AUTO; + mount_fds = g_hash_table_new(NULL, NULL); + /* * 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 Fri Jul 30 15:01:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411499 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 08021C4338F for ; Fri, 30 Jul 2021 15:04:50 +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 7EFBB60F0F for ; Fri, 30 Jul 2021 15:04:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 7EFBB60F0F 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]:55850 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U48-0000Ia-Kb for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:04:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49250) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2H-0005sS-OL for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:53 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:47837) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2F-0007sK-Mk for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:02:53 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657371; 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=Z5wooyjG0Iq3OHLEBZ67TxDToG5rsd2A4o6LIUKZpvY=; b=VUR9wmq8+e66GSt+tP29xR6qi8nPM+2PCbaFaufSqXqINzTO6UdU8XSk4wldKdd/vJhyLd 5UZnak6HD5+zTs3jLV3s1hUcKjBQ/CmUroWRRLN6qIFcTzQVyw2VkUfKkfn1gXlCGYl37x WZhgXDnLog0gZPpm+ry6ZFNSajrDeAU= 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-431-1OprWJ-sMWq1Jnc5lb_gFA-1; Fri, 30 Jul 2021 11:02:49 -0400 X-MC-Unique: 1OprWJ-sMWq1Jnc5lb_gFA-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 CCB85180FCD1 for ; Fri, 30 Jul 2021 15:02:48 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9F9C65D9CA; Fri, 30 Jul 2021 15:02:40 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 08/10] virtiofsd: Add inodes_by_handle hash table Date: Fri, 30 Jul 2021 17:01:32 +0200 Message-Id: <20210730150134.216126-9-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max 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 487448d666..f9d8b2f134 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -180,7 +180,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 */ @@ -263,8 +264,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); @@ -1064,18 +1066,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++; @@ -1215,7 +1239,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 { @@ -1245,7 +1269,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; @@ -1609,7 +1633,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); @@ -1776,7 +1800,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"); @@ -3603,7 +3627,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; } @@ -4129,10 +4153,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_ids) { + g_hash_table_destroy(lo->inodes_by_handle); } if (lo->root.posix_locks) { @@ -4189,7 +4237,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 Fri Jul 30 15:01:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411521 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 8D6AAC4338F for ; Fri, 30 Jul 2021 15:09:43 +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 1B17E60F5C for ; Fri, 30 Jul 2021 15:09:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 1B17E60F5C 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]:42588 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U8s-0001wA-5N for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:09:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49336) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2X-0006YY-8H for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:03:09 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:25081) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2U-00083L-UW for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:03:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657386; 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=LuYrN8GxFWd82heqYyENzZMStNxvD3WWBdVTcmX1GFU=; b=YkzoHR4tZbOiKWPGrLbxWfJ28R25g9C/OkODSmX8+IazpTfc/+Im34pgRV4tgGIfx7BtC8 2D7JXNkyiwZkKbTu5vXtKa7LN3f5Zn5589XQLhlZsXk/mwPzSyqU6B8mNKiVxtso7uvHYK nKUd4aIf1GeiVbVbXxv3fzUp3GJlpDc= 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-177-12tIIcIqNS6yp-F-qOOuCw-1; Fri, 30 Jul 2021 11:02:58 -0400 X-MC-Unique: 12tIIcIqNS6yp-F-qOOuCw-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 09C71800D55 for ; Fri, 30 Jul 2021 15:02:58 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9CF205D6A1; Fri, 30 Jul 2021 15:02:50 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 09/10] virtiofsd: Optionally fill lo_inode.fhandle Date: Fri, 30 Jul 2021 17:01:33 +0200 Message-Id: <20210730150134.216126-10-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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 the description text tells the user they will also need to specify -o modcaps=+dac_read_search. 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() fills the hash map by opening the file we have generated a handle for. 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: Max Reitz --- tools/virtiofsd/helper.c | 3 + tools/virtiofsd/passthrough_ll.c | 194 ++++++++++++++++++++++++-- tools/virtiofsd/passthrough_seccomp.c | 1 + 3 files changed, 190 insertions(+), 8 deletions(-) diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c index a8295d975a..aa63a21d43 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" + " (requires -o modcaps=+dac_read_search)\n" ); } diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index f9d8b2f134..ac95961d12 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -194,6 +194,7 @@ struct lo_data { /* If set, virtiofsd is responsible for setting umask during creation */ bool change_umask; int user_posix_acl, posix_acl; + int inode_file_handles; }; /** @@ -250,6 +251,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; @@ -321,6 +326,135 @@ static int temp_fd_steal(TempFd *temp_fd) } } +/** + * 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.) + */ +static struct lo_fhandle *get_file_handle(struct lo_data *lo, + int dirfd, const char *name) +{ + /* We need statx() to verify the mount ID */ +#if defined(CONFIG_STATX) && defined(STATX_MNT_ID) + struct lo_fhandle *fh; + int ret; + + if (!lo->use_statx || !lo->inode_file_handles) { + return NULL; + } + + 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; + } + + if (pthread_rwlock_rdlock(&mount_fds_lock)) { + goto fail; + } + if (!g_hash_table_contains(mount_fds, GINT_TO_POINTER(fh->mount_id))) { + g_auto(TempFd) path_fd = TEMP_FD_INIT; + struct statx stx; + char procname[64]; + int fd; + + pthread_rwlock_unlock(&mount_fds_lock); + + /* + * Before opening an O_RDONLY fd, check whether dirfd/name is a regular + * file or directory, because we must not open anything else with + * anything but O_PATH. + * (And we use that occasion to verify that the file has the mount ID we + * need.) + */ + if (name[0]) { + path_fd.fd = openat(dirfd, name, O_PATH); + if (path_fd.fd < 0) { + goto fail; + } + path_fd.owned = true; + } else { + path_fd.fd = dirfd; + path_fd.owned = false; + } + + ret = statx(path_fd.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; + } + if (!(stx.stx_mask & STATX_MNT_ID) || stx.stx_mnt_id != fh->mount_id) { + /* + * One reason for stx_mnt_id != mount_id could be that dirfd/name + * is a directory, and some other filesystem was mounted there + * between us generating the file handle and then opening the FD. + * (Other kinds of races might be possible, too.) + * Failing this function is not fatal, though, because our caller + * (lo_do_lookup()) will just fall back to opening an O_PATH FD to + * store in lo_inode.fd instead of storing a file handle in + * lo_inode.fhandle. So we do not need to try too hard to get an + * FD for fh->mount_id so this function could succeed. + */ + goto fail; + } + 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. + * 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; + } + + /* Now that we know this fd is safe to open, do it */ + snprintf(procname, sizeof(procname), "%i", path_fd.fd); + fd = openat(lo->proc_self_fd, procname, O_RDONLY); + if (fd < 0) { + goto fail; + } + + if (pthread_rwlock_wrlock(&mount_fds_lock)) { + goto fail; + } + + /* Check again, might have changed */ + if (g_hash_table_contains(mount_fds, GINT_TO_POINTER(fh->mount_id))) { + close(fd); + } else { + g_hash_table_insert(mount_fds, + GINT_TO_POINTER(fh->mount_id), + GINT_TO_POINTER(fd)); + } + } + pthread_rwlock_unlock(&mount_fds_lock); + + return fh; + +fail: + free(fh); + return NULL; +#else /* defined(CONFIG_STATX) && defined(STATX_MNT_ID) */ + return NULL; +#endif +} + /** * Open the given file handle with the given flags. * @@ -1165,6 +1299,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 @@ -1194,6 +1333,7 @@ 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; if (inodep) { *inodep = NULL; /* in case there is an error */ @@ -1223,13 +1363,21 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, goto out; } - newfd = openat(dir_fd.fd, name, O_PATH | O_NOFOLLOW); - if (newfd == -1) { - goto out_err; + fh = get_file_handle(lo, dir_fd.fd, name); + if (!fh) { + newfd = openat(dir_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_fd.fd, name, &e->attr, + AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, &mnt_id); + } if (res == -1) { goto out_err; } @@ -1239,9 +1387,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) { @@ -1259,6 +1417,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name, inode->nlookup = 1; inode->fd = newfd; + inode->fhandle = fh; inode->key.ino = e->attr.st_ino; inode->key.dev = e->attr.st_dev; inode->key.mnt_id = mnt_id; @@ -1270,6 +1429,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; @@ -1615,6 +1777,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; @@ -1628,12 +1791,16 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, goto out; } + fh = get_file_handle(lo, dir_fd.fd, name); + /* Ignore errors, this is just an optional key for the lookup */ + res = do_statx(lo, dir_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); + g_free(fh); out: lo_inode_put(lo, &dir); @@ -1801,6 +1968,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"); @@ -4362,6 +4532,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 Fri Jul 30 15:01:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 12411523 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 4ECD0C4338F for ; Fri, 30 Jul 2021 15:10:11 +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 EA1EC60F0F for ; Fri, 30 Jul 2021 15:10:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EA1EC60F0F 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]:44952 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m9U9K-0003Vv-3Q for qemu-devel@archiver.kernel.org; Fri, 30 Jul 2021 11:10:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49370) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2e-0006pv-0w for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:03:16 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:51734) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m9U2a-00086k-Gt for qemu-devel@nongnu.org; Fri, 30 Jul 2021 11:03:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627657392; 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=2ptimhwjg3rPLP1MIo8d26xTSfzWbXfiMCW8NqEp2Vg=; b=FdvDR5+7a9EwVp9NoramsoIlGfJDaKuRJLf3ukS+EuqgLjGmmqSv6O8rwWCrZSvFdjZjub vEOZhyCPYdIB99uAJ8Dkj325FZ7VCsTRKs4yagwZj6pabunx4A8P0tC2J/Q8ejySy2kmg0 8Cn2BUaAdDGjiQ37SbsF1vcAWOvQXKQ= 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-283-ConVPegUMzmgUdI2SBuIoQ-1; Fri, 30 Jul 2021 11:03:10 -0400 X-MC-Unique: ConVPegUMzmgUdI2SBuIoQ-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 BE24510060E2 for ; Fri, 30 Jul 2021 15:03:09 +0000 (UTC) Received: from localhost (unknown [10.39.193.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BEF0F27094; Fri, 30 Jul 2021 15:02:59 +0000 (UTC) From: Max Reitz To: qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: [PATCH v3 10/10] virtiofsd: Add lazy lo_do_find() Date: Fri, 30 Jul 2021 17:01:34 +0200 Message-Id: <20210730150134.216126-11-mreitz@redhat.com> In-Reply-To: <20210730150134.216126-1-mreitz@redhat.com> References: <20210730150134.216126-1-mreitz@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=mreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=mreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.717, 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_H3=0.001, RCVD_IN_MSPIKE_WL=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: Stefan Hajnoczi , "Dr . David Alan Gilbert" , Vivek Goyal , Max Reitz 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: Max Reitz --- tools/virtiofsd/passthrough_ll.c | 93 ++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 17 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index ac95961d12..41e9f53878 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1200,22 +1200,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 @@ -1244,6 +1245,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) { @@ -1769,14 +1800,41 @@ out_err: 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_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); @@ -1794,12 +1852,13 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, fh = get_file_handle(lo, dir_fd.fd, name); /* Ignore errors, this is just an optional key for the lookup */ - res = do_statx(lo, dir_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_fd.fd, + .name = name, + }; + inode = lo_do_find(lo, fh, lookup_name_get_ids_key, &stat_params); + lo_inode_put(lo, &dir); g_free(fh); out: