[PATCH v5 0/8] char: misc: Various cleanup for miscdevice

[PATCH v5 0/8] char: misc: Various cleanup for miscdevice

[Zijun Hu]

This patch series is to do cleanup for:

- Miscdevice APIs
- Miscdevice kunit test cases
- Drivers which use miscdevice APIs

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Previous discussion link:
https://lore.kernel.org/all/20250620-rfc_miscdev-v1-1-fda25d502a37@oss.qualcomm.com

---
Changes in v5:
- Replace space with table in fist patch's Makefile
- Correct title and commit messages
- Link to v4: https://lore.kernel.org/r/20250704-rfc_miscdev-v4-0-b48986112d6a@oss.qualcomm.com

Changes in v4:
- Fix WARNINGs reported by "kernel test robot <lkp@intel.com>"
- Link to v3: https://lore.kernel.org/r/20250702-rfc_miscdev-v3-0-d8925de7893d@oss.qualcomm.com

Changes in v3:
- Drop the change which allocates 4 fixed minors for watchdog
- Correct tile and commit message
- Link to v2: https://lore.kernel.org/r/20250701-rfc_miscdev-v2-0-3eb22bf533be@oss.qualcomm.com

---
Zijun Hu (8):
      char: misc: Move drivers/misc/misc_minor_kunit.c to drivers/char/
      char: misc: Adapt and add test cases for simple minor space division
      char: misc: Disallow registering miscdevice whose minor > MISC_DYNAMIC_MINOR
      char: misc: Add a case to test registering miscdevice again without reinitialization
      char: misc: Fix kunit test case miscdev_test_dynamic_reentry() failure
      char: misc: Does not request module for miscdevice with dynamic minor
      char: misc: Register fixed minor EISA_EEPROM_MINOR in linux/miscdevice.h
      sparc: kernel: apc: Remove macro APC_MINOR definition

 arch/sparc/kernel/apc.c                   |  3 +-
 drivers/char/Makefile                     |  1 +
 drivers/char/misc.c                       | 16 +++++-
 drivers/{misc => char}/misc_minor_kunit.c | 95 +++++++++++++++++++++----------
 drivers/misc/Makefile                     |  1 -
 drivers/parisc/eisa_eeprom.c              |  2 -
 include/linux/miscdevice.h                |  9 +++
 7 files changed, 89 insertions(+), 38 deletions(-)
---
base-commit: 626e89412dfb88766d90d842af4d9ec432d8526f
change-id: 20250701-rfc_miscdev-35dd3310c7c0

Best regards,
-- 
Zijun Hu <zijun.hu@oss.qualcomm.com>

[PATCH v5 1/8] char: misc: Move drivers/misc/misc_minor_kunit.c to drivers/char/

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

drivers/misc/misc_minor_kunit.c is to test APIs defined in
drivers/char/misc.c, but is not in the same directory as the later.

Move misc_minor_kunit.c to drivers/char/.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/char/Makefile                     | 1 +
 drivers/{misc => char}/misc_minor_kunit.c | 0
 drivers/misc/Makefile                     | 1 -
 3 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index e9b360cdc99a7fee9c70eabcffe9caf763552aa2..1291369b9126511ff41a06f956af4bb1de7ea4b2 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -6,6 +6,7 @@
 obj-y				+= mem.o random.o
 obj-$(CONFIG_TTY_PRINTK)	+= ttyprintk.o
 obj-y				+= misc.o
+obj-$(CONFIG_TEST_MISC_MINOR)	+= misc_minor_kunit.o
 obj-$(CONFIG_ATARI_DSP56K)	+= dsp56k.o
 obj-$(CONFIG_VIRTIO_CONSOLE)	+= virtio_console.o
 obj-$(CONFIG_UV_MMTIMER)	+= uv_mmtimer.o
diff --git a/drivers/misc/misc_minor_kunit.c b/drivers/char/misc_minor_kunit.c
similarity index 100%
rename from drivers/misc/misc_minor_kunit.c
rename to drivers/char/misc_minor_kunit.c
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 917b9a7183aa1db63cc4049c042c3437f6756a1a..94f44e6c2db79dedc3839bbfb53e10cde103fbc6 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -23,7 +23,6 @@ obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
 obj-$(CONFIG_SENSORS_APDS990X)	+= apds990x.o
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
 obj-$(CONFIG_KGDB_TESTS)	+= kgdbts.o
-obj-$(CONFIG_TEST_MISC_MINOR)	+= misc_minor_kunit.o
 obj-$(CONFIG_SGI_XP)		+= sgi-xp/
 obj-$(CONFIG_SGI_GRU)		+= sgi-gru/
 obj-$(CONFIG_SMPRO_ERRMON)	+= smpro-errmon.o

-- 
2.34.1

[PATCH v5 2/8] char: misc: Adapt and add test cases for simple minor space division

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

Adapt and add test cases for next change which regards minor whose
value > macro MISC_DYNAMIC_MINOR as invalid parameter when register
miscdevice, hence get a simple minor space division below:

