From patchwork Sat Oct 28 20:37:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 159290 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:d641:0:b0:403:3b70:6f57 with SMTP id cy1csp1390151vqb; Sat, 28 Oct 2023 13:44:23 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEHcS3II2cZ/qQMLOuaF0GRcafzdEPWDFw7SGN8xIusRU4sAkOxtO7rYMxzY7ZrMQisGSSv X-Received: by 2002:a17:90b:d8a:b0:27d:882f:e6c5 with SMTP id bg10-20020a17090b0d8a00b0027d882fe6c5mr4701419pjb.9.1698525863178; Sat, 28 Oct 2023 13:44:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698525863; cv=none; d=google.com; s=arc-20160816; b=cNv3Q/W7WIN2OsHdLcnlg/1WzeWqCLe+TpnxwAiQDRBsBhQ2M81GsNBxJFpnTJ645b GBI79nzwRhzmvthU3KNO5XgRXRKiOUAWPDxRhkTCxs1UNht3/OFu0SjrM5e+5DqBRbtz OBSzID7LbC83iWNANSIK/nVr6xnRNgpu2G5cI/WHmXLs44LueocBWV5Q1aIzgjRsoWL4 WMic60BkGAra50j6Hc2j6zjh1NwTTxdMg/2wEgxH9t/fiipnOlsgzfP2Xl26C4tZAcCa xob0KscYvoY9D696UN8fJRmxQj3S+f+eLYiviAtw+EUjO+8urjYhnDwr5w5AAK5wdZB6 FAtg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:subject:cc:to:from:date; bh=vBAynCKH58jZAkeHzRYALwmopvcOcVyAPn4Lj8VvgzA=; fh=vPrK7tFDo2Tq6BQLdH3ZS/ADpCGv/htoxFI9aQnVmkc=; b=HARAz8G8bufkvYorQJJrqd3JvXiux3qPb4tKJvCyy2wtmR93nuCqCSCAVkVdIAPJyT iouWMYHhfSSZicA5VQ2MZtudMhoW5EonxlvUdLCrOzlAxc347h9ilyV11oFhf5txCn/n OaKWz7mZpGOqOB9RfbOc0ULqZVx4Vh6lFfYwfnTg07tQKrXPf0BzVjwgNZFziWMtAhlk aNGEeKmbC9neKIyLXfRi3fM1kyHNPku2vfq44tNSxzo00XFcnH7w3Xvl9HEoFyiJcd89 hCG9wPao23busNGrgX0sXRV1c2SNVDnrIabnim+Z8Uym/qhtLSDfy5+sYM9sBi/963cY 9qTw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id c9-20020a17090aa60900b0026b502223cbsi2730033pjq.10.2023.10.28.13.44.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 28 Oct 2023 13:44:23 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 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 fry.vger.email (Postfix) with ESMTP id 36F86808EF45; Sat, 28 Oct 2023 13:44:20 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229450AbjJ1Uh5 (ORCPT + 29 others); Sat, 28 Oct 2023 16:37:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41186 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjJ1Uhz (ORCPT ); Sat, 28 Oct 2023 16:37:55 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A31A0D3 for ; Sat, 28 Oct 2023 13:37:52 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F0F38C433C7; Sat, 28 Oct 2023 20:37:50 +0000 (UTC) Date: Sat, 28 Oct 2023 16:37:49 -0400 From: Steven Rostedt To: LKML , Linux trace kernel Cc: Masami Hiramatsu , Mark Rutland , "Arnd Bergmann" , "Naresh Kamboju" , Beau Belgrave , "Ajay Kaher" , Andrew Morton Subject: [PATCH] eventfs: Test for ei->is_freed when accessing ei->dentry Message-ID: <20231028163749.0d3429a1@rorschach.local.home> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) 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 autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.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 (fry.vger.email [0.0.0.0]); Sat, 28 Oct 2023 13:44:20 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781033455627762335 X-GMAIL-MSGID: 1781033455627762335 From: "Steven Rostedt (Google)" The eventfs_inode (ei) is protected by SRCU, but the ei->dentry is not. It is protected by the eventfs_mutex. Anytime the eventfs_mutex is released, and access to the ei->dentry needs to be done, it should first check if ei->is_freed is set under the eventfs_mutex. If it is, then the ei->dentry is invalid and must not be used. The ei->dentry must only be accessed under the eventfs_mutex and after checking if ei->is_freed is set. When the ei is being freed, it will (under the eventfs_mutex) set is_freed and at the same time move the dentry to a free list to be cleared after the eventfs_mutex is released. This means that any access to the ei->dentry must check first if ei->is_freed is set, because if it is, then the dentry is on its way to be freed. Also add comments to describe this better. Link: https://lore.kernel.org/all/CA+G9fYt6pY+tMZEOg=SoEywQOe19fGP3uR15SGowkdK+_X85Cg@mail.gmail.com/ Link: https://lore.kernel.org/all/CA+G9fYuDP3hVQ3t7FfrBAjd_WFVSurMgCepTxunSJf=MTe=6aA@mail.gmail.com/ Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use eventfs_inode") Reported-by: Reported-by: Linux Kernel Functional Testing Reported-by: Naresh Kamboju Reported-by: Beau Belgrave Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 60 ++++++++++++++++++++++++++++++++-------- fs/tracefs/internal.h | 3 +- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 4d2da7480e5f..385aab6dc982 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -24,7 +24,20 @@ #include #include "internal.h" +/* + * eventfs_mutex protects the eventfs_inode (ei) dentry. Any access + * to the ei->dentry must be done under this mutex and after checking + * if ei->is_freed is not set. The ei->dentry is released under the + * mutex at the same time ei->is_freed is set. If ei->is_freed is set + * then the ei->dentry is invalid. + */ static DEFINE_MUTEX(eventfs_mutex); + +/* + * The eventfs_inode (ei) itself is protected by SRCU. It is released from + * its parent's list and will have is_freed set (under eventfs_mutex). + * After the SRCU grace period is over, the ei may be freed. + */ DEFINE_STATIC_SRCU(eventfs_srcu); static struct dentry *eventfs_root_lookup(struct inode *dir, @@ -234,6 +247,10 @@ create_file_dentry(struct eventfs_inode *ei, struct dentry **e_dentry, bool invalidate = false; mutex_lock(&eventfs_mutex); + if (ei->is_freed) { + mutex_unlock(&eventfs_mutex); + return NULL; + } /* If the e_dentry already has a dentry, use it */ if (*e_dentry) { /* lookup does not need to up the ref count */ @@ -307,6 +324,8 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei) struct eventfs_inode *ei_child; struct tracefs_inode *ti; + lockdep_assert_held(&eventfs_mutex); + /* srcu lock already held */ /* fill parent-child relation */ list_for_each_entry_srcu(ei_child, &ei->children, list, @@ -320,6 +339,7 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei) /** * create_dir_dentry - Create a directory dentry for the eventfs_inode + * @pei: The eventfs_inode parent of ei. * @ei: The eventfs_inode to create the directory for * @parent: The dentry of the parent of this directory * @lookup: True if this is called by the lookup code @@ -327,12 +347,17 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei) * This creates and attaches a directory dentry to the eventfs_inode @ei. */ static struct dentry * -create_dir_dentry(struct eventfs_inode *ei, struct dentry *parent, bool lookup) +create_dir_dentry(struct eventfs_inode *pei, struct eventfs_inode *ei, + struct dentry *parent, bool lookup) { bool invalidate = false; struct dentry *dentry = NULL; mutex_lock(&eventfs_mutex); + if (pei->is_freed || ei->is_freed) { + mutex_unlock(&eventfs_mutex); + return NULL; + } if (ei->dentry) { /* If the dentry already has a dentry, use it */ dentry = ei->dentry; @@ -435,7 +460,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir, */ mutex_lock(&eventfs_mutex); ei = READ_ONCE(ti->private); - if (ei) + if (ei && !ei->is_freed) ei_dentry = READ_ONCE(ei->dentry); mutex_unlock(&eventfs_mutex); @@ -449,7 +474,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir, if (strcmp(ei_child->name, name) != 0) continue; ret = simple_lookup(dir, dentry, flags); - create_dir_dentry(ei_child, ei_dentry, true); + create_dir_dentry(ei, ei_child, ei_dentry, true); created = true; break; } @@ -583,7 +608,7 @@ static int dcache_dir_open_wrapper(struct inode *inode, struct file *file) list_for_each_entry_srcu(ei_child, &ei->children, list, srcu_read_lock_held(&eventfs_srcu)) { - d = create_dir_dentry(ei_child, parent, false); + d = create_dir_dentry(ei, ei_child, parent, false); if (d) { ret = add_dentries(&dentries, d, cnt); if (ret < 0) @@ -637,6 +662,13 @@ static int dcache_readdir_wrapper(struct file *file, struct dir_context *ctx) return ret; } +static void free_ei(struct eventfs_inode *ei) +{ + kfree_const(ei->name); + kfree(ei->d_children); + kfree(ei); +} + /** * eventfs_create_dir - Create the eventfs_inode for this directory * @name: The name of the directory to create. @@ -700,12 +732,20 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode ei->nr_entries = size; ei->data = data; INIT_LIST_HEAD(&ei->children); + INIT_LIST_HEAD(&ei->list); mutex_lock(&eventfs_mutex); - list_add_tail(&ei->list, &parent->children); - ei->d_parent = parent->dentry; + if (!parent->is_freed) { + list_add_tail(&ei->list, &parent->children); + ei->d_parent = parent->dentry; + } mutex_unlock(&eventfs_mutex); + /* Was the parent freed? */ + if (list_empty(&ei->list)) { + free_ei(ei); + ei = NULL; + } return ei; } @@ -787,13 +827,11 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry return ERR_PTR(-ENOMEM); } -static void free_ei(struct rcu_head *head) +static void free_rcu_ei(struct rcu_head *head) { struct eventfs_inode *ei = container_of(head, struct eventfs_inode, rcu); - kfree_const(ei->name); - kfree(ei->d_children); - kfree(ei); + free_ei(ei); } /** @@ -880,7 +918,7 @@ void eventfs_remove_dir(struct eventfs_inode *ei) for (i = 0; i < ei->nr_entries; i++) unhook_dentry(&ei->d_children[i], &dentry_list); unhook_dentry(&ei->dentry, &dentry_list); - call_srcu(&eventfs_srcu, &ei->rcu, free_ei); + call_srcu(&eventfs_srcu, &ei->rcu, free_rcu_ei); } mutex_unlock(&eventfs_mutex); diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 64fde9490f52..21a1fa682b74 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -30,7 +30,7 @@ struct eventfs_inode { const struct eventfs_entry *entries; const char *name; struct list_head children; - struct dentry *dentry; + struct dentry *dentry; /* Check is_freed to access */ struct dentry *d_parent; struct dentry **d_children; void *data; @@ -39,6 +39,7 @@ struct eventfs_inode { * @del_list: list of eventfs_inode to delete * @rcu: eventfs_inode to delete in RCU * @is_freed: node is freed if one of the above is set + * Note if is_freed is set, then dentry is corrupted. */ union { struct list_head del_list;