linux-kernel - Re: [PATCH v2] drdb: fix debugfs_create_dir and debugfs_create

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: <2ec28ec3-1e22-e772-fc9a-d59c6d176b7d@linbit.com>
Date:   Mon, 15 May 2023 10:56:20 +0200
From:   Christoph Böhmwalder 
        <christoph.boehmwalder@...bit.com>
To:     mirimmad@...look.com
Cc:     axboe@...nel.dk, drbd-dev@...ts.linbit.com,
        lars.ellenberg@...bit.com, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, mirimmad17@...il.com,
        philipp.reisner@...bit.com, skhan@...uxfoundation.org
Subject: Re: [PATCH v2] drdb: fix debugfs_create_dir and
 debugfs_create_symlink error handling

Am 14.05.23 um 16:10 schrieb mirimmad@...look.com:
> From: Immad Mir <mirimmad17@...il.com>
> 
> debugfs_create_dir and debugfs_create_symlink return ERR_PTR incase of
> a failure which must be checked with the inline function IS_ERR. This
> patch attempts to do the same.
> 
> Signed-off-by: Immad Mir <mirimmad17@...il.com>
> ---
> Changes in v2:
>     - Fix indentation
>     - Fix potential memory leak
> 
>  drivers/block/drbd/drbd_debugfs.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c
> index 12460b584..ab431a3f2 100644
> --- a/drivers/block/drbd/drbd_debugfs.c
> +++ b/drivers/block/drbd/drbd_debugfs.c
> @@ -781,6 +781,7 @@ void drbd_debugfs_device_add(struct drbd_device *device)
> 
>  	snprintf(vnr_buf, sizeof(vnr_buf), "%u", device->vnr);
>  	dentry = debugfs_create_dir(vnr_buf, vols_dir);
> +    if (IS_ERR(dentry)) goto fail;
>  	device->debugfs_vol = dentry;
> 
>  	snprintf(minor_buf, sizeof(minor_buf), "%u", device->minor);
> @@ -789,9 +790,15 @@ void drbd_debugfs_device_add(struct drbd_device *device)
>  	if (!slink_name)
>  		goto fail;
>  	dentry = debugfs_create_symlink(minor_buf, drbd_debugfs_minors, slink_name);
> -	device->debugfs_minor = dentry;
> -	kfree(slink_name);
> -	slink_name = NULL;
> +    if (!IS_ERR(dentry)) {
> +        device->debugfs_minor = dentry;
> +        kfree(slink_name);
> +        slink_name = NULL;
> +    } else {
> +        kfree(slink_name);
> +        slink_name = NULL;
> +        goto fail;
> +    }
> 
>  #define DCF(name)	do {					\
>  	dentry = debugfs_create_file(#name, 0440,	\
> --
> 2.40.0
> 
> 

Hi, thanks for the patch.

Please see this commit:

commit d27e84a305980ac61df0a6841059d0eb09b8283d
Author: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Date:   Tue Jun 18 17:45:49 2019 +0200

    block: drbd: no need to check return value of debugfs_create functions

    When calling debugfs functions, there is no need to ever check the
    return value.  The function can work or not, but the code logic should
    never do something different based on this.

Also, it still looks like that whitespace is garbled.

NAK.

-- 
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage