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: <961b53a1-9c30-48ad-becf-81590b27464b@icloud.com>
Date: Wed, 9 Jul 2025 20:41:36 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
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

On 2025/7/6 16:55, Greg Kroah-Hartman wrote:
>> | 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?
> 

miscdevice APIs are sometimes used by the following way, "drv_probe()->
drv_remove()->drv_probe()" is possible, so "misc_register()->
misc_deregister()->misc_register()" is possible and considered by
previous patch's test case, which needs to register misc_dev again
without any reinitialization, namely, reentry.

actually, several in-kernel codes have such usages.

static struct miscdevice misc_dev = {
	...
		.minor = MISC_DYNAMIC_MINOR,
	...
};

int drv_probe() {
...
    // static misc_dev is not initialized before registering.
	misc_register(&misc_dev);
...
}

void drv_remove() {
	...
	misc_deregister(&misc_dev);
	...
}

struct device_driver driver = {
	.probe = drv_probe,
	.remove = drv_remove,
};

>> 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.

reset its minor code is to register it again without re-initialization
successfully

the other members are handed by misc_deregister() properly and don't
effect re-registering.