| Message ID | 20220312132856.65163-1-krzysztof.kozlowski@canonical.com |
|---|---|
| Headers | show |
| Series | Fix broken usage of driver_override (and kfree of static memory) | expand |
On Sat, Mar 12, 2022 at 02:28:53PM +0100, Krzysztof Kozlowski wrote: > Use a helper to set driver_override to reduce amount of duplicated code. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > drivers/vdpa/vdpa.c | 29 ++++------------------------- > include/linux/vdpa.h | 4 +++- > 2 files changed, 7 insertions(+), 26 deletions(-) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index 1ea525433a5c..2dabed1df35c 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -77,32 +77,11 @@ static ssize_t driver_override_store(struct device *dev, > const char *buf, size_t count) > { > struct vdpa_device *vdev = dev_to_vdpa(dev); > - const char *driver_override, *old; > - char *cp; > + int ret; > > - /* 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 = vdev->driver_override; > - if (strlen(driver_override)) { > - vdev->driver_override = driver_override; > - } else { > - kfree(driver_override); > - vdev->driver_override = NULL; > - } > - device_unlock(dev); > - > - kfree(old); > + ret = driver_set_override(dev, &vdev->driver_override, buf, count); > + if (ret) > + return ret; > > return count; > } > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 721089bb4c84..37117404660e 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -64,7 +64,9 @@ struct vdpa_mgmt_dev; > * struct vdpa_device - representation of a vDPA device > * @dev: underlying device > * @dma_dev: the actual device that is performing DMA > - * @driver_override: driver name to force a match > + * @driver_override: driver name to force a match; do not set directly, > + * because core frees it; use driver_set_override() to > + * set or clear it. > * @config: the configuration ops for this device. > * @cf_mutex: Protects get and set access to configuration layout. > * @index: device index > -- > 2.32.0
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 v3 ======================= 1. Wrap comments, extend comment in driver_set_override() about newline. 2. Minor commit msg fixes. 3. Add tags. 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/708eabb1-7b35-d525-d4c3-451d4a3de84f@rasmusvillemoes.dk/ 1. Add helper for setting driver_override. 2. Use the helper. Dependencies, merging and stable ================================ 1. All patches, including last three fixes, depend on the first patch introducing the helper. 2. Merging idea: everything via driver core (Greg KH?). 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/1550484960-2392-3-git-send-email-krzk@kernel.org/ 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 | 56 +++++++++++++++++++++++++++++++++ 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 | 6 +++- 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 | 6 +++- include/linux/device/driver.h | 2 ++ include/linux/fsl/mc.h | 6 ++-- include/linux/hyperv.h | 6 +++- include/linux/pci.h | 6 +++- include/linux/platform_device.h | 6 +++- include/linux/rpmsg.h | 6 ++-- include/linux/spi/spi.h | 2 ++ include/linux/vdpa.h | 4 ++- 24 files changed, 171 insertions(+), 205 deletions(-)