[PATCH] KEYS: Replace all non-returning 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 <redacted>
---
 security/keys/request_key_auth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e9735006d0..8f33cd170e42 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c

@@ -178,7 +178,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
 	if (!rka->callout_info)
 		goto error_free_rka;
 	rka->callout_len = callout_len;
-	strlcpy(rka->op, op, sizeof(rka->op));
+	strscpy(rka->op, op, sizeof(rka->op));
 
 	/* see if the calling process is already servicing the key request of
 	 * another process */

On Thu May 18, 2023 at 7:15 AM EEST, Azeem Shaikh wrote:

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

On Thu May 18, 2023 at 7:15 AM EEST, Azeem Shaikh wrote:

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

On Thu, May 18, 2023 at 2:01 PM Jarkko Sakkinen [off-list ref] wrote:

On Thu May 18, 2023 at 7:15 AM EEST, Azeem Shaikh wrote:

quoted

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 <redacted>
---
 security/keys/request_key_auth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e9735006d0..8f33cd170e42 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c

@@ -178,7 +178,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
      if (!rka->callout_info)
              goto error_free_rka;
      rka->callout_len = callout_len;
-     strlcpy(rka->op, op, sizeof(rka->op));
+     strscpy(rka->op, op, sizeof(rka->op));

      /* see if the calling process is already servicing the key request of
       * another process */

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Since you maintain this code Jarkko, are you planning to merge this
into your tree or would you prefer the KSPP folks merge it?

-- 
paul-moore.com

On Sat May 20, 2023 at 12:11 AM EEST, Paul Moore wrote:

On Thu, May 18, 2023 at 2:01 PM Jarkko Sakkinen [off-list ref] wrote:

quoted

On Thu May 18, 2023 at 7:15 AM EEST, Azeem Shaikh wrote:

quoted

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 <redacted>
---
 security/keys/request_key_auth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e9735006d0..8f33cd170e42 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c

@@ -178,7 +178,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
      if (!rka->callout_info)
              goto error_free_rka;
      rka->callout_len = callout_len;
-     strlcpy(rka->op, op, sizeof(rka->op));
+     strscpy(rka->op, op, sizeof(rka->op));

      /* see if the calling process is already servicing the key request of
       * another process */

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Since you maintain this code Jarkko, are you planning to merge this
into your tree or would you prefer the KSPP folks merge it?

I can pick it.

BR, Jarkko

On Wed May 24, 2023 at 5:45 AM EEST, Jarkko Sakkinen wrote:

On Sat May 20, 2023 at 12:11 AM EEST, Paul Moore wrote:

quoted

On Thu, May 18, 2023 at 2:01 PM Jarkko Sakkinen [off-list ref] wrote:

quoted

On Thu May 18, 2023 at 7:15 AM EEST, Azeem Shaikh wrote:

quoted

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 <redacted>
---
 security/keys/request_key_auth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e9735006d0..8f33cd170e42 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c

@@ -178,7 +178,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
      if (!rka->callout_info)
              goto error_free_rka;
      rka->callout_len = callout_len;
-     strlcpy(rka->op, op, sizeof(rka->op));
+     strscpy(rka->op, op, sizeof(rka->op));

      /* see if the calling process is already servicing the key request of
       * another process */

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Since you maintain this code Jarkko, are you planning to merge this
into your tree or would you prefer the KSPP folks merge it?

I can pick it.

Applied.

BR, Jarkko

On Thu, 18 May 2023 04:15:13 +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] KEYS: Replace all non-returning strlcpy with strscpy
      https://git.kernel.org/kees/c/8d27fcac4a08

-- 
Kees Cook