2023-06-21 03:19:17

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH 0/2] scsi: Replace strlcpy with strscpy

This patch series replaces strlcpy in the scsi subsystem wherever trivial
replacement is possible, i.e return value from strlcpy is unused. The patches
themselves are independent of each other and are included as a series for
ease of review.

Azeem Shaikh (2):
scsi: Replace strlcpy with strscpy
scsi: target: tcmu: Replace strlcpy with strscpy

drivers/scsi/ncr53c8xx.c | 2 +-
drivers/target/target_core_user.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

--
2.41.0.162.gfafddb0af9-goog



2023-06-21 03:21:00

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH 1/2] scsi: Replace strlcpy with strscpy

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().

No return values were used, so direct replacement is safe.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89

Signed-off-by: Azeem Shaikh <[email protected]>
---
drivers/scsi/ncr53c8xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ncr53c8xx.c b/drivers/scsi/ncr53c8xx.c
index 4458449c960b..35869b4f9329 100644
--- a/drivers/scsi/ncr53c8xx.c
+++ b/drivers/scsi/ncr53c8xx.c
@@ -4555,7 +4555,7 @@ static void ncr_detach(struct ncb *np)
char inst_name[16];

/* Local copy so we don't access np after freeing it! */
- strlcpy(inst_name, ncr_name(np), sizeof(inst_name));
+ strscpy(inst_name, ncr_name(np), sizeof(inst_name));

printk("%s: releasing host resources\n", ncr_name(np));

--
2.41.0.162.gfafddb0af9-goog


2023-06-21 03:21:44

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH 2/2] scsi: target: tcmu: Replace strlcpy with strscpy

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().

No return values were used, so direct replacement is safe.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89

Signed-off-by: Azeem Shaikh <[email protected]>
---
drivers/target/target_core_user.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 15ffc8d2ac7b..22cc6cac0ba2 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -2820,14 +2820,14 @@ static ssize_t tcmu_dev_config_store(struct config_item *item, const char *page,
pr_err("Unable to reconfigure device\n");
return ret;
}
- strlcpy(udev->dev_config, page, TCMU_CONFIG_LEN);
+ strscpy(udev->dev_config, page, TCMU_CONFIG_LEN);

ret = tcmu_update_uio_info(udev);
if (ret)
return ret;
return count;
}
- strlcpy(udev->dev_config, page, TCMU_CONFIG_LEN);
+ strscpy(udev->dev_config, page, TCMU_CONFIG_LEN);

return count;
}
--
2.41.0.162.gfafddb0af9-goog


2023-06-21 18:21:39

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 2/2] scsi: target: tcmu: Replace strlcpy with strscpy

On Wed, Jun 21, 2023 at 03:00:33AM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
>
> No return values were used, so direct replacement is safe.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-06-21 18:37:25

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 1/2] scsi: Replace strlcpy with strscpy

On Wed, Jun 21, 2023 at 03:00:32AM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
>
> No return values were used, so direct replacement is safe.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-06-22 01:27:43

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 0/2] scsi: Replace strlcpy with strscpy


Azeem,

> This patch series replaces strlcpy in the scsi subsystem wherever
> trivial replacement is possible, i.e return value from strlcpy is
> unused. The patches themselves are independent of each other and are
> included as a series for ease of review.

Applied to 6.5/scsi-staging, thanks!

--
Martin K. Petersen Oracle Linux Engineering

2023-06-29 03:08:45

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 0/2] scsi: Replace strlcpy with strscpy

On Wed, 21 Jun 2023 03:00:31 +0000, Azeem Shaikh wrote:

> This patch series replaces strlcpy in the scsi subsystem wherever trivial
> replacement is possible, i.e return value from strlcpy is unused. The patches
> themselves are independent of each other and are included as a series for
> ease of review.
>
> Azeem Shaikh (2):
> scsi: Replace strlcpy with strscpy
> scsi: target: tcmu: Replace strlcpy with strscpy
>
> [...]

Applied to 6.5/scsi-queue, thanks!

[1/2] scsi: Replace strlcpy with strscpy
https://git.kernel.org/mkp/scsi/c/d1e8a9fbb392
[2/2] scsi: target: tcmu: Replace strlcpy with strscpy
https://git.kernel.org/mkp/scsi/c/4b2e28758daf

--
Martin K. Petersen Oracle Linux Engineering