linux-kernel - Re: [PATCH] char: misc: Make the code for allocating minor in misc

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: <2025100723-ravishing-widget-07b0@gregkh>
Date: Tue, 7 Oct 2025 14:45:16 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: zhangjiao2 <zhangjiao2@...s.chinamobile.com>
Cc: arnd@...db.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] char: misc: Make the code for allocating minor in
 misc_register more concise

On Wed, Sep 10, 2025 at 09:44:20AM +0800, zhangjiao2 wrote:
> >> On Tue, Sep 09, 2025 at 04:58:35PM +0800, zhangjiao2 wrote:
> >> From: zhang jiao <zhangjiao2@...s.chinamobile.com>
> >>
> >> There is no need to check the registered misc dev in misc_list.
> >> If misc_minor_alloc failed, it meens the minor is already alloced
> >> and the misc dev is linked in misc_list.
> >>
> >> Signed-off-by: zhang jiao <zhangjiao2@...s.chinamobile.com>
> >> ---
> >>  drivers/char/misc.c | 32 +++++++-------------------------
> >>  1 file changed, 7 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> >> index a0aae0fc7926..fc2f5e8b2f95 100644
> >> --- a/drivers/char/misc.c
> >> +++ b/drivers/char/misc.c
> >> @@ -211,6 +211,7 @@ int misc_register(struct miscdevice *misc)
> >>  	dev_t dev;
> >>  	int err = 0;
> >>  	bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
> >> +	int minor = 0;
> >>
> >>  	if (misc->minor > MISC_DYNAMIC_MINOR) {
> >>  		pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
> >> @@ -221,32 +222,13 @@ int misc_register(struct miscdevice *misc)
> >>  	INIT_LIST_HEAD(&misc->list);
> >>
> >>  	mutex_lock(&misc_mtx);
> >> -
> >> -	if (is_dynamic) {
> >> -		int i = misc_minor_alloc(misc->minor);
> >> -
> >> -		if (i < 0) {
> >> -			err = -EBUSY;
> >> -			goto out;
> >> -		}
> >> -		misc->minor = i;
> >> -	} else {
> >> -		struct miscdevice *c;
> >> -		int i;
> >> -
> >> -		list_for_each_entry(c, &misc_list, list) {
> >> -			if (c->minor == misc->minor) {
> >> -				err = -EBUSY;
> >> -				goto out;
> >> -			}
> >> -		}
> >> -
> >> -		i = misc_minor_alloc(misc->minor);
> >> -		if (i < 0) {
> >> -			err = -EBUSY;
> >> -			goto out;
> >> -		}
> >> +	minor = misc_minor_alloc(misc->minor);
> >> +	if (minor < 0) {
> >> +		err = -EBUSY;
> >> +		goto out;
> >>  	}
> >> +	if (is_dynamic)
> >> +		misc->minor = minor;
> >>
> >>  	dev = MKDEV(MISC_MAJOR, misc->minor);
> >>
> 
> > Does this pass the new test suite for the misc code allocation logic
> > that we now have in the tree?  Or do we need to write a new test-case
> > for this codepath?
> 
>  I didn't take this new test suite, can you do it for me?

Please test your changes first, don't make maintainers do it for you :)

> I don't think there's 
>  a need to write a new test-case for this codepath. 

Why not?  It seems to be a code path that is worth changing, so why not
test to verify you got it correct?

thanks,

greg k-h