linux-kernel - Re: NULL pointer dereference regression when running `chmod -R root:tracing /sys/kernel/debug/tracing`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231112092626.5a93e49d@rorschach.local.home>
Date:   Sun, 12 Nov 2023 09:26:26 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Milian Wolff <milian.wolff@...b.com>
Cc:     akaher@...are.com, akpm@...ux-foundation.org,
        gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        mark.rutland@....com, mhiramat@...nel.org
Subject: Re: NULL pointer dereference regression when running `chmod -R
 root:tracing /sys/kernel/debug/tracing`

On Sun, 12 Nov 2023 07:14:39 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:
> > With 6.6.1 and 9aaee3eebc91dd9ccebf6b6bc8a5f59d04ef718b reverted,
> > the above `chmod` command works. With a normal 6.6.1 build, or re-applying
> > the patch again, the command fails and `dmesg` shows:  
> 
> Thanks for the report. I'll work on it on my way to Plumbers.

Can you test this patch?

Note, this code was rewritten for 6.7 so it probably doesn't affect
that tree, but I'm going to test to make sure, just in case.

Also, this shows I need to add a selftest to cover this case.

Thanks,

-- Steve

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 5fcfb634fec2..efbdc47c74dc 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -113,14 +113,14 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry,
 
 	mutex_lock(&eventfs_mutex);
 	ef = dentry->d_fsdata;
-	if (ef->is_freed) {
+	if (ef && ef->is_freed) {
 		/* Do not allow changes if the event is about to be removed. */
 		mutex_unlock(&eventfs_mutex);
 		return -ENODEV;
 	}
 
 	ret = simple_setattr(idmap, dentry, iattr);
-	if (!ret)
+	if (!ret && ef)
 		update_attr(ef, iattr);
 	mutex_unlock(&eventfs_mutex);
 	return ret;