| 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 |
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) >
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
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)
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(-)