[PATCH] mmc: sdio_cis: use strscpy() instead of strcpy() in cistpl_vers_1()

[PATCH] mmc: sdio_cis: use strscpy() instead of strcpy() in cistpl_vers_1()

[Hrushiraj Gandhi]

cistpl_vers_1() copies each NUL-terminated string out of the raw CIS
TPLLV1_INFO data into a single kzalloc()'d blob shared by all of the
strings, using strcpy() with no bound. The strings are already known
to be well-formed within `size` bytes by the counting loop above, so
this isn't currently exploitable, but strcpy()'s lack of any bound is
still worth removing on general principle.

Track the end of the allocated string storage and use strscpy() with
the remaining space as an explicit, always-safe bound instead.

No functional change.

Signed-off-by: Hrushiraj Gandhi <hrushirajg23@gmail.com>
---
 drivers/mmc/core/sdio_cis.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
index afaa6cab1adc..24f670a798e2 100644
--- a/drivers/mmc/core/sdio_cis.c
+++ b/drivers/mmc/core/sdio_cis.c
@@ -27,7 +27,7 @@ static int cistpl_vers_1(struct mmc_card *card, struct sdio_func *func,
 {
 	u8 major_rev, minor_rev;
 	unsigned i, nr_strings;
-	char **buffer, *string;
+	char **buffer, *string, *string_end;
 
 	if (size < 2)
 		return 0;
@@ -57,10 +57,11 @@ static int cistpl_vers_1(struct mmc_card *card, struct sdio_func *func,
 		return -ENOMEM;
 
 	string = (char*)(buffer + nr_strings);
+	string_end = string + size;
 
 	for (i = 0; i < nr_strings; i++) {
 		buffer[i] = string;
-		strcpy(string, buf);
+		strscpy(string, buf, string_end - string);
 		string += strlen(string) + 1;
 		buf += strlen(buf) + 1;
 	}

Re: [PATCH] mmc: sdio_cis: use strscpy() instead of strcpy() in cistpl_vers_1()

[Ulf Hansson]

On Tue, Sep 1, 2026 at 6:41 AM Hrushiraj Gandhi <hrushirajg23@gmail.com> wrote:
>
> cistpl_vers_1() copies each NUL-terminated string out of the raw CIS
> TPLLV1_INFO data into a single kzalloc()'d blob shared by all of the
> strings, using strcpy() with no bound. The strings are already known
> to be well-formed within `size` bytes by the counting loop above, so
> this isn't currently exploitable, but strcpy()'s lack of any bound is
> still worth removing on general principle.
>
> Track the end of the allocated string storage and use strscpy() with
> the remaining space as an explicit, always-safe bound instead.
>
> No functional change.
>
> Signed-off-by: Hrushiraj Gandhi <hrushirajg23@gmail.com>

This doesn't apply on my next branch, please rebase and submit a new version.

Kind regards
Uffe

> ---
>  drivers/mmc/core/sdio_cis.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
> index afaa6cab1adc..24f670a798e2 100644
> --- a/drivers/mmc/core/sdio_cis.c
> +++ b/drivers/mmc/core/sdio_cis.c
> @@ -27,7 +27,7 @@ static int cistpl_vers_1(struct mmc_card *card, struct sdio_func *func,
>  {
>         u8 major_rev, minor_rev;
>         unsigned i, nr_strings;
> -       char **buffer, *string;
> +       char **buffer, *string, *string_end;
>
>         if (size < 2)
>                 return 0;
> @@ -57,10 +57,11 @@ static int cistpl_vers_1(struct mmc_card *card, struct sdio_func *func,
>                 return -ENOMEM;
>
>         string = (char*)(buffer + nr_strings);
> +       string_end = string + size;
>
>         for (i = 0; i < nr_strings; i++) {
>                 buffer[i] = string;
> -               strcpy(string, buf);
> +               strscpy(string, buf, string_end - string);
>                 string += strlen(string) + 1;
>                 buf += strlen(buf) + 1;
>         }