| From 98e505fce42c58443bbb281db8c89525ba80ba1a Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 4 Apr 2023 18:21:09 +0100 |
| Subject: of: Fix modalias string generation |
| |
| 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.org |
| 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 da81583920103..7fb870097a849 100644 |
| --- a/drivers/of/device.c |
| +++ b/drivers/of/device.c |
| @@ -246,12 +246,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); |
| |
| -- |
| 2.39.2 |
| |