[PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy

Azeem Shaikh posted 1 patch 2 years, 8 months ago
drivers/w1/masters/sgi_w1.c |    2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
Posted by Azeem Shaikh 2 years, 8 months ago
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 <azeemshaikh38@gmail.com>
---
 drivers/w1/masters/sgi_w1.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/masters/sgi_w1.c b/drivers/w1/masters/sgi_w1.c
index e8c7fa68d3cc..d7fbc3c146e1 100644
--- a/drivers/w1/masters/sgi_w1.c
+++ b/drivers/w1/masters/sgi_w1.c
@@ -93,7 +93,7 @@ static int sgi_w1_probe(struct platform_device *pdev)
 
 	pdata = dev_get_platdata(&pdev->dev);
 	if (pdata) {
-		strlcpy(sdev->dev_id, pdata->dev_id, sizeof(sdev->dev_id));
+		strscpy(sdev->dev_id, pdata->dev_id, sizeof(sdev->dev_id));
 		sdev->bus_master.dev_id = sdev->dev_id;
 	}
Re: [PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
Posted by Krzysztof Kozlowski 2 years, 8 months ago
On 23/05/2023 04:20, 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

This was already fixed. Please work on linux-next.

https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-w1.git/commit/?h=for-next&id=5dfd3c73ff81618fee0ef682b6fd7779863f41e4

Best regards,
Krzysztof
Re: [PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
Posted by Kees Cook 2 years, 8 months ago
On Tue, 23 May 2023 02:20:23 +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.
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
      https://git.kernel.org/kees/c/4e4424b20cc4

-- 
Kees Cook
Re: [PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
Posted by Krzysztof Kozlowski 2 years, 8 months ago
On 31/05/2023 01:06, Kees Cook wrote:
> On Tue, 23 May 2023 02:20:23 +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.
>>
>> [...]
> 
> Applied to for-next/hardening, thanks!
> 
> [1/1] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
>       https://git.kernel.org/kees/c/4e4424b20cc4

Please drop. This was already fixed and is in linux-next since almost a
month:

https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-w1.git/commit/?h=for-next&id=5dfd3c73ff81618fee0ef682b6fd7779863f41e4

> 

Best regards,
Krzysztof
Re: [PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
Posted by Kees Cook 2 years, 8 months ago
On Thu, Jun 01, 2023 at 07:03:41PM +0200, Krzysztof Kozlowski wrote:
> On 31/05/2023 01:06, Kees Cook wrote:
> > On Tue, 23 May 2023 02:20:23 +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.
> >>
> >> [...]
> > 
> > Applied to for-next/hardening, thanks!
> > 
> > [1/1] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
> >       https://git.kernel.org/kees/c/4e4424b20cc4
> 
> Please drop. This was already fixed and is in linux-next since almost a
> month:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-w1.git/commit/?h=for-next&id=5dfd3c73ff81618fee0ef682b6fd7779863f41e4

Thanks! Dropped.

-- 
Kees Cook
Re: [PATCH] w1: sgi_w1: Replace all non-returning strlcpy with strscpy
Posted by Kees Cook 2 years, 8 months ago
On Tue, May 23, 2023 at 02:20:23AM +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 <azeemshaikh38@gmail.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook