All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
@ 2024-10-14 13:14 Vimal Agrawal
  2024-10-14 13:19 ` Greg KH
  2024-10-16 14:22 ` Dan Carpenter
  0 siblings, 2 replies; 5+ messages in thread
From: Vimal Agrawal @ 2024-10-14 13:14 UTC (permalink / raw)
  To: linux-kernel, gregkh, arnd; +Cc: avimalin, vimal.agrawal

misc_minor_alloc was allocating id for minor only in case of
MISC_DYNAMIC_MINOR but misc_minor_free was always freeing ids
using ida_free causing a mismatch and following
warn:
> > WARNING: CPU: 0 PID: 159 at lib/idr.c:525 ida_free+0x3e0/0x41f
> > ida_free called for id=127 which is not allocated.
> > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
...
> > [<60941eb4>] ida_free+0x3e0/0x41f
> > [<605ac993>] misc_minor_free+0x3e/0xbc
> > [<605acb82>] misc_deregister+0x171/0x1b3

misc_minor_alloc is changed to allocate id from ida for all dynamic/
misc dynamic minors

Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com>
---
 drivers/char/misc.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 541edc26ec89..fbe51e776c15 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -63,16 +63,28 @@ static DEFINE_MUTEX(misc_mtx);
 #define DYNAMIC_MINORS 128 /* like dynamic majors */
 static DEFINE_IDA(misc_minors_ida);
 
