All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Yichong Chen <chenyichong@uniontech.com>
Cc: dakr@kernel.org, djakov@kernel.org, driver-core@lists.linux.dev,
	linux-kernel@vger.kernel.org, quic_mdtipton@quicinc.com,
	rafael@kernel.org
Subject: Re: [PATCH] debugfs: serialize debugfs_create_str() writers
Date: Wed, 5 Aug 2026 10:00:05 +0200	[thread overview]
Message-ID: <2026080522-viper-movie-1c35@gregkh> (raw)
In-Reply-To: <20260804055056.877823-1-chenyichong@uniontech.com>

On Tue, Aug 04, 2026 at 01:50:56PM +0800, Yichong Chen wrote:
> Hi Greg,
> 
> I tried the direction you suggested and converted the SoundWire
> firmware_file debugfs entry away from debugfs_create_str().
> 
> The draft diff below does two things:
> 
>   1. debugfs_create_str() becomes read-only only.  It drops write
>      permission bits from the requested mode.  If the caller passed only
>      write bits, it creates an owner-readable file instead of a 0000 file.

If the caller passes write bits, the function should fail to create the
file at all, to make it easy to determine what just went wrong :)

>   2. drivers/soundwire/debugfs.c uses debugfs_create_file() with a small
>      local write-only file operation for firmware_file.

Great!

> The SoundWire command path copies firmware_file under a mutex before using
> it for request_firmware(), so a later debugfs write can replace the global
> string without invalidating the name being used by the command.

As this is debugging only, and root only, and loading firmware files
which better only be done by a developer on their own, this should be
fine and not really an issue at all (i.e. if it breaks, they get to keep
the pieces of their broken system...)

> I tested the generic debugfs_create_str() mode handling with a small test
> module:
> 
>   0444 -> 0444, readable, write fails
>   0600 -> 0400, readable, write fails
>   0200 -> 0400, readable, write fails
> 
> I do not have SoundWire hardware in my test VM, so I could only build that
> part.
> 
> Does this match the direction you had in mind?  If so, I can finish the
> conversion for the interconnect writable string users as well and send a
> proper v2.

Sure, but it should be a patch series, first convert the existing write
string usages to local copies, and then change the debugfs code to
refuse to handle writing strings.

thanks,

greg k-h

      reply	other threads:[~2026-08-05  8:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-03  6:09 [PATCH] debugfs: serialize debugfs_create_str() writers Yichong Chen
2026-08-03  6:19 ` Greg KH
2026-08-03  8:34   ` Yichong Chen
2026-08-03  8:43     ` Greg KH
2026-08-04  5:50       ` Yichong Chen
2026-08-05  8:00         ` Greg KH [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026080522-viper-movie-1c35@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=chenyichong@uniontech.com \
    --cc=dakr@kernel.org \
    --cc=djakov@kernel.org \
    --cc=driver-core@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quic_mdtipton@quicinc.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.