Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Fix NULL pointer dereference and use struct_size
@ 2019-01-04 17:13 Gustavo A. R. Silva
  2019-01-04 17:17 ` [PATCH 1/2] ARM: integrator: impd1: fix NULL pointer dereference Gustavo A. R. Silva
  2019-01-04 17:18 ` [PATCH 2/2] ARM: integrator: impd1: use struct_size() in devm_kzalloc() Gustavo A. R. Silva
  0 siblings, 2 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2019-01-04 17:13 UTC (permalink / raw)
  To: Linus Walleij, Russell King
  Cc: linux-kernel, linux-arm-kernel, Gustavo A. R. Silva

Hi,

The first patch in this series fixes a potential NULL pointer
dereference by adding a NULL check. A tag for stable has been
added in this patch.

The second patch promotes the use of struct_size() in devm_kzalloc().

Both issues were detected with the help of Coccinelle.

Thanks

Gustavo A. R. Silva (2):
  ARM: integrator: impd1: fix NULL pointer dereference
  ARM: integrator: impd1: use struct_size() in devm_kzalloc()

 arch/arm/mach-integrator/impd1.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/2] ARM: integrator: impd1: fix NULL pointer dereference
  2019-01-04 17:13 [PATCH 0/2] Fix NULL pointer dereference and use struct_size Gustavo A. R. Silva
@ 2019-01-04 17:17 ` Gustavo A. R. Silva
  2019-01-11 12:22   ` Linus Walleij
  2019-01-04 17:18 ` [PATCH 2/2] ARM: integrator: impd1: use struct_size() in devm_kzalloc() Gustavo A. R. Silva
  1 sibling, 1 reply; 5+ messages in thread
From: Gustavo A. R. Silva @ 2019-01-04 17:17 UTC (permalink / raw)
  To: Russell King, Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, Gustavo A. R. Silva

There is a potential NULL pointer dereference in case devm_kzalloc()
fails and returns NULL.

Fix this by adding a NULL check on lookup.

This issue was detected with the help of Coccinelle.

Fixes: 684284b64aae ("ARM: integrator: add MMCI device to IM-PD1")
Cc: stable@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 arch/arm/mach-integrator/impd1.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c
index a109f6482413..eb0149561be2 100644
--- a/arch/arm/mach-integrator/impd1.c
+++ b/arch/arm/mach-integrator/impd1.c
@@ -392,6 +392,9 @@ static int __ref impd1_probe(struct lm_device *dev)
 			lookup = devm_kzalloc(&dev->dev,
 					      sizeof(*lookup) + 3 * sizeof(struct gpiod_lookup),
 					      GFP_KERNEL);
+			if (!lookup)
+				return -ENOMEM;
+
 			chipname = devm_kstrdup(&dev->dev, devname, GFP_KERNEL);
 			mmciname = kasprintf(GFP_KERNEL, "lm%x:00700", dev->id);
 			lookup->dev_id = mmciname;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/2] ARM: integrator: impd1: use struct_size() in devm_kzalloc()
  2019-01-04 17:13 [PATCH 0/2] Fix NULL pointer dereference and use struct_size Gustavo A. R. Silva
  2019-01-04 17:17 ` [PATCH 1/2] ARM: integrator: impd1: fix NULL pointer dereference Gustavo A. R. Silva
@ 2019-01-04 17:18 ` Gustavo A. R. Silva
  2019-01-05  5:26   ` kbuild test robot
  1 sibling, 1 reply; 5+ messages in thread
From: Gustavo A. R. Silva @ 2019-01-04 17:18 UTC (permalink / raw)
  To: Linus Walleij, Russell King
  Cc: linux-kernel, linux-arm-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    void *entry[];
};

