diff mbox series

[v2] mmc: core: use sysfs_emit() in #define sdio_info_attr()

Message ID b0d9cdda-33f3-1eb0-a76e-26125089e5c5@omp.ru
State New
Headers show
Series [v2] mmc: core: use sysfs_emit() in #define sdio_info_attr() | expand

Commit Message

Sergey Shtylyov Jan. 27, 2022, 9:01 p.m. UTC
sprintf() (still used in #define sdio_info_attr()) is vulnerable to the
buffer overflow.  Use the new-fangled sysfs_emit() instead.

While at it, add spaces around the minus sign...

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

---
This patch is against the 'next' branch of Ulf Hansson's 'mmc.git' repo.

Changes in version 2:
- added the same macro change in drivers/mmc/corfe/sdio[_bus].c;
- updated the patch subject.

 drivers/mmc/core/sd.c       |    4 ++--
 drivers/mmc/core/sdio.c     |    4 ++--
 drivers/mmc/core/sdio_bus.c |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

Comments

Ulf Hansson Jan. 31, 2022, 4:14 p.m. UTC | #1
On Thu, 27 Jan 2022 at 22:01, Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
>
> sprintf() (still used in #define sdio_info_attr()) is vulnerable to the
> buffer overflow.  Use the new-fangled sysfs_emit() instead.
>
> While at it, add spaces around the minus sign...
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

Thanks for fixing this!

However, rather than applying these piece by piece, would you mind
squashing these "sysfs_emit" fixes into one patch for the mmc core? It
would be easier for me to handle - and it should still be an easy
backport, I think.

Kind regards
Uffe

>
> ---
> This patch is against the 'next' branch of Ulf Hansson's 'mmc.git' repo.
>
> Changes in version 2:
> - added the same macro change in drivers/mmc/corfe/sdio[_bus].c;
> - updated the patch subject.
>
>  drivers/mmc/core/sd.c       |    4 ++--
>  drivers/mmc/core/sdio.c     |    4 ++--
>  drivers/mmc/core/sdio_bus.c |    4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> Index: mmc/drivers/mmc/core/sd.c
> ===================================================================
> --- mmc.orig/drivers/mmc/core/sd.c
> +++ mmc/drivers/mmc/core/sd.c
> @@ -735,9 +735,9 @@ static ssize_t info##num##_show(struct d
>                                                                                                 \
>         if (num > card->num_info)                                                               \
>                 return -ENODATA;                                                                \
> -       if (!card->info[num-1][0])                                                              \
> +       if (!card->info[num - 1][0])                                                            \
>                 return 0;                                                                       \
> -       return sprintf(buf, "%s\n", card->info[num-1]);                                         \
> +       return sysfs_emit(buf, "%s\n", card->info[num - 1]);                                    \
>  }                                                                                              \
>  static DEVICE_ATTR_RO(info##num)
>
> Index: mmc/drivers/mmc/core/sdio.c
> ===================================================================
> --- mmc.orig/drivers/mmc/core/sdio.c
> +++ mmc/drivers/mmc/core/sdio.c
> @@ -40,9 +40,9 @@ static ssize_t info##num##_show(struct d
>                                                                                                 \
>         if (num > card->num_info)                                                               \
>                 return -ENODATA;                                                                \
> -       if (!card->info[num-1][0])                                                              \
> +       if (!card->info[num - 1][0])                                                            \
>                 return 0;                                                                       \
> -       return sprintf(buf, "%s\n", card->info[num-1]);                                         \
> +       return sysfs_emit(buf, "%s\n", card->info[num - 1]);                                    \
>  }                                                                                              \
>  static DEVICE_ATTR_RO(info##num)
>
> Index: mmc/drivers/mmc/core/sdio_bus.c
> ===================================================================
> --- mmc.orig/drivers/mmc/core/sdio_bus.c
> +++ mmc/drivers/mmc/core/sdio_bus.c
> @@ -52,9 +52,9 @@ static ssize_t info##num##_show(struct d
>                                                                                                 \
>         if (num > func->num_info)                                                               \
>                 return -ENODATA;                                                                \
> -       if (!func->info[num-1][0])                                                              \
> +       if (!func->info[num - 1][0])                                                            \
>                 return 0;                                                                       \
> -       return sprintf(buf, "%s\n", func->info[num-1]);                                         \
> +       return sysfs_emit(buf, "%s\n", func->info[num - 1]);                                            \
>  }                                                                                              \
>  static DEVICE_ATTR_RO(info##num)
>
Sergey Shtylyov Feb. 7, 2022, 8:08 p.m. UTC | #2
Hello!

On 1/31/22 7:14 PM, Ulf Hansson wrote:

>> sprintf() (still used in #define sdio_info_attr()) is vulnerable to the
>> buffer overflow.  Use the new-fangled sysfs_emit() instead.
>>
>> While at it, add spaces around the minus sign...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> Thanks for fixing this!
> 
> However, rather than applying these piece by piece, would you mind
> squashing these "sysfs_emit" fixes into one patch for the mmc core? It
> would be easier for me to handle - and it should still be an easy
> backport, I think.

   OK, just posted!
   Note that in some place I had to reindent the entire function (indented
with spaces) because checkpatch.pl was bitching...

> Kind regards
> Uffe

[...]

MBR, Sergey
diff mbox series

Patch

Index: mmc/drivers/mmc/core/sd.c
===================================================================
--- mmc.orig/drivers/mmc/core/sd.c
+++ mmc/drivers/mmc/core/sd.c
@@ -735,9 +735,9 @@  static ssize_t info##num##_show(struct d
 												\
 	if (num > card->num_info)								\
 		return -ENODATA;								\
-	if (!card->info[num-1][0])								\
+	if (!card->info[num - 1][0])								\
 		return 0;									\
-	return sprintf(buf, "%s\n", card->info[num-1]);						\
+	return sysfs_emit(buf, "%s\n", card->info[num - 1]);					\
 }												\
 static DEVICE_ATTR_RO(info##num)
 
Index: mmc/drivers/mmc/core/sdio.c
===================================================================
--- mmc.orig/drivers/mmc/core/sdio.c
+++ mmc/drivers/mmc/core/sdio.c
@@ -40,9 +40,9 @@  static ssize_t info##num##_show(struct d
 												\
 	if (num > card->num_info)								\
 		return -ENODATA;								\
-	if (!card->info[num-1][0])								\
+	if (!card->info[num - 1][0])								\
 		return 0;									\
-	return sprintf(buf, "%s\n", card->info[num-1]);						\
+	return sysfs_emit(buf, "%s\n", card->info[num - 1]);					\
 }												\
 static DEVICE_ATTR_RO(info##num)
 
Index: mmc/drivers/mmc/core/sdio_bus.c
===================================================================
--- mmc.orig/drivers/mmc/core/sdio_bus.c
+++ mmc/drivers/mmc/core/sdio_bus.c
@@ -52,9 +52,9 @@  static ssize_t info##num##_show(struct d
 												\
 	if (num > func->num_info)								\
 		return -ENODATA;								\
-	if (!func->info[num-1][0])								\
+	if (!func->info[num - 1][0])								\
 		return 0;									\
-	return sprintf(buf, "%s\n", func->info[num-1]);						\
+	return sysfs_emit(buf, "%s\n", func->info[num - 1]);						\
 }												\
 static DEVICE_ATTR_RO(info##num)