linux-kernel - Re: [PATCH v4 5/8] char: misc: Make registering miscdevice reentry who wants dynamic minor

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]

Message-ID: <2025070658-brought-shakable-935e@gregkh>
Date: Sun, 6 Jul 2025 10:55:03 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: Arnd Bergmann <arnd@...db.de>,
	"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
	Helge Deller <deller@....de>,
	"David S. Miller" <davem@...emloft.net>,
	Andreas Larsson <andreas@...sler.com>,
	Thadeu Lima de Souza Cascardo <cascardo@...lia.com>,
	linux-kernel@...r.kernel.org, linux-parisc@...r.kernel.org,
	sparclinux@...r.kernel.org, Zijun Hu <zijun.hu@....qualcomm.com>
Subject: Re: [PATCH v4 5/8] char: misc: Make registering miscdevice reentry
 who wants dynamic minor

The subject does not make much sense, can you please reword it?

On Fri, Jul 04, 2025 at 09:26:03PM +0800, Zijun Hu wrote:
> From: Zijun Hu <zijun.hu@....qualcomm.com>
> 
> misc_deregister() frees dynamic minor @misc->minor but does not reset it
> and cause kunit test case miscdev_test_dynamic_reentry() failure:
> 
> | Invalid fixed minor 257 for miscdevice 'miscdyn_a'
> | #miscdev_test_dynamic_reentry: ASSERTION FAILED at misc_minor_kunit.c:639
> | Expected ret == 0, but
> | ret == -22 (0xffffffffffffffea)
> | [FAILED] miscdev_test_dynamic_reentry
> 
> misc_register()/misc_deregister() are sometimes invoked by driver's
> probe()/remove() separately, which has reentry requirement.

What do you mean?  Why is it required that this is reentrant?  What
in-kernel drivers require this?

> Fix by resetting @misc->minor to MISC_DYNAMIC_MINOR in misc_deregister()
> as error handling of misc_register() does.
> 
> Signed-off-by: Zijun Hu <zijun.hu@....qualcomm.com>
> ---
>  drivers/char/misc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index b8e66466184fa21fb66d968db7950e0b5669ac43..96ed343cf5c8509a09855020049a9af82a3ede95 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -288,6 +288,8 @@ void misc_deregister(struct miscdevice *misc)
>  	list_del(&misc->list);
>  	device_destroy(&misc_class, MKDEV(MISC_MAJOR, misc->minor));
>  	misc_minor_free(misc->minor);
> +	if (misc->minor > MISC_DYNAMIC_MINOR)
> +		misc->minor = MISC_DYNAMIC_MINOR;
>  	mutex_unlock(&misc_mtx);

misc is being unregistered here, so why are you changing the minor
field?  It's now invalid as it is not registered, so this value should
never be relied on at all, neither is anything else in this structure.

thanks,

greg k-h