<  MISC_DYNAMIC_MINOR: fixed minor code
== MISC_DYNAMIC_MINOR: indicator to request dynamic minor code
>  MISC_DYNAMIC_MINOR: dynamic minor code requested

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/char/misc_minor_kunit.c | 51 +++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/drivers/char/misc_minor_kunit.c b/drivers/char/misc_minor_kunit.c
index 30eceac5f1b6402b0f918af6f56602ed1a6c14ec..3184f383bea8c77cbca69ff5e315ea5de2d5512e 100644
--- a/drivers/char/misc_minor_kunit.c
+++ b/drivers/char/misc_minor_kunit.c
@@ -7,12 +7,6 @@
 #include <linux/file.h>
 #include <linux/init_syscalls.h>
 
-/* dynamic minor (2) */
-static struct miscdevice dev_dynamic_minor = {
-	.minor  = 2,
-	.name   = "dev_dynamic_minor",
-};
-
 /* static minor (LCD_MINOR) */
 static struct miscdevice dev_static_minor = {
 	.minor  = LCD_MINOR,
@@ -25,16 +19,6 @@ static struct miscdevice dev_misc_dynamic_minor = {
 	.name   = "dev_misc_dynamic_minor",
 };
 
-static void kunit_dynamic_minor(struct kunit *test)
-{
-	int ret;
-
-	ret = misc_register(&dev_dynamic_minor);
-	KUNIT_EXPECT_EQ(test, 0, ret);
-	KUNIT_EXPECT_EQ(test, 2, dev_dynamic_minor.minor);
-	misc_deregister(&dev_dynamic_minor);
-}
-
 static void kunit_static_minor(struct kunit *test)
 {
 	int ret;
@@ -157,13 +141,7 @@ static bool is_valid_dynamic_minor(int minor)
 {
 	if (minor < 0)
 		return false;
-	if (minor == MISC_DYNAMIC_MINOR)
-		return false;
-	if (minor >= 0 && minor <= 15)
-		return false;
-	if (minor >= 128 && minor < MISC_DYNAMIC_MINOR)
-		return false;
-	return true;
+	return minor > MISC_DYNAMIC_MINOR;
 }
 
 static int miscdev_test_open(struct inode *inode, struct file *file)
@@ -557,7 +535,7 @@ static void __init miscdev_test_conflict(struct kunit *test)
 	 */
 	miscstat.minor = miscdyn.minor;
 	ret = misc_register(&miscstat);
-	KUNIT_EXPECT_EQ(test, ret, -EBUSY);
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
 	if (ret == 0)
 		misc_deregister(&miscstat);
 
@@ -590,8 +568,9 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
 	misc_deregister(&miscdyn);
 
 	ret = misc_register(&miscstat);
-	KUNIT_EXPECT_EQ(test, ret, 0);
-	KUNIT_EXPECT_EQ(test, miscstat.minor, miscdyn.minor);
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+	if (ret == 0)
+		misc_deregister(&miscstat);
 
 	/*
 	 * Try to register a dynamic minor after registering a static minor
@@ -601,20 +580,32 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
 	miscdyn.minor = MISC_DYNAMIC_MINOR;
 	ret = misc_register(&miscdyn);
 	KUNIT_EXPECT_EQ(test, ret, 0);
-	KUNIT_EXPECT_NE(test, miscdyn.minor, miscstat.minor);
+	KUNIT_EXPECT_EQ(test, miscdyn.minor, miscstat.minor);
 	KUNIT_EXPECT_TRUE(test, is_valid_dynamic_minor(miscdyn.minor));
 	if (ret == 0)
 		misc_deregister(&miscdyn);
+}
 
-	miscdev_test_can_open(test, &miscstat);
+/* Take minor(> MISC_DYNAMIC_MINOR) as invalid when register miscdevice */
+static void miscdev_test_invalid_input(struct kunit *test)
+{
+	struct miscdevice misc_test = {
+		.minor = MISC_DYNAMIC_MINOR + 1,
+		.name = "misc_test",
+		.fops = &miscdev_test_fops,
+	};
+	int ret;
 
-	misc_deregister(&miscstat);
+	ret = misc_register(&misc_test);
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+	if (ret == 0)
+		misc_deregister(&misc_test);
 }
 
 static struct kunit_case test_cases[] = {
-	KUNIT_CASE(kunit_dynamic_minor),
 	KUNIT_CASE(kunit_static_minor),
 	KUNIT_CASE(kunit_misc_dynamic_minor),
+	KUNIT_CASE(miscdev_test_invalid_input),
 	KUNIT_CASE_PARAM(miscdev_test_twice, miscdev_gen_params),
 	KUNIT_CASE_PARAM(miscdev_test_duplicate_minor, miscdev_gen_params),
 	KUNIT_CASE(miscdev_test_duplicate_name),

-- 
2.34.1

[PATCH v5 3/8] char: misc: Disallow registering miscdevice whose minor > MISC_DYNAMIC_MINOR

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

Currently, It is allowed to register miscdevice with minor > 255
which is defined by macro MISC_DYNAMIC_MINOR, and cause:

- Chaos regarding division and management of minor codes.
- Registering failure if the minor was allocated to other dynamic request.

Fortunately, in-kernel users have not had such usage yet.
Fix by refusing to register miscdevice whose minor > 255.

Also bring in a very simple minor code space division and management:

<  255 : Fixed minor code
== 255 : Indicator to request dynamic minor code
>  255 : Dynamic minor code requested, 1048320 minor codes totally
And all fixed minors allocated should be registered in 'linux/miscdevice.h'

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/char/misc.c        | 6 ++++++
 include/linux/miscdevice.h | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 558302a64dd908aee30341547a5413df1af71459..b8e66466184fa21fb66d968db7950e0b5669ac43 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -210,6 +210,12 @@ int misc_register(struct miscdevice *misc)
 	int err = 0;
 	bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
 
+	if (misc->minor > MISC_DYNAMIC_MINOR) {
+		pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
+		       misc->minor, misc->name);
+		return -EINVAL;
+	}
+
 	INIT_LIST_HEAD(&misc->list);
 
 	mutex_lock(&misc_mtx);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 3e6deb00fc8535a7571f85489c74979e18385bad..565b88efeb23d02b7d91df1cd7df4bdcf2898224 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -71,6 +71,14 @@
 #define USERIO_MINOR		240
 #define VHOST_VSOCK_MINOR	241
 #define RFKILL_MINOR		242
+
+/*
+ * Misc char device minor code space division related to below macro:
+ *
+ * <  255  : Fixed minor code
+ * == 255  : Indicator to request dynamic minor code
+ * >  255  : Dynamic minor code requested, 1048320 minor codes totally.
+ */
 #define MISC_DYNAMIC_MINOR	255
 
 struct miscdevice {

-- 
2.34.1

[PATCH v5 4/8] char: misc: Add a case to test registering miscdevice again without reinitialization

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

For miscdevice who wants dynamic minor, add a cast to test if it can be
successfully registered again without reinitialization:

1) Provide Both miscdevice @dev_A and @dev_B want to request dynamic
   minor by initializing their minor to MISC_DYNAMIC_MINOR.

2) Register then de-register @dev_A.

3) Register @dev_B.

