linux-kernel - Re: [Merge tag 'pci-v4.12-changes' of git] 857f864014: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a8

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: <f848a5fb-096c-a025-cc3b-314cc3899f9e@deltatee.com>
Date:   Tue, 13 Jun 2017 11:47:30 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Bjorn Helgaas <helgaas@...nel.org>,
        kernel test robot <fengguang.wu@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        LKP <lkp@...org>, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>, wfg@...ux.intel.com,
        Alan Cox <alan@...ux.intel.com>, Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        David Airlie <airlied@...ux.ie>,
        David Herrmann <dh.herrmann@...il.com>,
        Daniel Vetter <daniel.vetter@...ll.ch>
Subject: Re: [Merge tag 'pci-v4.12-changes' of git] 857f864014: BUG: unable to
 handle kernel NULL pointer dereference at 00000000000000a8



On 13/06/17 10:35 AM, Greg Kroah-Hartman wrote:
> For char devices, I doubt it, but we can't take the chance, which is why
> you make it an option.  Then, it's enabled for 'allmodconfig' builds,
> which helps testers out.

Well I took a look at this and it looks like a lot of work to modify all
the drivers to support a possible dynamic allocation and I'm not really
able to do all that right now.

However, correct me if I'm missing something, but it looks fairly
straightforward to extend the dynamic region above 256 in cases like
this. There are already fixed major numbers above 255 and the
infrastructure appears to support it. So what are your thoughts on the
change below? I'd be happy to clean it up into a proper patch if you
agree it's a workable option.

Thanks,

Logan



diff --git a/fs/char_dev.c b/fs/char_dev.c
index fb8507f521b2..539352425d95 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -59,6 +59,29 @@ void chrdev_show(struct seq_file *f, off_t offset)

 #endif /* CONFIG_PROC_FS */

+static int find_dynamic_major(void)
+{
+	int i;
+	struct char_device_struct **cp;
+
+	for (i = ARRAY_SIZE(chrdevs)-1; i > CHRDEV_MAJOR_DYN_END; i--) {
+		if (chrdevs[i] == NULL)
+			return i;
+	}
+
+	for (i = CHRDEV_MAJOR_DYN_EXT_START;
+	     i > CHRDEV_MAJOR_DYN_EXT_END; i--) {
+		for (cp = &chrdevs[major_to_index(i)]; *cp; cp = &(*cp)->next)
+			if ((*cp)->major == i)
+				break;
+
+		if (*cp == NULL || (*cp)->major != i)
+			return i;
+	}
+
+	return -EBUSY;
+}
+
 /*
  * Register a single major with a specified minor range.
  *
@@ -84,22 +107,11 @@ __register_chrdev_region(unsigned int major,
unsigned int baseminor,

 	mutex_lock(&chrdevs_lock);

-	/* temporary */
 	if (major == 0) {
-		for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--) {
-			if (chrdevs[i] == NULL)
-				break;
-		}
-
-		if (i < CHRDEV_MAJOR_DYN_END)
-			pr_warn("CHRDEV \"%s\" major number %d goes below the dynamic
allocation range\n",
-				name, i);
-
-		if (i == 0) {
-			ret = -EBUSY;
+		ret = find_dynamic_major();
+		if (ret < 0)
 			goto out;
-		}
-		major = i;
+		major = ret;
 	}

 	cd->major = major;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 249dad4e8d26..5780d69034ca 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2448,6 +2448,9 @@ static inline void bd_unlink_disk_holder(struct
block_device *bdev,
 #define CHRDEV_MAJOR_HASH_SIZE	255
 /* Marks the bottom of the first segment of free char majors */
 #define CHRDEV_MAJOR_DYN_END 234
+#define CHRDEV_MAJOR_DYN_EXT_START 511
+#define CHRDEV_MAJOR_DYN_EXT_END 384
+
 extern int alloc_chrdev_region(dev_t *, unsigned, unsigned, const char *);
 extern int register_chrdev_region(dev_t, unsigned, const char *);
 extern int __register_chrdev(unsigned int major, unsigned int baseminor,



----

This results in char devices like this (another patch might be prudent
to fix the ordering):

Character devices:
510 ttySLM
  1 mem
511 noz
  4 /dev/vc/0
  4 tty
  4 ttyS
  5 /dev/tty
  5 /dev/console
  5 /dev/ptmx
  7 vcs
  9 st
 10 misc
 13 input
 21 sg
 29 fb
 43 ttyI
 45 isdn
 68 capi20
 86 ch
 90 mtd
 99 ppdev
108 ppp
128 ptm
136 pts
152 aoechr
153 spi
161 ircomm
172 ttyMX
174 ttyMI
202 cpu/msr
203 cpu/cpuid
204 ttyJ
204 ttyMAX
206 osst
226 drm
235 ttyS
236 ttyRP
237 ttyARC
238 ttyPS
239 ttyIFX
494 rio_cm
240 ttySC
495 cros_ec
241 ipmidev
496 hidraw
242 rio_mport
497 ttySDIO
243 xz_dec_test
498 uio
244 bsg
499 firewire
245 pvfs2-req
500 nvme
246 watchdog
501 aac
247 iio
502 mei
248 ptp
503 phantom
249 pps
504 aux
250 dax
505 cmx
251 dimmctl
506 cmm
252 ndctl
507 ttySLP
253 tpm
508 ttyIPWp
254 gpiochip
509 ttySL