instance = devm_kzalloc(dev, sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = devm_kzalloc(dev, struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 arch/arm/mach-integrator/impd1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c
index eb0149561be2..8ccf01af3817 100644
--- a/arch/arm/mach-integrator/impd1.c
+++ b/arch/arm/mach-integrator/impd1.c
@@ -389,8 +389,8 @@ static int __ref impd1_probe(struct lm_device *dev)
 			char *chipname;
 			char *mmciname;
 
-			lookup = devm_kzalloc(&dev->dev,
-					      sizeof(*lookup) + 3 * sizeof(struct gpiod_lookup),
+			lookup = devm_kzalloc(&dev->deva,
+					      struct_size(lookup, table, 3),
 					      GFP_KERNEL);
 			if (!lookup)
 				return -ENOMEM;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/2] ARM: integrator: impd1: use struct_size() in devm_kzalloc()
  2019-01-04 17:18 ` [PATCH 2/2] ARM: integrator: impd1: use struct_size() in devm_kzalloc() Gustavo A. R. Silva
@ 2019-01-05  5:26   ` kbuild test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2019-01-05  5:26 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Gustavo A. R. Silva, Linus Walleij, linux-kernel, Russell King,
	kbuild-all, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 5241 bytes --]

Hi Gustavo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on arm-soc/for-next]
[also build test ERROR on v4.20 next-20190103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/Fix-NULL-pointer-dereference-and-use-struct_size/20190105-033105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/mach-integrator/impd1.c: In function 'impd1_probe':
>> arch/arm/mach-integrator/impd1.c:392:32: error: 'struct lm_device' has no member named 'deva'; did you mean 'dev'?
       lookup = devm_kzalloc(&dev->deva,
                                   ^~~~
                                   dev

vim +392 arch/arm/mach-integrator/impd1.c

   320	
   321	/*
   322	 * As this module is bool, it is OK to have this as __ref() - no
   323	 * probe calls will be done after the initial system bootup, as devices
   324	 * are discovered as part of the machine startup.
   325	 */
   326	static int __ref impd1_probe(struct lm_device *dev)
   327	{
   328		struct impd1_module *impd1;
   329		int irq_base;
   330		int i;
   331	
   332		if (dev->id != module_id)
   333			return -EINVAL;
   334	
   335		if (!devm_request_mem_region(&dev->dev, dev->resource.start,
   336					     SZ_4K, "LM registers"))
   337			return -EBUSY;
   338	
   339		impd1 = devm_kzalloc(&dev->dev, sizeof(struct impd1_module),
   340				     GFP_KERNEL);
   341		if (!impd1)
   342			return -ENOMEM;
   343	
   344		impd1->base = devm_ioremap(&dev->dev, dev->resource.start, SZ_4K);
   345		if (!impd1->base)
   346			return -ENOMEM;
   347	
   348		integrator_impd1_clk_init(impd1->base, dev->id);
   349	
   350		if (!devm_request_mem_region(&dev->dev,
   351					     dev->resource.start + 0x03000000,
   352					     SZ_4K, "VIC"))
   353			return -EBUSY;
   354	
   355		impd1->vic_base = devm_ioremap(&dev->dev,
   356					       dev->resource.start + 0x03000000,
   357					       SZ_4K);
   358		if (!impd1->vic_base)
   359			return -ENOMEM;
   360	
   361		irq_base = vic_init_cascaded(impd1->vic_base, dev->irq,
   362					     IMPD1_VALID_IRQS, 0);
   363	
   364		lm_set_drvdata(dev, impd1);
   365	
   366		dev_info(&dev->dev, "IM-PD1 found at 0x%08lx\n",
   367			 (unsigned long)dev->resource.start);
   368	
   369		for (i = 0; i < ARRAY_SIZE(impd1_devs); i++) {
   370			struct impd1_device *idev = impd1_devs + i;
   371			struct amba_device *d;
   372			unsigned long pc_base;
   373			char devname[32];
   374			int irq1 = idev->irq[0];
   375			int irq2 = idev->irq[1];
   376	
   377			/* Translate IRQs to IM-PD1 local numberspace */
   378			if (irq1)
   379				irq1 += irq_base;
   380			if (irq2)
   381				irq2 += irq_base;
   382	
   383			pc_base = dev->resource.start + idev->offset;
   384			snprintf(devname, 32, "lm%x:%5.5lx", dev->id, idev->offset >> 12);
   385	
   386			/* Add GPIO descriptor lookup table for the PL061 block */
   387			if (idev->offset == 0x00400000) {
   388				struct gpiod_lookup_table *lookup;
   389				char *chipname;
   390				char *mmciname;
   391	
 > 392				lookup = devm_kzalloc(&dev->deva,
   393						      struct_size(lookup, table, 3),
   394						      GFP_KERNEL);
   395				if (!lookup)
   396					return -ENOMEM;
   397	
   398				chipname = devm_kstrdup(&dev->dev, devname, GFP_KERNEL);
   399				mmciname = kasprintf(GFP_KERNEL, "lm%x:00700", dev->id);
   400				lookup->dev_id = mmciname;
   401				/*
   402				 * Offsets on GPIO block 1:
   403				 * 3 = MMC WP (write protect)
   404				 * 4 = MMC CD (card detect)
   405				 *
   406				 * Offsets on GPIO block 2:
   407				 * 0 = Up key
   408				 * 1 = Down key
   409				 * 2 = Left key
   410				 * 3 = Right key
   411				 * 4 = Key lower left
   412				 * 5 = Key lower right
   413				 */
   414				/* We need the two MMCI GPIO entries */
   415				lookup->table[0].chip_label = chipname;
   416				lookup->table[0].chip_hwnum = 3;
   417				lookup->table[0].con_id = "wp";
   418				lookup->table[1].chip_label = chipname;
   419				lookup->table[1].chip_hwnum = 4;
   420				lookup->table[1].con_id = "cd";
   421				lookup->table[1].flags = GPIO_ACTIVE_LOW;
   422				gpiod_add_lookup_table(lookup);
   423			}
   424	
   425			d = amba_ahb_device_add_res(&dev->dev, devname, pc_base, SZ_4K,
   426						    irq1, irq2,
   427						    idev->platform_data, idev->id,
   428						    &dev->resource);
   429			if (IS_ERR(d)) {
   430				dev_err(&dev->dev, "unable to register device: %ld\n", PTR_ERR(d));
   431				continue;
   432			}
   433		}
   434	
   435		return 0;
   436	}
   437	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67771 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/2] ARM: integrator: impd1: fix NULL pointer dereference
  2019-01-04 17:17 ` [PATCH 1/2] ARM: integrator: impd1: fix NULL pointer dereference Gustavo A. R. Silva
@ 2019-01-11 12:22   ` Linus Walleij
  0 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2019-01-11 12:22 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Nicholas Mc Guire
  Cc: Russell King, Linux ARM, linux-kernel

On Fri, Jan 4, 2019 at 6:18 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:

> There is a potential NULL pointer dereference in case devm_kzalloc()
> fails and returns NULL.
>
> Fix this by adding a NULL check on lookup.
>
> This issue was detected with the help of Coccinelle.
>
> Fixes: 684284b64aae ("ARM: integrator: add MMCI device to IM-PD1")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

I already have a patch fixing this from Nicholas Mc Guire, just that I
forgot to push it upstream. I'll fix, sorry for not applying his patch
earlier.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-04 17:13 [PATCH 0/2] Fix NULL pointer dereference and use struct_size Gustavo A. R. Silva
2019-01-04 17:17 ` [PATCH 1/2] ARM: integrator: impd1: fix NULL pointer dereference Gustavo A. R. Silva
2019-01-11 12:22   ` Linus Walleij
2019-01-04 17:18 ` [PATCH 2/2] ARM: integrator: impd1: use struct_size() in devm_kzalloc() Gustavo A. R. Silva
2019-01-05  5:26   ` kbuild test robot

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox