From 92743071464fca5acbbe812d9a0d88de3eaaad36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 21 Jun 2021 22:16:51 +0200 Subject: [PATCH 1/4] firmware: arm_ffa: Ensure drivers provide a probe function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bus probe callback calls the driver callback without further checking. Better be safe than sorry and refuse registration of a driver without a probe function to prevent a NULL pointer exception. Link: https://lore.kernel.org/r/20210621201652.127611-1-u.kleine-koenig@pengutronix.de Fixes: e781858488b9 ("firmware: arm_ffa: Add initial FFA bus support for device enumeration") Signed-off-by: Uwe Kleine-König Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/bus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c index 83166e02b191..d2cc24319626 100644 --- a/drivers/firmware/arm_ffa/bus.c +++ b/drivers/firmware/arm_ffa/bus.c @@ -99,6 +99,9 @@ int ffa_driver_register(struct ffa_driver *driver, struct module *owner, { int ret; + if (!driver->probe) + return -EINVAL; + driver->driver.bus = &ffa_bus_type; driver->driver.name = driver->name; driver->driver.owner = owner; From e362547addc39e4bb18ad5bdfd59ce4d512d0c08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 21 Jun 2021 22:16:52 +0200 Subject: [PATCH 2/4] firmware: arm_ffa: Simplify probe function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the driver core calls the probe callback it already checked that the devices match, so there is no need to call the match callback again. Link: https://lore.kernel.org/r/20210621201652.127611-2-u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/bus.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c index d2cc24319626..00fe595a5bc8 100644 --- a/drivers/firmware/arm_ffa/bus.c +++ b/drivers/firmware/arm_ffa/bus.c @@ -46,9 +46,6 @@ static int ffa_device_probe(struct device *dev) struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver); struct ffa_device *ffa_dev = to_ffa_dev(dev); - if (!ffa_device_match(dev, dev->driver)) - return -ENODEV; - return ffa_drv->probe(ffa_dev); } From ba684a31d3626c86cd9097e12d6ed57d224d077d Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 22 Jun 2021 17:22:02 +0100 Subject: [PATCH 3/4] firmware: arm_ffa: Fix the comment style clang produces the following warning: drivers/firmware/arm_ffa/driver.c:123: warning: expecting prototype for FF(). Prototype was for FFA_PAGE_SIZE() instead This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst Fix the same by removing the kernel-doc style comment here. Link: https://lore.kernel.org/r/20210622162202.3485866-1-sudeep.holla@arm.com Reported-by: kernel test robot Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index b1edb4b2e94a..88b822575ac4 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -120,7 +120,7 @@ #define PACK_TARGET_INFO(s, r) \ (FIELD_PREP(SENDER_ID_MASK, (s)) | FIELD_PREP(RECEIVER_ID_MASK, (r))) -/** +/* * FF-A specification mentions explicitly about '4K pages'. This should * not be confused with the kernel PAGE_SIZE, which is the translation * granule kernel is configured and may be one among 4K, 16K and 64K. From dd925db6f07556061c11ab1fbfa4a0145ae6b438 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 7 Jul 2021 14:47:39 +0100 Subject: [PATCH 4/4] firmware: arm_ffa: Fix a possible ffa_linux_errmap buffer overflow The ffa_linux_errmap buffer access index is supposed to range from 0-8 but it ranges from 1-9 instead. It reads one element out of bounds. It also changes the success into -EINVAL though ffa_to_linux_errno is never used in case of success, it is expected to work for success case too. It is slightly confusing code as the negative of the error code is used as index to the buffer. Fix it by negating it at the start and make it more readable. Link: https://lore.kernel.org/r/20210707134739.1869481-1-sudeep.holla@arm.com Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 88b822575ac4..c9fb56afbcb4 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -149,8 +149,10 @@ static const int ffa_linux_errmap[] = { static inline int ffa_to_linux_errno(int errno) { - if (errno < FFA_RET_SUCCESS && errno >= -ARRAY_SIZE(ffa_linux_errmap)) - return ffa_linux_errmap[-errno]; + int err_idx = -errno; + + if (err_idx >= 0 && err_idx < ARRAY_SIZE(ffa_linux_errmap)) + return ffa_linux_errmap[err_idx]; return -EINVAL; }