4) Register @dev_A again without reinitialization.

5) Check if @dev_A can be successfully registered.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/char/misc_minor_kunit.c | 44 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/char/misc_minor_kunit.c b/drivers/char/misc_minor_kunit.c
index 3184f383bea8c77cbca69ff5e315ea5de2d5512e..6fc8b05169c5754b96588088471a844a2e2ee29b 100644
--- a/drivers/char/misc_minor_kunit.c
+++ b/drivers/char/misc_minor_kunit.c
@@ -602,6 +602,49 @@ static void miscdev_test_invalid_input(struct kunit *test)
 		misc_deregister(&misc_test);
 }
 
+/*
+ * Verify if @miscdyn_a can still be registered successfully without
+ * reinitialization even if its minor ever owned was requested by
+ * another miscdevice such as @miscdyn_b.
+ */
+static void miscdev_test_dynamic_reentry(struct kunit *test)
+{
+	struct miscdevice miscdyn_a = {
+		.name = "miscdyn_a",
+		.minor = MISC_DYNAMIC_MINOR,
+		.fops = &miscdev_test_fops,
+	};
+	struct miscdevice miscdyn_b = {
+		.name = "miscdyn_b",
+		.minor = MISC_DYNAMIC_MINOR,
+		.fops = &miscdev_test_fops,
+	};
+	int ret, minor_a;
+
+	ret = misc_register(&miscdyn_a);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	KUNIT_EXPECT_TRUE(test, is_valid_dynamic_minor(miscdyn_a.minor));
+	minor_a = miscdyn_a.minor;
+	if (ret != 0)
+		return;
+	misc_deregister(&miscdyn_a);
+
+	ret = misc_register(&miscdyn_b);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, miscdyn_b.minor, minor_a);
+	if (ret != 0)
+		return;
+
+	ret = misc_register(&miscdyn_a);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	KUNIT_EXPECT_TRUE(test, is_valid_dynamic_minor(miscdyn_a.minor));
+	KUNIT_EXPECT_NE(test, miscdyn_a.minor, miscdyn_b.minor);
+	if (ret == 0)
+		misc_deregister(&miscdyn_a);
+
+	misc_deregister(&miscdyn_b);
+}
+
 static struct kunit_case test_cases[] = {
 	KUNIT_CASE(kunit_static_minor),
 	KUNIT_CASE(kunit_misc_dynamic_minor),
@@ -611,6 +654,7 @@ static struct kunit_case test_cases[] = {
 	KUNIT_CASE(miscdev_test_duplicate_name),
 	KUNIT_CASE(miscdev_test_duplicate_name_leak),
 	KUNIT_CASE_PARAM(miscdev_test_duplicate_error, miscdev_gen_params),
+	KUNIT_CASE(miscdev_test_dynamic_reentry),
 	{}
 };
 

-- 
2.34.1

[PATCH v5 5/8] char: misc: Fix kunit test case miscdev_test_dynamic_reentry() failure

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

misc_deregister() frees dynamic minor @misc->minor but does not
reset it to macro MISC_DYNAMIC_MINOR, so 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

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@oss.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);
 }
 EXPORT_SYMBOL(misc_deregister);

-- 
2.34.1

[PATCH v5 6/8] char: misc: Does not request module for miscdevice with dynamic minor

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

misc_open() may request module for miscdevice with dynamic minor, which
is meaningless since:

- The dynamic minor allocated is unknown in advance without registering
  miscdevice firstly.
- Macro MODULE_ALIAS_MISCDEV() is not applicable for dynamic minor.

Fix by only requesting module for miscdevice with fixed minor.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/char/misc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 96ed343cf5c8509a09855020049a9af82a3ede95..a0aae0fc792666a7bdc0ba00da9dc02ff9cead42 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -132,7 +132,8 @@ static int misc_open(struct inode *inode, struct file *file)
 		break;
 	}
 
-	if (!new_fops) {
+	/* Only request module for fixed minor code */
+	if (!new_fops && minor < MISC_DYNAMIC_MINOR) {
 		mutex_unlock(&misc_mtx);
 		request_module("char-major-%d-%d", MISC_MAJOR, minor);
 		mutex_lock(&misc_mtx);
@@ -144,10 +145,11 @@ static int misc_open(struct inode *inode, struct file *file)
 			new_fops = fops_get(iter->fops);
 			break;
 		}
-		if (!new_fops)
-			goto fail;
 	}
 
+	if (!new_fops)
+		goto fail;
+
 	/*
 	 * Place the miscdevice in the file's
 	 * private_data so it can be used by the

-- 
2.34.1

[PATCH v5 7/8] char: misc: Register fixed minor EISA_EEPROM_MINOR in linux/miscdevice.h

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

Move fixed minor EISA_EEPROM_MINOR definition to linux/miscdevice.h.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/parisc/eisa_eeprom.c | 2 --
 include/linux/miscdevice.h   | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/parisc/eisa_eeprom.c b/drivers/parisc/eisa_eeprom.c
index 443b15422fc179c7379082bb165ea2bb80785fb3..601cbb22574fd64b0c6be568442d5064d38a2c20 100644
--- a/drivers/parisc/eisa_eeprom.c
+++ b/drivers/parisc/eisa_eeprom.c
@@ -15,8 +15,6 @@
 #include <linux/uaccess.h>
 #include <asm/eisa_eeprom.h>
 
-#define 	EISA_EEPROM_MINOR 241
-
 static loff_t eisa_eeprom_llseek(struct file *file, loff_t offset, int origin)
 {
 	return fixed_size_llseek(file, offset, origin, HPEE_MAX_LENGTH);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 565b88efeb23d02b7d91df1cd7df4bdcf2898224..7d0aa718499cc1867790e98eb6b84c1673091905 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -70,6 +70,7 @@
 #define UHID_MINOR		239
 #define USERIO_MINOR		240
 #define VHOST_VSOCK_MINOR	241
+#define EISA_EEPROM_MINOR	241
 #define RFKILL_MINOR		242
 
 /*

-- 
2.34.1

[PATCH v5 8/8] sparc: kernel: apc: Remove macro APC_MINOR definition

[Zijun Hu]

From: Zijun Hu <zijun.hu@oss.qualcomm.com>

Macro APC_MINOR is defined as MISC_DYNAMIC_MINOR to request dynamic
minor, but its name 'APC_MINOR' looks like fixed minor.

Remove the macro definition and directly use MISC_DYNAMIC_MINOR instead.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 arch/sparc/kernel/apc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/apc.c b/arch/sparc/kernel/apc.c
index d44725d37e30f388bf8cf19d72baf720f94da084..849db20e7165cdf48d4d36cf770dd6aaaa191b41 100644
--- a/arch/sparc/kernel/apc.c
+++ b/arch/sparc/kernel/apc.c
@@ -28,7 +28,6 @@
  * #define APC_DEBUG_LED
  */
 
-#define APC_MINOR	MISC_DYNAMIC_MINOR
 #define APC_OBPNAME	"power-management"
 #define APC_DEVNAME "apc"
 
@@ -138,7 +137,7 @@ static const struct file_operations apc_fops = {
 	.llseek =		noop_llseek,
 };
 
