Re: Bug in chardev checking of overlapping ranges(send again as TEXT/PLAIN)

[Omar Sandoval <osandov@osandov.com>]

On Wed, May 25, 2016 at 10:53:36PM +0800, Gavin Chang wrote:
> Hi,
> 
> I'm a new person to learn Linux kernel.
> 
> In fs/char_dev.c function __register_chrdev_region(), there is the
> following code:
> 
> for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
> if ((*cp)->major > major ||
>     ((*cp)->major == major &&
>      (((*cp)->baseminor >= baseminor) ||
>       ((*cp)->baseminor + (*cp)->minorct > baseminor))))
> break;
> 
> /* Check for overlapping minor ranges.  */
> if (*cp && (*cp)->major == major) {
> int old_min = (*cp)->baseminor;
> int old_max = (*cp)->baseminor + (*cp)->minorct - 1;
> int new_min = baseminor;
> int new_max = baseminor + minorct - 1;
> 
> /* New driver overlaps from the left.  */
> if (new_max >= old_min && new_max <= old_max) {
> ret = -EBUSY;
> goto out;
> }
> 
> /* New driver overlaps from the right.  */
> if (new_min <= old_max && new_min >= old_min) {
> ret = -EBUSY;
> goto out;
> }
> }
> 
> I think there is a bug in checking of overlapping ranges.
> For example, driver X has registered with major=x and minor=1-3, and
> __register_chrdev_region() will allow driver Y to register with
> major=x and minor=0-4.
> The minor of driver Y will not meet the two if statements:
> if (new_max >= old_min && new_max <= old_max)  and if (new_min <=
> old_max && new_min >= old_min)
> 
> The attached is my patch to repair this problem.
> And there is a related commit 01d553d0fe9f90a132c5ff494872be8d4126be1e
> for reference.

> diff --git a/fs/char_dev.c b/fs/char_dev.c
> index 24b1425..d65765e 100644
> --- a/fs/char_dev.c
> +++ b/fs/char_dev.c
> @@ -107,29 +107,14 @@ __register_chrdev_region(unsigned int major, unsigned int baseminor,
>  
>  	for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
>  		if ((*cp)->major > major ||
> -		    ((*cp)->major == major &&
> -		     (((*cp)->baseminor >= baseminor) ||
> -		      ((*cp)->baseminor + (*cp)->minorct > baseminor))))
> +		    ((*cp)->major == major && ((*cp)->baseminor + (*cp)->minorct > baseminor)))
>  			break;
>  
>  	/* Check for overlapping minor ranges.  */
> -	if (*cp && (*cp)->major == major) {
> -		int old_min = (*cp)->baseminor;
> -		int old_max = (*cp)->baseminor + (*cp)->minorct - 1;
> -		int new_min = baseminor;
> -		int new_max = baseminor + minorct - 1;
> -
> -		/* New driver overlaps from the left.  */
> -		if (new_max >= old_min && new_max <= old_max) {
> -			ret = -EBUSY;
> -			goto out;
> -		}
> -
> -		/* New driver overlaps from the right.  */
> -		if (new_min <= old_max && new_min >= old_min) {
> -			ret = -EBUSY;
> -			goto out;
> -		}
> +	if (*cp && (*cp)->major == major &&
> +	    (*cp)->baseminor < baseminor + minorct) {
> +               ret = -EBUSY;
> +               goto out;
>  	}
>  
>  	cd->next = *cp;

Gavin,

You're going to want to read Documentation/SubmittingPatches. Basically,
you want to use git commit -s, git format-patch, and git send-email
rather than attachments.

Thanks,
-- 
Omar