public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: rafael@kernel.org, linux-kernel@vger.kernel.org,
	patches@opensource.cirrus.com
Subject: Re: [PATCH 5/5] debugfs: Add debugfs_create_const_str()
Date: Tue, 16 May 2023 18:37:34 +0200	[thread overview]
Message-ID: <2023051614-dingo-quicksand-ad6d@gregkh> (raw)
In-Reply-To: <20230516160753.32317-6-rf@opensource.cirrus.com>

On Tue, May 16, 2023 at 05:07:53PM +0100, Richard Fitzgerald wrote:
> Add a wrapper for debugfs_create_str() that takes a const char **.
> 
> It's never nice to have to cast a const pointer to a non-const to be
> able to pass it to an API. It always looks suspicious and it is relying
> on "knowing" that it's safe. A function that explicitly takes a const
> pointer is creating a contract that a const pointer is safe.
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  include/linux/debugfs.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index ea2d919fd9c7..2723690aedd1 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -401,4 +401,31 @@ static inline void debugfs_create_xul(const char *name, umode_t mode,
>  		debugfs_create_x64(name, mode, parent, (u64 *)value);
>  }
>  
> +/**
> + * debugfs_create_const_str - create a debugfs file that is used to read a string value
> + * @name: a pointer to a string containing the name of the file to create.
> + * @mode: the permission that the file should have
> + * @parent: a pointer to the parent dentry for this file.  This should be a
> + *          directory dentry if set.  If this parameter is %NULL, then the
> + *          file will be created in the root of the debugfs filesystem.
> + * @value: a pointer to the variable that the file should read from.
> + *         The const char* pointer must not change, except from NULL to
> + *         non-NULL.
> + *
> + * This function creates a file in debugfs with the given name that
> + * contains the value of the variable @value.
> + *
> + * The const char* pointed to by @value must not change after calling this
> + * function EXCEPT that it may change from NULL to non-NULL. This is to
> + * prevent the file read from accessing a stale pointer. A change from
> + * NULL to non-NULL is the only safe change, because the read will
> + * instantaneously see either NULL or the valid pointer.
> + */
> +static inline void debugfs_create_const_str(const char *name, umode_t mode,
> +					    struct dentry *parent,
> +					    const char **value)
> +{
> +	debugfs_create_str(name, mode & ~0222, parent, (char **)value);
> +}

Also, we need a user of the new function in order to be able to add it,
otherwise I'll just delete it eventually :)

thanks,

greg k-h

  parent reply	other threads:[~2023-05-16 16:38 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-16 16:07 [PATCH 0/5] debugfs: Fixes and improvements to debugfs_create_str() Richard Fitzgerald
2023-05-16 16:07 ` [PATCH 1/5] debugfs: Prevent NULL dereference reading from string property Richard Fitzgerald
2023-05-16 16:33   ` Greg KH
2023-05-16 17:29     ` Richard Fitzgerald
2023-05-16 17:43       ` Greg KH
2023-05-16 18:04         ` Richard Fitzgerald
2023-05-17  6:19           ` Greg KH
2023-05-16 17:14   ` Richard Fitzgerald
2023-05-16 16:07 ` [PATCH 2/5] debugfs: Remove kerneldoc that says debugfs_create_str() returns a value Richard Fitzgerald
2023-05-16 16:07 ` [PATCH 3/5] debugfs: Update debugfs_create_str() kerneldoc to warn about pointer race Richard Fitzgerald
2023-05-16 16:35   ` Greg KH
2023-05-16 17:50     ` Richard Fitzgerald
2023-05-17  6:18       ` Greg KH
2023-05-16 16:07 ` [PATCH 4/5] debugfs: Move debugfs_create_str() export to correct location Richard Fitzgerald
2023-05-16 16:07 ` [PATCH 5/5] debugfs: Add debugfs_create_const_str() Richard Fitzgerald
2023-05-16 16:37   ` Greg KH
2023-05-16 16:37   ` Greg KH [this message]
2023-05-16 16:38   ` Greg KH

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=2023051614-dingo-quicksand-ad6d@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=rafael@kernel.org \
    --cc=rf@opensource.cirrus.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox