diff mbox series

[RFC] gpio: pl061: handle failed allocations

Message ID 1543665438-26756-1-git-send-email-hofrat@osadl.org (mailing list archive)
State Mainlined, archived
Commit df209c43a0e8258e096fb722dfbdae4f0dd13fde
Headers show
Series [RFC] gpio: pl061: handle failed allocations | expand

Commit Message

Nicholas Mc Guire Dec. 1, 2018, 11:57 a.m. UTC
devm_kzalloc(), devm_kstrdup() and devm_kasprintf() all can
fail internal allocation and return NULL. Using any of the assigned
objects without checking is not safe. As this is early in the boot
phase and these allocations really should not fail, any failure here
is probably an indication of a more serious issue so it makes little
sense to try and rollback the previous allocated resources or try to
continue;  but rather the probe function is simply exited with -ENOMEM.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Fixes: 684284b64aae ("ARM: integrator: add MMCI device to IM-PD1")
---

devm_kstrdup() issue found by experimental coccinelle script, the rest during
review of the reported issue.

Q: Not sure if brute force returning here without rollback of previously
   allocated resources is acceptable - but I guess if any of those
   allocations failed the system is doomed anyway so trying a  continue;
   here does not seem sensible to me.

While at it switched the kasprintf to the managed API variant - or should
this be done in a separate patch ?

Patch was compile tested with: multi_v4t_defconfig (implies
INTEGRATOR_IMPD1=y)

Patch is against 4.20-rc4 (localversion-next is next-20181130)

 arch/arm/mach-integrator/impd1.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Linus Walleij Dec. 7, 2018, 10:14 a.m. UTC | #1
On Sat, Dec 1, 2018 at 1:03 PM Nicholas Mc Guire <hofrat@osadl.org> wrote:

> devm_kzalloc(), devm_kstrdup() and devm_kasprintf() all can
> fail internal allocation and return NULL. Using any of the assigned
> objects without checking is not safe. As this is early in the boot
> phase and these allocations really should not fail, any failure here
> is probably an indication of a more serious issue so it makes little
> sense to try and rollback the previous allocated resources or try to
> continue;  but rather the probe function is simply exited with -ENOMEM.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> Fixes: 684284b64aae ("ARM: integrator: add MMCI device to IM-PD1")

Looks good to me so patch applied to my Integrator tree
for fixes.

Yours,
Liinus Walleij
diff mbox series

Patch

diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c
index a109f648..0f916c2 100644
--- a/arch/arm/mach-integrator/impd1.c
+++ b/arch/arm/mach-integrator/impd1.c
@@ -393,7 +393,11 @@  static int __ref impd1_probe(struct lm_device *dev)
 					      sizeof(*lookup) + 3 * sizeof(struct gpiod_lookup),
 					      GFP_KERNEL);
 			chipname = devm_kstrdup(&dev->dev, devname, GFP_KERNEL);
-			mmciname = kasprintf(GFP_KERNEL, "lm%x:00700", dev->id);
+			mmciname = devm_kasprintf(&dev->dev, GFP_KERNEL,
+						  "lm%x:00700", dev->id);
+			if (!lookup || !chipname || !mmciname)
+				return -ENOMEM;
+
 			lookup->dev_id = mmciname;
 			/*
 			 * Offsets on GPIO block 1: