From patchwork Sun Nov 5 15:56:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 161656 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:8f47:0:b0:403:3b70:6f57 with SMTP id j7csp2193147vqu; Sun, 5 Nov 2023 08:02:55 -0800 (PST) X-Google-Smtp-Source: AGHT+IE5hH/MCh14He8c9f/U9tUCx2wJpIw+oBZKJ7wD1ggkhNU4JvtpWxn4CWP3YJck5fyhb/C2 X-Received: by 2002:a4a:df58:0:b0:581:f2de:25f8 with SMTP id j24-20020a4adf58000000b00581f2de25f8mr24664165oou.0.1699200175679; Sun, 05 Nov 2023 08:02:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699200175; cv=none; d=google.com; s=arc-20160816; b=lKOsSL2jjAAEgqKKruvjwA8aoL+x6OvsfedJLXr024qJuRdOvjvntyBzVdZwndzBe+ nsI66tysl3iXsU2gCASNe+87EuPsxWAgleMixTsL9VuIZefLEtG7IW/LIlKXaeHKznU1 QplX16CsK9fA63jqyUt+qOHzUEF2rGDXj0tpoqTMsgKIJFT0D5C2DbTpFqe3OFN1JHSB tec2uYkYGWu4wKVi+O83nlHCeUy0gF/3i3TUfLoGnj7A8RoZ9PjbNfofNRaGCf1AHy+b sV+Lc+i0KaFVaK/LkYhYMOCGpJUKZRBBzqzOdDXzN50Xl69e2kaZbNH8afZgjaJ/l5bZ 25zQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:references:subject:cc:to:from:date :user-agent:message-id; bh=/qfgYmNoqcFfOxiNZnt1Q2KtxlwE65KhVVkh22rVT9c=; fh=5fXiwuINd15usHF5af0bVHUWftYpivNmVKkJ+nuVsWg=; b=Py3mGhIbSVUcRlKVzP/J3OY0OLQ4ys5K7E00JxGXBTYwocaMeA/qKUIyL/XvEK2i8O IcIOtuiQ1r4qaIJw1MLUfHre+x87de24s2GLSda1cZwF3dJg2innOi3JFZF7/fTcjSRl Wevn+ByRTwhu8oyNeVeaFb1+5XiFIIxUetwjP6ySlHPz5oM5NyfkN3ZyaFjMA6i4nt7t 4/rKveUC/lAlyb4LArDqVkBxA8TfsilFpl495A71XMtIFTULut0iAfh1ygh+hr60s2Or hAY2fcTG85cphb7bNtR8d8VxwDiplAPymeMo+j0JLhNqYbxrqX2KQNCxnbWs5Q8qzwc2 UIkg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from lipwig.vger.email (lipwig.vger.email. [23.128.96.33]) by mx.google.com with ESMTPS id r6-20020a4a4e06000000b0057b80b61a12si2366626ooa.86.2023.11.05.08.02.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Nov 2023 08:02:55 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id CA0E08056999; Sun, 5 Nov 2023 08:02:52 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229731AbjKEQB6 (ORCPT + 34 others); Sun, 5 Nov 2023 11:01:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46894 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229562AbjKEQBn (ORCPT ); Sun, 5 Nov 2023 11:01:43 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3DBBE100; Sun, 5 Nov 2023 08:01:40 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8866EC433C7; Sun, 5 Nov 2023 16:01:38 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97-RC3) (envelope-from ) id 1qzfZD-00000000CiC-41xO; Sun, 05 Nov 2023 11:01:39 -0500 Message-ID: <20231105160139.821908367@goodmis.org> User-Agent: quilt/0.67 Date: Sun, 05 Nov 2023 10:56:34 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Cc: Masami Hiramatsu , Mark Rutland , Andrew Morton , Ajay Kaher Subject: [v6.6][PATCH 4/5] eventfs: Delete eventfs_inode when the last dentry is freed References: <20231105155630.925114107@goodmis.org> MIME-Version: 1.0 X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Sun, 05 Nov 2023 08:02:52 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781740523731889458 X-GMAIL-MSGID: 1781740523731889458 From: "Steven Rostedt (Google)" commit 020010fbfa202aa528a52743eba4ab0da3400a4e upstream There exists a race between holding a reference of an eventfs_inode dentry and the freeing of the eventfs_inode. If user space has a dentry held long enough, it may still be able to access the dentry's eventfs_inode after it has been freed. To prevent this, have he eventfs_inode freed via the last dput() (or via RCU if the eventfs_inode does not have a dentry). This means reintroducing the eventfs_inode del_list field at a temporary place to put the eventfs_inode. It needs to mark it as freed (via the list) but also must invalidate the dentry immediately as the return from eventfs_remove_dir() expects that they are. But the dentry invalidation must not be called under the eventfs_mutex, so it must be done after the eventfs_inode is marked as free (put on a deletion list). Link: https://lkml.kernel.org/r/20231101172650.123479767@goodmis.org Cc: stable@vger.kernel.org Cc: Masami Hiramatsu Cc: Mark Rutland Cc: Andrew Morton Cc: Ajay Kaher Fixes: 5bdcd5f5331a2 ("eventfs: Implement removal of meta data from eventfs") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 150 +++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 76 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 6a3f7502310c..7aa92b8ebc51 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -53,10 +53,12 @@ struct eventfs_file { const struct inode_operations *iop; /* * Union - used for deletion + * @llist: for calling dput() if needed after RCU * @del_list: list of eventfs_file to delete * @rcu: eventfs_file to delete in RCU */ union { + struct llist_node llist; struct list_head del_list; struct rcu_head rcu; }; @@ -113,8 +115,7 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, mutex_lock(&eventfs_mutex); ef = dentry->d_fsdata; - /* The LSB is set when the eventfs_inode is being freed */ - if (((unsigned long)ef & 1UL) || ef->is_freed) { + if (ef->is_freed) { /* Do not allow changes if the event is about to be removed. */ mutex_unlock(&eventfs_mutex); return -ENODEV; @@ -258,6 +259,13 @@ static struct dentry *create_dir(struct eventfs_file *ef, return eventfs_end_creating(dentry); } +static void free_ef(struct eventfs_file *ef) +{ + kfree(ef->name); + kfree(ef->ei); + kfree(ef); +} + /** * eventfs_set_ef_status_free - set the ef->status to free * @ti: the tracefs_inode of the dentry @@ -270,34 +278,20 @@ void eventfs_set_ef_status_free(struct tracefs_inode *ti, struct dentry *dentry) { struct tracefs_inode *ti_parent; struct eventfs_inode *ei; - struct eventfs_file *ef, *tmp; + struct eventfs_file *ef; /* The top level events directory may be freed by this */ if (unlikely(ti->flags & TRACEFS_EVENT_TOP_INODE)) { - LIST_HEAD(ef_del_list); - mutex_lock(&eventfs_mutex); - ei = ti->private; - /* Record all the top level files */ - list_for_each_entry_srcu(ef, &ei->e_top_files, list, - lockdep_is_held(&eventfs_mutex)) { - list_add_tail(&ef->del_list, &ef_del_list); - } - /* Nothing should access this, but just in case! */ ti->private = NULL; - mutex_unlock(&eventfs_mutex); - /* Now safely free the top level files and their children */ - list_for_each_entry_safe(ef, tmp, &ef_del_list, del_list) { - list_del(&ef->del_list); - eventfs_remove(ef); - } - - kfree(ei); + ef = dentry->d_fsdata; + if (ef) + free_ef(ef); return; } @@ -311,16 +305,13 @@ void eventfs_set_ef_status_free(struct tracefs_inode *ti, struct dentry *dentry) if (!ef) goto out; - /* - * If ef was freed, then the LSB bit is set for d_fsdata. - * But this should not happen, as it should still have a - * ref count that prevents it. Warn in case it does. - */ - if (WARN_ON_ONCE((unsigned long)ef & 1)) - goto out; + if (ef->is_freed) { + free_ef(ef); + } else { + ef->dentry = NULL; + } dentry->d_fsdata = NULL; - ef->dentry = NULL; out: mutex_unlock(&eventfs_mutex); } @@ -847,13 +838,53 @@ int eventfs_add_file(const char *name, umode_t mode, return 0; } -static void free_ef(struct rcu_head *head) +static LLIST_HEAD(free_list); + +static void eventfs_workfn(struct work_struct *work) +{ + struct eventfs_file *ef, *tmp; + struct llist_node *llnode; + + llnode = llist_del_all(&free_list); + llist_for_each_entry_safe(ef, tmp, llnode, llist) { + /* This should only get here if it had a dentry */ + if (!WARN_ON_ONCE(!ef->dentry)) + dput(ef->dentry); + } +} + +static DECLARE_WORK(eventfs_work, eventfs_workfn); + +static void free_rcu_ef(struct rcu_head *head) { struct eventfs_file *ef = container_of(head, struct eventfs_file, rcu); - kfree(ef->name); - kfree(ef->ei); - kfree(ef); + if (ef->dentry) { + /* Do not free the ef until all references of dentry are gone */ + if (llist_add(&ef->llist, &free_list)) + queue_work(system_unbound_wq, &eventfs_work); + return; + } + + free_ef(ef); +} + +static void unhook_dentry(struct dentry *dentry) +{ + if (!dentry) + return; + + /* Keep the dentry from being freed yet (see eventfs_workfn()) */ + dget(dentry); + + dentry->d_fsdata = NULL; + d_invalidate(dentry); + mutex_lock(&eventfs_mutex); + /* dentry should now have at least a single reference */ + WARN_ONCE((int)d_count(dentry) < 1, + "dentry %px (%s) less than one reference (%d) after invalidate\n", + dentry, dentry->d_name.name, d_count(dentry)); + mutex_unlock(&eventfs_mutex); } /** @@ -905,58 +936,25 @@ void eventfs_remove(struct eventfs_file *ef) { struct eventfs_file *tmp; LIST_HEAD(ef_del_list); - struct dentry *dentry_list = NULL; - struct dentry *dentry; if (!ef) return; + /* + * Move the deleted eventfs_inodes onto the ei_del_list + * which will also set the is_freed value. Note, this has to be + * done under the eventfs_mutex, but the deletions of + * the dentries must be done outside the eventfs_mutex. + * Hence moving them to this temporary list. + */ mutex_lock(&eventfs_mutex); eventfs_remove_rec(ef, &ef_del_list, 0); - list_for_each_entry_safe(ef, tmp, &ef_del_list, del_list) { - if (ef->dentry) { - unsigned long ptr = (unsigned long)dentry_list; - - /* Keep the dentry from being freed yet */ - dget(ef->dentry); - - /* - * Paranoid: The dget() above should prevent the dentry - * from being freed and calling eventfs_set_ef_status_free(). - * But just in case, set the link list LSB pointer to 1 - * and have eventfs_set_ef_status_free() check that to - * make sure that if it does happen, it will not think - * the d_fsdata is an event_file. - * - * For this to work, no event_file should be allocated - * on a odd space, as the ef should always be allocated - * to be at least word aligned. Check for that too. - */ - WARN_ON_ONCE(ptr & 1); - - ef->dentry->d_fsdata = (void *)(ptr | 1); - dentry_list = ef->dentry; - ef->dentry = NULL; - } - call_srcu(&eventfs_srcu, &ef->rcu, free_ef); - } mutex_unlock(&eventfs_mutex); - while (dentry_list) { - unsigned long ptr; - - dentry = dentry_list; - ptr = (unsigned long)dentry->d_fsdata & ~1UL; - dentry_list = (struct dentry *)ptr; - dentry->d_fsdata = NULL; - d_invalidate(dentry); - mutex_lock(&eventfs_mutex); - /* dentry should now have at least a single reference */ - WARN_ONCE((int)d_count(dentry) < 1, - "dentry %p less than one reference (%d) after invalidate\n", - dentry, d_count(dentry)); - mutex_unlock(&eventfs_mutex); - dput(dentry); + list_for_each_entry_safe(ef, tmp, &ef_del_list, del_list) { + unhook_dentry(ef->dentry); + list_del(&ef->del_list); + call_srcu(&eventfs_srcu, &ef->rcu, free_rcu_ef); } }