-static int misc_minor_alloc(void)
+static int misc_minor_alloc(int minor)
 {
 	int ret;
 
-	ret = ida_alloc_max(&misc_minors_ida, DYNAMIC_MINORS - 1, GFP_KERNEL);
-	if (ret >= 0) {
-		ret = DYNAMIC_MINORS - ret - 1;
-	} else {
-		ret = ida_alloc_range(&misc_minors_ida, MISC_DYNAMIC_MINOR + 1,
+	if (minor == MISC_DYNAMIC_MINOR) {
+		/* allocate free id */
+		ret = ida_alloc_max(&misc_minors_ida, DYNAMIC_MINORS - 1, GFP_KERNEL);
+		if (ret >= 0) {
+			ret = DYNAMIC_MINORS - ret - 1;
+		} else {
+			ret = ida_alloc_range(&misc_minors_ida, MISC_DYNAMIC_MINOR + 1,
 				      MINORMASK, GFP_KERNEL);
+		}
+	} else {
+		/* specific minor, check if it is in dynamic or misc dynamic range  */
+		if (minor < DYNAMIC_MINORS) {
+			minor = DYNAMIC_MINORS - minor - 1;
+			ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
+		}
+
+		if (minor > MISC_DYNAMIC_MINOR)
+			ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
 	}
 	return ret;
 }
@@ -219,7 +231,7 @@ int misc_register(struct miscdevice *misc)
 	mutex_lock(&misc_mtx);
 
 	if (is_dynamic) {
-		int i = misc_minor_alloc();
+		int i = misc_minor_alloc(misc->minor);
 
 		if (i < 0) {
 			err = -EBUSY;
@@ -228,6 +240,7 @@ int misc_register(struct miscdevice *misc)
 		misc->minor = i;
 	} else {
 		struct miscdevice *c;
+		int i;
 
 		list_for_each_entry(c, &misc_list, list) {
 			if (c->minor == misc->minor) {
@@ -235,6 +248,12 @@ int misc_register(struct miscdevice *misc)
 				goto out;
 			}
 		}
+
+		i = misc_minor_alloc(misc->minor);
+		if (i < 0) {
+			err = -EBUSY;
+			goto out;
+		}
 	}
 
 	dev = MKDEV(MISC_MAJOR, misc->minor);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
  2024-10-14 13:14 Vimal Agrawal
@ 2024-10-14 13:19 ` Greg KH
  2024-10-16 14:22 ` Dan Carpenter
  1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2024-10-14 13:19 UTC (permalink / raw)
  To: Vimal Agrawal; +Cc: linux-kernel, arnd, vimal.agrawal

On Mon, Oct 14, 2024 at 01:14:16PM +0000, Vimal Agrawal wrote:
> misc_minor_alloc was allocating id for minor only in case of
> MISC_DYNAMIC_MINOR but misc_minor_free was always freeing ids
> using ida_free causing a mismatch and following
> warn:
> > > WARNING: CPU: 0 PID: 159 at lib/idr.c:525 ida_free+0x3e0/0x41f
> > > ida_free called for id=127 which is not allocated.
> > > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> ...
> > > [<60941eb4>] ida_free+0x3e0/0x41f
> > > [<605ac993>] misc_minor_free+0x3e/0xbc
> > > [<605acb82>] misc_deregister+0x171/0x1b3
> 
> misc_minor_alloc is changed to allocate id from ida for all dynamic/
> misc dynamic minors
> 
> Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com>

What commit id does this fix?

And I think we need a test for this somewhere, care to write a kunit
test?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
@ 2024-10-16 10:28 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-10-16 10:28 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20241014131416.27324-1-vimal.agrawal@sophos.com>
References: <20241014131416.27324-1-vimal.agrawal@sophos.com>
TO: Vimal Agrawal <avimalin@gmail.com>
TO: linux-kernel@vger.kernel.org
TO: gregkh@linuxfoundation.org
TO: arnd@arndb.de
CC: avimalin@gmail.com
CC: vimal.agrawal@sophos.com

Hi Vimal,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.12-rc3 next-20241016]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vimal-Agrawal/misc-misc_minor_alloc-to-allocate-ids-for-all-dynamic-misc-dynamic-minors/20241014-211915
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241014131416.27324-1-vimal.agrawal%40sophos.com
patch subject: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: sparc64-randconfig-r072-20241015 (https://download.01.org/0day-ci/archive/20241016/202410161811.aIPEJHOt-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202410161811.aIPEJHOt-lkp@intel.com/

smatch warnings:
drivers/char/misc.c:89 misc_minor_alloc() error: uninitialized symbol 'ret'.

vim +/ret +89 drivers/char/misc.c

ab760791c0cfbb D Scott Phillips 2022-11-14  65  
d52c84545e305b Vimal Agrawal    2024-10-14  66  static int misc_minor_alloc(int minor)
ab760791c0cfbb D Scott Phillips 2022-11-14  67  {
ab760791c0cfbb D Scott Phillips 2022-11-14  68  	int ret;
ab760791c0cfbb D Scott Phillips 2022-11-14  69  
d52c84545e305b Vimal Agrawal    2024-10-14  70  	if (minor == MISC_DYNAMIC_MINOR) {
d52c84545e305b Vimal Agrawal    2024-10-14  71  		/* allocate free id */
ab760791c0cfbb D Scott Phillips 2022-11-14  72  		ret = ida_alloc_max(&misc_minors_ida, DYNAMIC_MINORS - 1, GFP_KERNEL);
ab760791c0cfbb D Scott Phillips 2022-11-14  73  		if (ret >= 0) {
ab760791c0cfbb D Scott Phillips 2022-11-14  74  			ret = DYNAMIC_MINORS - ret - 1;
ab760791c0cfbb D Scott Phillips 2022-11-14  75  		} else {
ab760791c0cfbb D Scott Phillips 2022-11-14  76  			ret = ida_alloc_range(&misc_minors_ida, MISC_DYNAMIC_MINOR + 1,
ab760791c0cfbb D Scott Phillips 2022-11-14  77  				      MINORMASK, GFP_KERNEL);
ab760791c0cfbb D Scott Phillips 2022-11-14  78  		}
d52c84545e305b Vimal Agrawal    2024-10-14  79  	} else {
d52c84545e305b Vimal Agrawal    2024-10-14  80  		/* specific minor, check if it is in dynamic or misc dynamic range  */
d52c84545e305b Vimal Agrawal    2024-10-14  81  		if (minor < DYNAMIC_MINORS) {
d52c84545e305b Vimal Agrawal    2024-10-14  82  			minor = DYNAMIC_MINORS - minor - 1;
d52c84545e305b Vimal Agrawal    2024-10-14  83  			ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
d52c84545e305b Vimal Agrawal    2024-10-14  84  		}
d52c84545e305b Vimal Agrawal    2024-10-14  85  
d52c84545e305b Vimal Agrawal    2024-10-14  86  		if (minor > MISC_DYNAMIC_MINOR)
d52c84545e305b Vimal Agrawal    2024-10-14  87  			ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
d52c84545e305b Vimal Agrawal    2024-10-14  88  	}
ab760791c0cfbb D Scott Phillips 2022-11-14 @89  	return ret;
ab760791c0cfbb D Scott Phillips 2022-11-14  90  }
ab760791c0cfbb D Scott Phillips 2022-11-14  91  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
  2024-10-14 13:14 Vimal Agrawal
  2024-10-14 13:19 ` Greg KH
@ 2024-10-16 14:22 ` Dan Carpenter
  2024-10-17 11:08   ` Vimal Agrawal
  1 sibling, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2024-10-16 14:22 UTC (permalink / raw)
  To: oe-kbuild, Vimal Agrawal, linux-kernel, gregkh, arnd
  Cc: lkp, oe-kbuild-all, avimalin, vimal.agrawal

Hi Vimal,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vimal-Agrawal/misc-misc_minor_alloc-to-allocate-ids-for-all-dynamic-misc-dynamic-minors/20241014-211915
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241014131416.27324-1-vimal.agrawal%40sophos.com
patch subject: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
config: sparc64-randconfig-r072-20241015 (https://download.01.org/0day-ci/archive/20241016/202410161811.aIPEJHOt-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202410161811.aIPEJHOt-lkp@intel.com/

smatch warnings:
drivers/char/misc.c:89 misc_minor_alloc() error: uninitialized symbol 'ret'.

vim +/ret +89 drivers/char/misc.c

d52c84545e305b Vimal Agrawal    2024-10-14  66  static int misc_minor_alloc(int minor)
ab760791c0cfbb D Scott Phillips 2022-11-14  67  {
ab760791c0cfbb D Scott Phillips 2022-11-14  68  	int ret;
ab760791c0cfbb D Scott Phillips 2022-11-14  69  
d52c84545e305b Vimal Agrawal    2024-10-14  70  	if (minor == MISC_DYNAMIC_MINOR) {
d52c84545e305b Vimal Agrawal    2024-10-14  71  		/* allocate free id */
ab760791c0cfbb D Scott Phillips 2022-11-14  72  		ret = ida_alloc_max(&misc_minors_ida, DYNAMIC_MINORS - 1, GFP_KERNEL);
ab760791c0cfbb D Scott Phillips 2022-11-14  73  		if (ret >= 0) {
ab760791c0cfbb D Scott Phillips 2022-11-14  74  			ret = DYNAMIC_MINORS - ret - 1;
ab760791c0cfbb D Scott Phillips 2022-11-14  75  		} else {
ab760791c0cfbb D Scott Phillips 2022-11-14  76  			ret = ida_alloc_range(&misc_minors_ida, MISC_DYNAMIC_MINOR + 1,
ab760791c0cfbb D Scott Phillips 2022-11-14  77  				      MINORMASK, GFP_KERNEL);
ab760791c0cfbb D Scott Phillips 2022-11-14  78  		}
d52c84545e305b Vimal Agrawal    2024-10-14  79  	} else {
d52c84545e305b Vimal Agrawal    2024-10-14  80  		/* specific minor, check if it is in dynamic or misc dynamic range  */
d52c84545e305b Vimal Agrawal    2024-10-14  81  		if (minor < DYNAMIC_MINORS) {
d52c84545e305b Vimal Agrawal    2024-10-14  82  			minor = DYNAMIC_MINORS - minor - 1;
d52c84545e305b Vimal Agrawal    2024-10-14  83  			ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
d52c84545e305b Vimal Agrawal    2024-10-14  84  		}
d52c84545e305b Vimal Agrawal    2024-10-14  85  
d52c84545e305b Vimal Agrawal    2024-10-14  86  		if (minor > MISC_DYNAMIC_MINOR)
d52c84545e305b Vimal Agrawal    2024-10-14  87  			ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);

What about if minor is >= DYNAMIC_MINORS (128) but <= MISC_DYNAMIC_MINOR (255)?

d52c84545e305b Vimal Agrawal    2024-10-14  88  	}
ab760791c0cfbb D Scott Phillips 2022-11-14 @89  	return ret;
ab760791c0cfbb D Scott Phillips 2022-11-14  90  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
  2024-10-16 14:22 ` Dan Carpenter
@ 2024-10-17 11:08   ` Vimal Agrawal
  0 siblings, 0 replies; 5+ messages in thread
From: Vimal Agrawal @ 2024-10-17 11:08 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, linux-kernel, gregkh, arnd, lkp, oe-kbuild-all,
	vimal.agrawal

Hi Dan,

Both warning and missed case for return from this function for static
minor are already fixed in the v2 version of the patch. I will be
sending out the v3 version shortly ( after splitting it from kunit
changes).
Thanks for pointing this out.

Vimal

On Wed, Oct 16, 2024 at 7:52 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hi Vimal,
>
> kernel test robot noticed the following build warnings:
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Vimal-Agrawal/misc-misc_minor_alloc-to-allocate-ids-for-all-dynamic-misc-dynamic-minors/20241014-211915
> base:   char-misc/char-misc-testing
> patch link:    https://lore.kernel.org/r/20241014131416.27324-1-vimal.agrawal%40sophos.com
> patch subject: [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors
> config: sparc64-randconfig-r072-20241015 (https://download.01.org/0day-ci/archive/20241016/202410161811.aIPEJHOt-lkp@intel.com/config)
> compiler: sparc64-linux-gcc (GCC) 14.1.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202410161811.aIPEJHOt-lkp@intel.com/
>
> smatch warnings:
> drivers/char/misc.c:89 misc_minor_alloc() error: uninitialized symbol 'ret'.
>
> vim +/ret +89 drivers/char/misc.c
>
> d52c84545e305b Vimal Agrawal    2024-10-14  66  static int misc_minor_alloc(int minor)
> ab760791c0cfbb D Scott Phillips 2022-11-14  67  {
> ab760791c0cfbb D Scott Phillips 2022-11-14  68          int ret;
> ab760791c0cfbb D Scott Phillips 2022-11-14  69
> d52c84545e305b Vimal Agrawal    2024-10-14  70          if (minor == MISC_DYNAMIC_MINOR) {
> d52c84545e305b Vimal Agrawal    2024-10-14  71                  /* allocate free id */
> ab760791c0cfbb D Scott Phillips 2022-11-14  72                  ret = ida_alloc_max(&misc_minors_ida, DYNAMIC_MINORS - 1, GFP_KERNEL);
> ab760791c0cfbb D Scott Phillips 2022-11-14  73                  if (ret >= 0) {
> ab760791c0cfbb D Scott Phillips 2022-11-14  74                          ret = DYNAMIC_MINORS - ret - 1;
> ab760791c0cfbb D Scott Phillips 2022-11-14  75                  } else {
> ab760791c0cfbb D Scott Phillips 2022-11-14  76                          ret = ida_alloc_range(&misc_minors_ida, MISC_DYNAMIC_MINOR + 1,
> ab760791c0cfbb D Scott Phillips 2022-11-14  77                                        MINORMASK, GFP_KERNEL);
> ab760791c0cfbb D Scott Phillips 2022-11-14  78                  }
> d52c84545e305b Vimal Agrawal    2024-10-14  79          } else {
> d52c84545e305b Vimal Agrawal    2024-10-14  80                  /* specific minor, check if it is in dynamic or misc dynamic range  */
> d52c84545e305b Vimal Agrawal    2024-10-14  81                  if (minor < DYNAMIC_MINORS) {
> d52c84545e305b Vimal Agrawal    2024-10-14  82                          minor = DYNAMIC_MINORS - minor - 1;
> d52c84545e305b Vimal Agrawal    2024-10-14  83                          ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
> d52c84545e305b Vimal Agrawal    2024-10-14  84                  }
> d52c84545e305b Vimal Agrawal    2024-10-14  85
> d52c84545e305b Vimal Agrawal    2024-10-14  86                  if (minor > MISC_DYNAMIC_MINOR)
> d52c84545e305b Vimal Agrawal    2024-10-14  87                          ret = ida_alloc_range(&misc_minors_ida, minor, minor, GFP_KERNEL);
>
> What about if minor is >= DYNAMIC_MINORS (128) but <= MISC_DYNAMIC_MINOR (255)?
>
> d52c84545e305b Vimal Agrawal    2024-10-14  88          }
> ab760791c0cfbb D Scott Phillips 2022-11-14 @89          return ret;
> ab760791c0cfbb D Scott Phillips 2022-11-14  90  }
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-10-17 11:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-16 10:28 [PATCH] misc: misc_minor_alloc to allocate ids for all dynamic/misc dynamic minors kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2024-10-14 13:14 Vimal Agrawal
2024-10-14 13:19 ` Greg KH
2024-10-16 14:22 ` Dan Carpenter
2024-10-17 11:08   ` Vimal Agrawal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.