[PATCH v2] IOMUX: Fix buffer overflow in iomux_replace_device()

Use of strcat() against an uninitialized buffer would lead to buffer overflow. This patch fixes it.
Signed-off-by: Yuichiro Goto goto@k-tech.co.jp Cc: Peter Robinson pbrobinson@gmail.com Cc: Andy Shevchenko andriy.shevchenko@linux.intel.com Cc: Nicolas Saenz Julienne nsaenzjulienne@suse.de ---
Changes for v2: - Add "IOMUX" in title
common/iomux.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/common/iomux.c b/common/iomux.c index b9088aa3b5..27baca6931 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -158,7 +158,10 @@ int iomux_replace_device(const int console, const char *old, const char *new) return -ENOMEM; }
- strcat(tmp, ","); + if (!arg) + strcpy(tmp, ","); + else + strcat(tmp, ","); strcat(tmp, name);
arg = tmp;

On Fri, Apr 23, 2021 at 05:08:05PM +0900, Yuichiro Goto wrote:
Use of strcat() against an uninitialized buffer would lead to buffer overflow. This patch fixes it.
Thanks for report!
Can you also add a Fixes tag?
Signed-off-by: Yuichiro Goto goto@k-tech.co.jp Cc: Peter Robinson pbrobinson@gmail.com Cc: Andy Shevchenko andriy.shevchenko@linux.intel.com Cc: Nicolas Saenz Julienne nsaenzjulienne@suse.de
Changes for v2:
- Add "IOMUX" in title
common/iomux.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/common/iomux.c b/common/iomux.c index b9088aa3b5..27baca6931 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -158,7 +158,10 @@ int iomux_replace_device(const int console, const char *old, const char *new) return -ENOMEM; }
strcat(tmp, ",");
if (!arg)
strcpy(tmp, ",");
else
strcat(tmp, name);strcat(tmp, ",");
It would be better to have it like
if (arg) { strcat(tmp, ","); strcat(tmp, name); } else strcpy(tmp, name);
arg = tmp;
Peter, does it fix your issue seen on ARMv7?

On Fri, Apr 23, 2021 at 10:09 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, Apr 23, 2021 at 05:08:05PM +0900, Yuichiro Goto wrote:
Use of strcat() against an uninitialized buffer would lead to buffer overflow. This patch fixes it.
Thanks for report!
Can you also add a Fixes tag?
Signed-off-by: Yuichiro Goto goto@k-tech.co.jp Cc: Peter Robinson pbrobinson@gmail.com Cc: Andy Shevchenko andriy.shevchenko@linux.intel.com Cc: Nicolas Saenz Julienne nsaenzjulienne@suse.de
Changes for v2:
- Add "IOMUX" in title
common/iomux.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/common/iomux.c b/common/iomux.c index b9088aa3b5..27baca6931 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -158,7 +158,10 @@ int iomux_replace_device(const int console, const char *old, const char *new) return -ENOMEM; }
strcat(tmp, ",");
if (!arg)
strcpy(tmp, ",");
else
strcat(tmp, ","); strcat(tmp, name);
It would be better to have it like
if (arg) { strcat(tmp, ","); strcat(tmp, name); } else strcpy(tmp, name);
arg = tmp;
Peter, does it fix your issue seen on ARMv7?
-- With Best Regards, Andy Shevchenko
Hi Andy,
Thank you for your review. I submitted the patch v3 based on you suggestions.
participants (2)
-
Andy Shevchenko
-
Yuichiro Goto