From: Miquel Raynal miquel.raynal@bootlin.com
[ Upstream commit b19a4266c52de78496fe40f0b37580a3b762e67d ]
The helper generating an OF based modalias (of_device_get_modalias()) works fine, but due to the use of snprintf() internally it needs a buffer one byte longer than what should be needed just for the entire string (excluding the '\0'). Most users of this helper are sysfs hooks providing the modalias string to users. They all provide a PAGE_SIZE buffer which is way above the number of bytes required to fit the modalias string and hence do not suffer from this issue.
There is another user though, of_device_request_module(), which is only called by drivers/usb/common/ulpi.c. This request module function is faulty, but maybe because in most cases there is an alternative, ULPI driver users have not noticed it.
In this function, of_device_get_modalias() is called twice. The first time without buffer just to get the number of bytes required by the modalias string (excluding the null byte), and a second time, after buffer allocation, to fill the buffer. The allocation asks for an additional byte, in order to store the trailing '\0'. However, the buffer *length* provided to of_device_get_modalias() excludes this extra byte. The internal use of snprintf() with a length that is exactly the number of bytes to be written has the effect of using the last available byte to store a '\0', which then smashes the last character of the modalias string.
Provide the actual size of the buffer to of_device_get_modalias() to fix this issue.
Note: the "str[size - 1] = '\0';" line is not really needed as snprintf will anyway end the string with a null byte, but there is a possibility that this function might be called on a struct device_node without compatible, in this case snprintf() would not be executed. So we keep it just to avoid possible unbounded strings.
Cc: Stephen Boyd sboyd@kernel.org Cc: Peter Chen peter.chen@kernel.org Fixes: 9c829c097f2f ("of: device: Support loading a module with OF based modalias") Signed-off-by: Miquel Raynal miquel.raynal@bootlin.com Reviewed-by: Rob Herring robh@kernel.org Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org Link: https://lore.kernel.org/r/20230404172148.82422-2-srinivas.kandagatla@linaro.... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/of/device.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c index 8cefe5a7d04e2..ce225d2590b54 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -297,12 +297,15 @@ int of_device_request_module(struct device *dev) if (size < 0) return size;
- str = kmalloc(size + 1, GFP_KERNEL); + /* Reserve an additional byte for the trailing '\0' */ + size++; + + str = kmalloc(size, GFP_KERNEL); if (!str) return -ENOMEM;
of_device_get_modalias(dev, str, size); - str[size] = '\0'; + str[size - 1] = '\0'; ret = request_module(str); kfree(str);