linux-kernel - Re: [PATCH] drm/arm/komeda: Add a condition check before removing sysfs attribute

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: <Z72bMsXmoQv1UVv0@bart.dudau.co.uk>
Date: Tue, 25 Feb 2025 10:28:02 +0000
From: Liviu Dudau <liviu@...au.co.uk>
To: oushixiong1025@....com
Cc: Liviu Dudau <liviu.dudau@....com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	Shixiong Ou <oushixiong@...inos.cn>
Subject: Re: [PATCH] drm/arm/komeda: Add a condition check before removing
 sysfs attribute

Hello,

Replying from my personal email as the corporate system seems to have blackholed your emails
while I was on holiday.

Can you tell me more why you think that if sysfs_create_group() fails we should not call
sysfs_remove_group()? After all, we don't know how far sysfs_create_group() has progressed before
it encountered an error, so we still need to do some cleanup.

Best regards,
Liviu

On Thu, Feb 20, 2025 at 04:53:58PM +0800, oushixiong1025@....com wrote:
> From: Shixiong Ou <oushixiong@...inos.cn>
> 
> [WHY] If the call to sysfs_create_group() fails, there is
>       no need to call function sysfs_remove_group().
> 
> [HOW] Add a condition check before removing sysfs attribute.
> 
> Signed-off-by: Shixiong Ou <oushixiong@...inos.cn>
> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 7 ++++++-
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 2 ++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 5ba62e637a61..7d646f978640 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -259,6 +259,8 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>  		goto err_cleanup;
>  	}
>  
> +	mdev->sysfs_attr_enabled = true;
> +
>  	mdev->err_verbosity = KOMEDA_DEV_PRINT_ERR_EVENTS;
>  
>  	komeda_debugfs_init(mdev);
> @@ -278,7 +280,10 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  	const struct komeda_dev_funcs *funcs = mdev->funcs;
>  	int i;
>  
> -	sysfs_remove_group(&dev->kobj, &komeda_sysfs_attr_group);
> +	if (mdev->sysfs_attr_enabled) {
> +		sysfs_remove_group(&dev->kobj, &komeda_sysfs_attr_group);
> +		mdev->sysfs_attr_enabled = false;
> +	}
>  
>  	debugfs_remove_recursive(mdev->debugfs_root);
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index 5b536f0cb548..af087540325c 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -216,6 +216,8 @@ struct komeda_dev {
>  #define KOMEDA_DEV_PRINT_DUMP_STATE_ON_EVENT BIT(8)
>  	/* Disable rate limiting of event prints (normally one per commit) */
>  #define KOMEDA_DEV_PRINT_DISABLE_RATELIMIT BIT(12)
> +
> +	bool sysfs_attr_enabled;
>  };
>  
>  static inline bool
> -- 
> 2.17.1
> 

-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!