linux-kernel - Re: Re: [PATCH] debugfs: Fix NULL pointer dereference at debugfs_read_file

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] [day] [month] [year] [list]

Message-ID: <2025122221-gag-malt-75ba@gregkh>
Date: Mon, 22 Dec 2025 15:11:38 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: yangshiguang <yangshiguang1011@....com>
Cc: rafael@...nel.org, dakr@...nel.org, peterz@...radead.org,
	linux-kernel@...r.kernel.org, yangshiguang@...omi.com,
	stable@...r.kernel.org
Subject: Re: Re: [PATCH] debugfs: Fix NULL pointer dereference at
 debugfs_read_file_str

On Mon, Dec 22, 2025 at 08:41:33PM +0800, yangshiguang wrote:
> 
> At 2025-12-22 19:54:22, "Greg KH" <gregkh@...uxfoundation.org> wrote:
> >On Mon, Dec 22, 2025 at 05:36:16PM +0800, yangshiguang1011@....com wrote:
> >> From: yangshiguang <yangshiguang@...omi.com>
> >> 
> >> Check in debugfs_read_file_str() if the string pointer is NULL.
> >> 
> >> When creating a node using debugfs_create_str(), the string parameter
> >> value can be NULL to indicate empty/unused/ignored.
> >
> >Why would you create an empty debugfs string file?  That is not ok, we
> >should change that to not allow this.
> 
> Hi greg k-h,
> 
> Thanks for your reply.
> 
> This is due to the usage step, should write first and then read.
> However, there is no way to guarantee that everyone will know about this step.

True.

> And debugfs_create_str() allows passing in a NULL string. 

Then we should fix that :)

> Therefore, when reading a NULL string, should return an invalid error 
> instead of panic.

If you call write on a NULL string, then you could call strlen() of that
NULL string, and do a memcpy out of that NULL string.  All not good
things, so your quick fix here really doesn't solve the root problem :(

> >>  	str = *(char **)file->private_data;
> >> +	if (!str)
> >> +		return -EINVAL;
> >
> >What in kernel user causes this to happen?  Let's fix that up instead
> >please.
> >
> 
> Currently I known problematic nodes in the kernel:
> 
> drivers/interconnect/debugfs-client.c:
>   155: 	debugfs_create_str("src_node", 0600, client_dir, &src_node);
>   156: 	debugfs_create_str("dst_node", 0600, client_dir, &dst_node);

Ick, ok, that should be fixed.

> drivers/soundwire/debugfs.c:
>   362: 	debugfs_create_str("firmware_file", 0200, d, &firmware_file);

That too should be fixed, all should just create an "empty" string to
start with.

> test case:
> 1. create a NULL string node
> char *test_node = NULL;
> debugfs_create_str("test_node", 0600, parent_dir, &test_node);
> 
> 2. read the node, like bellow:
> cat /sys/kernel/debug/test_node

With your patch, you could change step 2 to do a write, and still cause
a crash :)

So let's fix this properly, let's just fail the creation of NULL
strings, and fix up all callers.

thanks,

greg k-h