-static struct miscdevice apc_miscdev = { APC_MINOR, APC_DEVNAME, &apc_fops };
+static struct miscdevice apc_miscdev = { MISC_DYNAMIC_MINOR, APC_DEVNAME, &apc_fops };
 
 static int apc_probe(struct platform_device *op)
 {

-- 
2.34.1

Re: [PATCH v5 2/8] char: misc: Adapt and add test cases for simple minor space division

[Thadeu Lima de Souza Cascardo]

On Thu, Jul 10, 2025 at 07:56:45PM +0800, Zijun Hu wrote:
> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
> 
> Adapt and add test cases for next change which regards minor whose
> value > macro MISC_DYNAMIC_MINOR as invalid parameter when register
> miscdevice, hence get a simple minor space division below:
> 
> <  MISC_DYNAMIC_MINOR: fixed minor code
> == MISC_DYNAMIC_MINOR: indicator to request dynamic minor code
> >  MISC_DYNAMIC_MINOR: dynamic minor code requested
> 
> Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
> ---
>  drivers/char/misc_minor_kunit.c | 51 +++++++++++++++++------------------------
>  1 file changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/char/misc_minor_kunit.c b/drivers/char/misc_minor_kunit.c
> index 30eceac5f1b6402b0f918af6f56602ed1a6c14ec..3184f383bea8c77cbca69ff5e315ea5de2d5512e 100644
> --- a/drivers/char/misc_minor_kunit.c
> +++ b/drivers/char/misc_minor_kunit.c
> @@ -7,12 +7,6 @@
>  #include <linux/file.h>
>  #include <linux/init_syscalls.h>
>  
> -/* dynamic minor (2) */
> -static struct miscdevice dev_dynamic_minor = {
> -	.minor  = 2,
> -	.name   = "dev_dynamic_minor",
> -};
> -
>  /* static minor (LCD_MINOR) */
>  static struct miscdevice dev_static_minor = {
>  	.minor  = LCD_MINOR,
> @@ -25,16 +19,6 @@ static struct miscdevice dev_misc_dynamic_minor = {
>  	.name   = "dev_misc_dynamic_minor",
>  };
>  
> -static void kunit_dynamic_minor(struct kunit *test)
> -{
> -	int ret;
> -
> -	ret = misc_register(&dev_dynamic_minor);
> -	KUNIT_EXPECT_EQ(test, 0, ret);
> -	KUNIT_EXPECT_EQ(test, 2, dev_dynamic_minor.minor);
> -	misc_deregister(&dev_dynamic_minor);
> -}
> -
>  static void kunit_static_minor(struct kunit *test)
>  {
>  	int ret;
> @@ -157,13 +141,7 @@ static bool is_valid_dynamic_minor(int minor)
>  {
>  	if (minor < 0)
>  		return false;
> -	if (minor == MISC_DYNAMIC_MINOR)
> -		return false;
> -	if (minor >= 0 && minor <= 15)
> -		return false;
> -	if (minor >= 128 && minor < MISC_DYNAMIC_MINOR)
> -		return false;
> -	return true;
> +	return minor > MISC_DYNAMIC_MINOR;
>  }
>  
>  static int miscdev_test_open(struct inode *inode, struct file *file)
> @@ -557,7 +535,7 @@ static void __init miscdev_test_conflict(struct kunit *test)
>  	 */
>  	miscstat.minor = miscdyn.minor;
>  	ret = misc_register(&miscstat);
> -	KUNIT_EXPECT_EQ(test, ret, -EBUSY);
> +	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
>  	if (ret == 0)
>  		misc_deregister(&miscstat);
>  
> @@ -590,8 +568,9 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
>  	misc_deregister(&miscdyn);
>  
>  	ret = misc_register(&miscstat);
> -	KUNIT_EXPECT_EQ(test, ret, 0);
> -	KUNIT_EXPECT_EQ(test, miscstat.minor, miscdyn.minor);
> +	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
> +	if (ret == 0)
> +		misc_deregister(&miscstat);
>  
>  	/*
>  	 * Try to register a dynamic minor after registering a static minor
> @@ -601,20 +580,32 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
>  	miscdyn.minor = MISC_DYNAMIC_MINOR;
>  	ret = misc_register(&miscdyn);
>  	KUNIT_EXPECT_EQ(test, ret, 0);
> -	KUNIT_EXPECT_NE(test, miscdyn.minor, miscstat.minor);
> +	KUNIT_EXPECT_EQ(test, miscdyn.minor, miscstat.minor);
>  	KUNIT_EXPECT_TRUE(test, is_valid_dynamic_minor(miscdyn.minor));
>  	if (ret == 0)
>  		misc_deregister(&miscdyn);
> +}
>  
> -	miscdev_test_can_open(test, &miscstat);
> +/* Take minor(> MISC_DYNAMIC_MINOR) as invalid when register miscdevice */
> +static void miscdev_test_invalid_input(struct kunit *test)
> +{
> +	struct miscdevice misc_test = {
> +		.minor = MISC_DYNAMIC_MINOR + 1,
> +		.name = "misc_test",
> +		.fops = &miscdev_test_fops,
> +	};
> +	int ret;
>  
> -	misc_deregister(&miscstat);
> +	ret = misc_register(&misc_test);
> +	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
> +	if (ret == 0)
> +		misc_deregister(&misc_test);
>  }
>  

These tests will fail if applied before patch 3 "char: misc: Disallow registering miscdevice whose minor > MISC_DYNAMIC_MINOR".

One option is just merge the two commits.

I have worked on a different option, which would be the two patches above,
before applying patch 3. Then, we can either support the two different
behaviors in the test case, or remove the support for the old behavior
after patch 3 is applied.


>  static struct kunit_case test_cases[] = {
> -	KUNIT_CASE(kunit_dynamic_minor),
>  	KUNIT_CASE(kunit_static_minor),
>  	KUNIT_CASE(kunit_misc_dynamic_minor),
> +	KUNIT_CASE(miscdev_test_invalid_input),
>  	KUNIT_CASE_PARAM(miscdev_test_twice, miscdev_gen_params),
>  	KUNIT_CASE_PARAM(miscdev_test_duplicate_minor, miscdev_gen_params),
>  	KUNIT_CASE(miscdev_test_duplicate_name),
> 
> -- 
> 2.34.1
> 


From 12e05f189745ce38bf792a448ff8d7117f20a3c0 Mon Sep 17 00:00:00 2001
From: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Date: Thu, 26 Jun 2025 10:32:03 -0300
Subject: [PATCH 1/3] allow static register to fail

---
 drivers/misc/misc_minor_kunit.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/misc_minor_kunit.c b/drivers/misc/misc_minor_kunit.c
index 30eceac5f1b6..2ef92ce3d053 100644
--- a/drivers/misc/misc_minor_kunit.c
+++ b/drivers/misc/misc_minor_kunit.c
@@ -569,6 +569,7 @@ static void __init miscdev_test_conflict(struct kunit *test)
 static void __init miscdev_test_conflict_reverse(struct kunit *test)
 {
 	int ret;
+	bool stat_registered = false;
 	struct miscdevice miscdyn = {
 		.name = "miscdyn",
 		.minor = MISC_DYNAMIC_MINOR,
@@ -592,6 +593,8 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
 	ret = misc_register(&miscstat);
 	KUNIT_EXPECT_EQ(test, ret, 0);
 	KUNIT_EXPECT_EQ(test, miscstat.minor, miscdyn.minor);
+	if (ret == 0)
+		stat_registered = true;
 
 	/*
 	 * Try to register a dynamic minor after registering a static minor
@@ -606,9 +609,10 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
 	if (ret == 0)
 		misc_deregister(&miscdyn);
 
-	miscdev_test_can_open(test, &miscstat);
-
-	misc_deregister(&miscstat);
+	if (stat_registered) {
+		miscdev_test_can_open(test, &miscstat);
+		misc_deregister(&miscstat);
+	}
 }
 
 static struct kunit_case test_cases[] = {
-- 
2.47.2


From c9974a5b2cb59ad3ee5e371cb42c5969a46128b2 Mon Sep 17 00:00:00 2001
From: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Date: Thu, 26 Jun 2025 15:43:31 -0300
Subject: [PATCH 2/3] support the case where registering on dynamic range is
 not valid

---
 drivers/misc/misc_minor_kunit.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/misc_minor_kunit.c b/drivers/misc/misc_minor_kunit.c
index 2ef92ce3d053..18f477b18562 100644
--- a/drivers/misc/misc_minor_kunit.c
+++ b/drivers/misc/misc_minor_kunit.c
@@ -557,7 +557,7 @@ static void __init miscdev_test_conflict(struct kunit *test)
 	 */
 	miscstat.minor = miscdyn.minor;
 	ret = misc_register(&miscstat);
-	KUNIT_EXPECT_EQ(test, ret, -EBUSY);
+	KUNIT_EXPECT_TRUE(test, ret == -EINVAL || ret == -EBUSY);
 	if (ret == 0)
 		misc_deregister(&miscstat);
 
@@ -591,10 +591,12 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
 	misc_deregister(&miscdyn);
 
 	ret = misc_register(&miscstat);
-	KUNIT_EXPECT_EQ(test, ret, 0);
-	KUNIT_EXPECT_EQ(test, miscstat.minor, miscdyn.minor);
-	if (ret == 0)
+	if (ret == 0) {
+		KUNIT_EXPECT_EQ(test, miscstat.minor, miscdyn.minor);
 		stat_registered = true;
+	} else {
+		KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+	}
 
 	/*
 	 * Try to register a dynamic minor after registering a static minor
@@ -604,7 +606,8 @@ static void __init miscdev_test_conflict_reverse(struct kunit *test)
 	miscdyn.minor = MISC_DYNAMIC_MINOR;
 	ret = misc_register(&miscdyn);
 	KUNIT_EXPECT_EQ(test, ret, 0);
-	KUNIT_EXPECT_NE(test, miscdyn.minor, miscstat.minor);
+	if (stat_registered)
+		KUNIT_EXPECT_NE(test, miscdyn.minor, miscstat.minor);
 	KUNIT_EXPECT_TRUE(test, is_valid_dynamic_minor(miscdyn.minor));
 	if (ret == 0)
 		misc_deregister(&miscdyn);
-- 
2.47.2

Re: [PATCH v5 5/8] char: misc: Fix kunit test case miscdev_test_dynamic_reentry() failure

[Thadeu Lima de Souza Cascardo]

On Thu, Jul 10, 2025 at 07:56:48PM +0800, Zijun Hu wrote:
> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
> 
> misc_deregister() frees dynamic minor @misc->minor but does not
> reset it to macro MISC_DYNAMIC_MINOR, so 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
> 
> Fix by resetting @misc->minor to MISC_DYNAMIC_MINOR in misc_deregister()
> as error handling of misc_register() does.
> 

Adding a failing test and then fixing the code does not seem the best way
to justify this change. I would rather add the fix with a proper
justification and then add the test.

On the other hand, I have found real cases where this might happen, some by
code inspection only, but I also managed to reproduce the issue here,
where:

1) wmi/dell-smbios registered minor 122, acpi_thermal_rel registered minor
123.
2) unbind "int3400 thermal" driver from its device, this will unregister
acpi_thermal_rel
3) remove dell_smbios module
4) reinstall dell_smbios module, now wmi/dell-smbios is using misc 123
5) bind the device to "int3400 thermal" driver again, acpi_thermal_rel
fails to register

I think we have a few options to fix these bugs:

1) Apply your suggested fix.
2) Fix all the buggy drivers.
3) Change API and have the minor be a misc_register parameter.

The advantage of your option is that it is simple and contained and easy to
backport.

Changing API would require changing a lot of code and hard to backport, but
I find it less error-prone than requiring the minor member to be reset, if
we end up deciding about fixing the drivers.

As for fixing individual drivers, one helpful feature is applying your
previous patch [1], but perhaps with stronger message, maybe a WARN_ON.

[1] char: misc: Disallow registering miscdevice whose minor > MISC_DYNAMIC_MINOR

I am leaning towards your suggested fix, but with different wording, and
before adding the test case.

Something like:

Some drivers may reuse the miscdevice structure after they are
deregistered. If the intention is to allocate a dynamic minor, if the minor
number is not reset to MISC_DYNAMIC_MINOR before calling misc_register, it
will try to register a previously dynamically allocated minor number, which
may have been registered by a different driver.

One such case is the acpi_thermal_rel misc device, registered by the
int3400 thermal driver. If the device is unbound from the driver and later
bound, if there was another dynamic misc device registered in between, it
would fail to register the acpi_thermal_rel misc device. Other drivers
behave similarly.

Instead of fixing all the drivers, just reset the minor member to
MISC_DYNAMIC_MINOR when calling misc_deregister in case it was a
dynamically allocated minor number.

> Signed-off-by: Zijun Hu <zijun.hu@oss.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);
>  }
>  EXPORT_SYMBOL(misc_deregister);
> 
> -- 
> 2.34.1
> 

Re: [PATCH v5 6/8] char: misc: Does not request module for miscdevice with dynamic minor

[Thadeu Lima de Souza Cascardo]

On Thu, Jul 10, 2025 at 07:56:49PM +0800, Zijun Hu wrote:
> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
> 
> misc_open() may request module for miscdevice with dynamic minor, which
> is meaningless since:
> 
> - The dynamic minor allocated is unknown in advance without registering
>   miscdevice firstly.
> - Macro MODULE_ALIAS_MISCDEV() is not applicable for dynamic minor.
> 
> Fix by only requesting module for miscdevice with fixed minor.
> 
> Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
> ---
>  drivers/char/misc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index 96ed343cf5c8509a09855020049a9af82a3ede95..a0aae0fc792666a7bdc0ba00da9dc02ff9cead42 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -132,7 +132,8 @@ static int misc_open(struct inode *inode, struct file *file)
>  		break;
>  	}
>  
> -	if (!new_fops) {
> +	/* Only request module for fixed minor code */
> +	if (!new_fops && minor < MISC_DYNAMIC_MINOR) {
>  		mutex_unlock(&misc_mtx);
>  		request_module("char-major-%d-%d", MISC_MAJOR, minor);
>  		mutex_lock(&misc_mtx);
> @@ -144,10 +145,11 @@ static int misc_open(struct inode *inode, struct file *file)
>  			new_fops = fops_get(iter->fops);
>  			break;
>  		}
> -		if (!new_fops)
> -			goto fail;
>  	}
>  
> +	if (!new_fops)
> +		goto fail;
> +
>  	/*
>  	 * Place the miscdevice in the file's
>  	 * private_data so it can be used by the
> 
> -- 
> 2.34.1
> 

Given this should not break any code, as there should be no legit drivers
requesting a minor >= 255,

Acked-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Re: [PATCH v5 2/8] char: misc: Adapt and add test cases for simple minor space division

[Zijun Hu]

On 2025/7/10 22:34, Thadeu Lima de Souza Cascardo wrote:
> These tests will fail if applied before patch 3 "char: misc: Disallow 
> registering miscdevice whose minor > MISC_DYNAMIC_MINOR".
>
yes, that is expected since:
test case to expose issue may need to be sorted before fix to solve
the issue.

I ever had below observation that below patch order were reversed when
they are applied by robh finally.

https://lore.kernel.org/all/20241216-of_core_fix-v2-0-e69b8f60da63@quicinc.com/
[PATCH v2 1/7] of: Fix API of_find_node_opts_by_path() finding OF device
node failure
[PATCH v2 2/7] of: unittest: Add a test case for API
of_find_node_opts_by_path()

> One option is just merge the two commits.
> 
> I have worked on a different option, which would be the two patches 
> above,
> before applying patch 3. Then, we can either support the two different
> behaviors in the test case, or remove the support for the old behavior
> after patch 3 is applied.

Re: [PATCH v5 5/8] char: misc: Fix kunit test case miscdev_test_dynamic_reentry() failure

[Zijun Hu]

On 2025/7/11 02:15, Thadeu Lima de Souza Cascardo wrote:
> Adding a failing test and then fixing the code does not seem the best way
> to justify this change. I would rather add the fix with a proper
> justification and then add the test.
>
may need to only correct commit message. the order about unit test and
fix may be right as last reply.

> On the other hand, I have found real cases where this might happen, some by
> code inspection only, but I also managed to reproduce the issue here,
> where:
> 
> 1) wmi/dell-smbios registered minor 122, acpi_thermal_rel registered minor
> 123.
> 2) unbind "int3400 thermal" driver from its device, this will unregister
> acpi_thermal_rel
> 3) remove dell_smbios module
> 4) reinstall dell_smbios module, now wmi/dell-smbios is using misc 123
> 5) bind the device to "int3400 thermal" driver again, acpi_thermal_rel
> fails to register
> 

above issue should not happen with current char-misc tree since fixed
minor have no such reentry issue:

for any fixed minor fixed_A in range [0, 255): ".minor = fixed_A" ->
registered -> ".minor = fixed_A" -> de-registered -> ".minor = fixed_A"
, namely, for fixed minor, it is always un-changed about registering
and de-registering.


> I think we have a few options to fix these bugs:
> 
> 1) Apply your suggested fix.
> 2) Fix all the buggy drivers.
> 3) Change API and have the minor be a misc_register parameter.
> 
> The advantage of your option is that it is simple and contained and easy to
> backport.
> 
> Changing API would require changing a lot of code and hard to backport, but
> I find it less error-prone than requiring the minor member to be reset, if
> we end up deciding about fixing the drivers.
> 
> As for fixing individual drivers, one helpful feature is applying your
> previous patch [1], but perhaps with stronger message, maybe a WARN_ON.
> 
> [1] char: misc: Disallow registering miscdevice whose minor > MISC_DYNAMIC_MINOR
> 
> I am leaning towards your suggested fix, but with different wording, and
> before adding the test case.
> 
> Something like:
> 
> Some drivers may reuse the miscdevice structure after they are
> deregistered. If the intention is to allocate a dynamic minor, if the minor
> number is not reset to MISC_DYNAMIC_MINOR before calling misc_register, it
> will try to register a previously dynamically allocated minor number, which
> may have been registered by a different driver.
> 

let me correct commit message based on this suggestions.
thank you.

> One such case is the acpi_thermal_rel misc device, registered by the
> int3400 thermal driver. If the device is unbound from the driver and later
> bound, if there was another dynamic misc device registered in between, it
> would fail to register the acpi_thermal_rel misc device. Other drivers
> behave similarly.
> 
> Instead of fixing all the drivers, just reset the minor member to
> MISC_DYNAMIC_MINOR when calling misc_deregister in case it was a
> dynamically allocated minor number.

Re: [PATCH v5 5/8] char: misc: Fix kunit test case miscdev_test_dynamic_reentry() failure

[Thadeu Lima de Souza Cascardo]

On Mon, Jul 14, 2025 at 09:02:09AM +0800, Zijun Hu wrote:
> On 2025/7/11 02:15, Thadeu Lima de Souza Cascardo wrote:
> > Adding a failing test and then fixing the code does not seem the best way
> > to justify this change. I would rather add the fix with a proper
> > justification and then add the test.
> >
> may need to only correct commit message. the order about unit test and
> fix may be right as last reply.
> 
> > On the other hand, I have found real cases where this might happen, some by
> > code inspection only, but I also managed to reproduce the issue here,
> > where:
> > 
> > 1) wmi/dell-smbios registered minor 122, acpi_thermal_rel registered minor
> > 123.
> > 2) unbind "int3400 thermal" driver from its device, this will unregister
> > acpi_thermal_rel
> > 3) remove dell_smbios module
> > 4) reinstall dell_smbios module, now wmi/dell-smbios is using misc 123
> > 5) bind the device to "int3400 thermal" driver again, acpi_thermal_rel
> > fails to register
> > 
> 
> above issue should not happen with current char-misc tree since fixed
> minor have no such reentry issue:
> 
> for any fixed minor fixed_A in range [0, 255): ".minor = fixed_A" ->
> registered -> ".minor = fixed_A" -> de-registered -> ".minor = fixed_A"
> , namely, for fixed minor, it is always un-changed about registering
> and de-registering.
> 

I am running an older tree, where the misc range is still below 128, but
notice those numbers are in the dynamic range. Those two drivers are using
the dynamic misc. I am just showing that what you are trying to fix here is
a real issue. And, below, I suggested a paragraph in the commit message
mentioning it.

Cascardo.

> 
> > I think we have a few options to fix these bugs:
> > 
> > 1) Apply your suggested fix.
> > 2) Fix all the buggy drivers.
> > 3) Change API and have the minor be a misc_register parameter.
> > 
> > The advantage of your option is that it is simple and contained and easy to
> > backport.
> > 
> > Changing API would require changing a lot of code and hard to backport, but
> > I find it less error-prone than requiring the minor member to be reset, if
> > we end up deciding about fixing the drivers.
> > 
> > As for fixing individual drivers, one helpful feature is applying your
> > previous patch [1], but perhaps with stronger message, maybe a WARN_ON.
> > 
> > [1] char: misc: Disallow registering miscdevice whose minor > MISC_DYNAMIC_MINOR
> > 
> > I am leaning towards your suggested fix, but with different wording, and
> > before adding the test case.
> > 
> > Something like:
> > 
> > Some drivers may reuse the miscdevice structure after they are
> > deregistered. If the intention is to allocate a dynamic minor, if the minor
> > number is not reset to MISC_DYNAMIC_MINOR before calling misc_register, it
> > will try to register a previously dynamically allocated minor number, which
> > may have been registered by a different driver.
> > 
> 
> let me correct commit message based on this suggestions.
> thank you.
> 
> > One such case is the acpi_thermal_rel misc device, registered by the
> > int3400 thermal driver. If the device is unbound from the driver and later
> > bound, if there was another dynamic misc device registered in between, it
> > would fail to register the acpi_thermal_rel misc device. Other drivers
> > behave similarly.
> > 
> > Instead of fixing all the drivers, just reset the minor member to
> > MISC_DYNAMIC_MINOR when calling misc_deregister in case it was a
> > dynamically allocated minor number.
>