Bjorn Andersson
2022-Mar-11 20:19 UTC
[PATCH v3 11/11] rpmsg: Fix kfree() of static memory on setting driver_override
On Sun 27 Feb 07:53 CST 2022, Krzysztof Kozlowski wrote:> The driver_override field from platform driver should not be initialized > from static memory (string literal) because the core later kfree() it, > for example when driver_override is set via sysfs. > > Use dedicated helper to set driver_override properly. > > Fixes: 950a7388f02b ("rpmsg: Turn name service into a stand alone driver") > Fixes: c0cdc19f84a4 ("rpmsg: Driver for user space endpoint interface") > Cc: <stable at vger.kernel.org> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski at canonical.com>Reviewed-by: Bjorn Andersson <bjorn.andersson at linaro.org> Regards, Bjorn> --- > drivers/rpmsg/rpmsg_core.c | 3 ++- > drivers/rpmsg/rpmsg_internal.h | 13 +++++++++++-- > drivers/rpmsg/rpmsg_ns.c | 14 ++++++++++++-- > include/linux/rpmsg.h | 6 ++++-- > 4 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index d9e612f4f0f2..6e2bf2742973 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -397,7 +397,8 @@ field##_store(struct device *dev, struct device_attribute *attr, \ > const char *buf, size_t sz) \ > { \ > struct rpmsg_device *rpdev = to_rpmsg_device(dev); \ > - char *new, *old; \ > + const char *old; \ > + char *new; \ > \ > new = kstrndup(buf, sz, GFP_KERNEL); \ > if (!new) \ > diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h > index b1245d3ed7c6..31345d6e9a7e 100644 > --- a/drivers/rpmsg/rpmsg_internal.h > +++ b/drivers/rpmsg/rpmsg_internal.h > @@ -92,10 +92,19 @@ int rpmsg_release_channel(struct rpmsg_device *rpdev, > */ > static inline int rpmsg_chrdev_register_device(struct rpmsg_device *rpdev) > { > + int ret; > + > strcpy(rpdev->id.name, "rpmsg_chrdev"); > - rpdev->driver_override = "rpmsg_chrdev"; > + ret = driver_set_override(&rpdev->dev, &rpdev->driver_override, > + "rpmsg_chrdev", strlen("rpmsg_chrdev")); > + if (ret) > + return ret; > + > + ret = rpmsg_register_device(rpdev); > + if (ret) > + kfree(rpdev->driver_override); > > - return rpmsg_register_device(rpdev); > + return ret; > } > > #endif > diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c > index 762ff1ae279f..95a51543f5ad 100644 > --- a/drivers/rpmsg/rpmsg_ns.c > +++ b/drivers/rpmsg/rpmsg_ns.c > @@ -20,12 +20,22 @@ > */ > int rpmsg_ns_register_device(struct rpmsg_device *rpdev) > { > + int ret; > + > strcpy(rpdev->id.name, "rpmsg_ns"); > - rpdev->driver_override = "rpmsg_ns"; > + ret = driver_set_override(&rpdev->dev, &rpdev->driver_override, > + "rpmsg_ns", strlen("rpmsg_ns")); > + if (ret) > + return ret; > + > rpdev->src = RPMSG_NS_ADDR; > rpdev->dst = RPMSG_NS_ADDR; > > - return rpmsg_register_device(rpdev); > + ret = rpmsg_register_device(rpdev); > + if (ret) > + kfree(rpdev->driver_override); > + > + return ret; > } > EXPORT_SYMBOL(rpmsg_ns_register_device); > > diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h > index 02fa9116cd60..20c8cd1cde21 100644 > --- a/include/linux/rpmsg.h > +++ b/include/linux/rpmsg.h > @@ -41,7 +41,9 @@ struct rpmsg_channel_info { > * rpmsg_device - device that belong to the rpmsg bus > * @dev: the device struct > * @id: device id (used to match between rpmsg drivers and devices) > - * @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. > * @src: local address > * @dst: destination address > * @ept: the rpmsg endpoint of this channel > @@ -51,7 +53,7 @@ struct rpmsg_channel_info { > struct rpmsg_device { > struct device dev; > struct rpmsg_device_id id; > - char *driver_override; > + const char *driver_override; > u32 src; > u32 dst; > struct rpmsg_endpoint *ept; > -- > 2.32.0 >