linux-kernel - Re: [PATCH -next 2/3] devtmpfs: add debug info to handle()

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]

Date:   Thu, 2 Feb 2023 09:30:30 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Longlong Xia <xialonglong1@...wei.com>
Cc:     rafael@...nel.org, linux-kernel@...r.kernel.org,
        chenwandun@...wei.com, wangkefeng.wang@...wei.com,
        sunnanyong@...wei.com
Subject: Re: [PATCH -next 2/3] devtmpfs: add debug info to handle()

On Thu, Feb 02, 2023 at 03:32:02AM +0000, Longlong Xia wrote:
> The devtmpfs_*_node() are used to mount/unmount devices to /dev, but their
> callers don't check their return value, so we don't know the reason for
> the failure. Let's add some debug info in handle() to help users know
> why failed.
> 
> Signed-off-by: Longlong Xia <xialonglong1@...wei.com>
> ---
>  drivers/base/devtmpfs.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index ae72d4ba8547..77ca64f708ce 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -389,10 +389,18 @@ static __initdata DECLARE_COMPLETION(setup_done);
>  static int handle(const char *name, umode_t mode, kuid_t uid, kgid_t gid,
>  		  struct device *dev)
>  {
> +	int ret;
> +
>  	if (mode)
> -		return handle_create(name, mode, uid, gid, dev);
> +		ret = handle_create(name, mode, uid, gid, dev);
>  	else
> -		return handle_remove(name, dev);
> +		ret = handle_remove(name, dev);
> +
> +	if (ret)
> +		pr_err_ratelimited("failed to %s %s, ret = %d\n",
> +				   mode ? "create" : "remove", name, ret);

As you have a struct device * here, why not use dev_err() instead?

And why rate limited?  What is going to cause this to be spammed with
lots and lots of failures?  What were you doing that caused a failure
here, how is it triggered?

thanks,

greg k-h