2022-02-27 18:04:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v3 00/11] Fix broken usage of driver_override (and kfree of static memory)

Hi,

This is a continuation of my old patchset from 2019. [1]
Back then, few drivers set driver_override wrong. I fixed Exynos
in a different way after discussions. QCOM NGD was not fixed
and a new user appeared - IMX SCU.

It seems "char *" in driver_override looks too consty, so we
tend to make a mistake of storing there string literals.

Changes since latest v2
=======================
1. Make all driver_override fields as "const char *", just like SPI
and VDPA. (Mark)
2. Move "count" check to the new helper and add "count" argument. (Michael)
3. Fix typos in docs, patch subject. Extend doc. (Michael, Bjorn)
4. Compare pointers to reduce number of string readings in the helper.
5. Fix clk-imx return value.

Changes since latest v1 (not the old 2019 solution):
====================================================
https://lore.kernel.org/all/[email protected]/
1. Add helper for setting driver_override.
2. Use the helper.

Dependencies (and stable):
==========================
1. All patches, including last three fixes, depend on the first patch
introducing the helper.
2. The last three commits - fixes - are probably not backportable
directly, because of this dependency. I don't know how to express
this dependency here, since stable-kernel-rules.rst mentions only commits as
possible dependencies.

[1] https://lore.kernel.org/all/[email protected]/

Best regards,
Krzysztof

Krzysztof Kozlowski (11):
driver: platform: Add helper for safer setting of driver_override
amba: Use driver_set_override() instead of open-coding
fsl-mc: Use driver_set_override() instead of open-coding
hv: Use driver_set_override() instead of open-coding
PCI: Use driver_set_override() instead of open-coding
s390: cio: Use driver_set_override() instead of open-coding
spi: Use helper for safer setting of driver_override
vdpa: Use helper for safer setting of driver_override
clk: imx: scu: Fix kfree() of static memory on setting driver_override
slimbus: qcom-ngd: Fix kfree() of static memory on setting
driver_override
rpmsg: Fix kfree() of static memory on setting driver_override

drivers/amba/bus.c | 28 +++---------------
drivers/base/driver.c | 51 +++++++++++++++++++++++++++++++++
drivers/base/platform.c | 28 +++---------------
drivers/bus/fsl-mc/fsl-mc-bus.c | 25 +++-------------
drivers/clk/imx/clk-scu.c | 7 ++++-
drivers/hv/vmbus_drv.c | 28 +++---------------
drivers/pci/pci-sysfs.c | 28 +++---------------
drivers/rpmsg/rpmsg_core.c | 3 +-
drivers/rpmsg/rpmsg_internal.h | 13 +++++++--
drivers/rpmsg/rpmsg_ns.c | 14 +++++++--
drivers/s390/cio/cio.h | 7 ++++-
drivers/s390/cio/css.c | 28 +++---------------
drivers/slimbus/qcom-ngd-ctrl.c | 13 ++++++++-
drivers/spi/spi.c | 26 +++--------------
drivers/vdpa/vdpa.c | 29 +++----------------
include/linux/amba/bus.h | 7 ++++-
include/linux/device/driver.h | 2 ++
include/linux/fsl/mc.h | 6 ++--
include/linux/hyperv.h | 7 ++++-
include/linux/pci.h | 7 ++++-
include/linux/platform_device.h | 7 ++++-
include/linux/rpmsg.h | 6 ++--
include/linux/spi/spi.h | 2 ++
include/linux/vdpa.h | 4 ++-
24 files changed, 171 insertions(+), 205 deletions(-)

--
2.32.0


2022-02-27 18:48:47

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v3 04/11] hv: Use driver_set_override() instead of open-coding

Use a helper for seting driver_override to reduce amount of duplicated
code. Make the driver_override field const char, because it is not
modified by the core and it matches other subsystems.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/hv/vmbus_drv.c | 28 ++++------------------------
include/linux/hyperv.h | 7 ++++++-
2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 12a2b37e87f3..a0ff4139c3d2 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -575,31 +575,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct hv_device *hv_dev = device_to_hv_device(dev);
- char *driver_override, *old, *cp;
-
- /* We need to keep extra room for a newline */
- if (count >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, count, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
-
- cp = strchr(driver_override, '\n');
- if (cp)
- *cp = '\0';
-
- device_lock(dev);
- old = hv_dev->driver_override;
- if (strlen(driver_override)) {
- hv_dev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- hv_dev->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;

- kfree(old);
+ ret = driver_set_override(dev, &hv_dev->driver_override, buf, count);
+ if (ret)
+ return ret;

return count;
}
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fe2e0179ed51..beea11874be2 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1257,7 +1257,12 @@ struct hv_device {
u16 device_id;

struct device device;
- char *driver_override; /* Driver name to force a match */
+ /*
+ * Driver name to force a match.
+ * Do not set directly, because core frees it.
+ * Use driver_set_override() to set or clear it.
+ */
+ const char *driver_override;

struct vmbus_channel *channel;
struct kset *channels_kset;
--
2.32.0