* [PATCH] netfilter: ipset: Replace strlcpy with strscpy
@ 2023-06-13 0:34 Azeem Shaikh
2023-06-13 19:26 ` Kees Cook
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Azeem Shaikh @ 2023-06-13 0:34 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: linux-hardening, Azeem Shaikh, netfilter-devel, coreteam,
linux-kernel, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev
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().
Direct replacement is safe here since return value from all
callers of STRLCPY macro were ignored.
[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>
---
net/netfilter/ipset/ip_set_hash_netiface.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 031073286236..95aeb31c60e0 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -40,7 +40,7 @@ MODULE_ALIAS("ip_set_hash:net,iface");
#define IP_SET_HASH_WITH_MULTI
#define IP_SET_HASH_WITH_NET0
-#define STRLCPY(a, b) strlcpy(a, b, IFNAMSIZ)
+#define STRSCPY(a, b) strscpy(a, b, IFNAMSIZ)
/* IPv4 variant */
@@ -182,11 +182,11 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
if (!eiface)
return -EINVAL;
- STRLCPY(e.iface, eiface);
+ STRSCPY(e.iface, eiface);
e.physdev = 1;
#endif
} else {
- STRLCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
+ STRSCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
}
if (strlen(e.iface) == 0)
@@ -400,11 +400,11 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
if (!eiface)
return -EINVAL;
- STRLCPY(e.iface, eiface);
+ STRSCPY(e.iface, eiface);
e.physdev = 1;
#endif
} else {
- STRLCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
+ STRSCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
}
if (strlen(e.iface) == 0)
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] netfilter: ipset: Replace strlcpy with strscpy
2023-06-13 0:34 [PATCH] netfilter: ipset: Replace strlcpy with strscpy Azeem Shaikh
@ 2023-06-13 19:26 ` Kees Cook
2023-06-14 12:10 ` Simon Horman
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2023-06-13 19:26 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
linux-hardening, netfilter-devel, coreteam, linux-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
On Tue, Jun 13, 2023 at 12:34:37AM +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().
>
> Direct replacement is safe here since return value from all
> callers of STRLCPY macro were ignored.
Yeah, the macro name is probably not super helpful here. It seems like
it should have originally be named IFNAME_CPY or something.
>
> [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>
But, regardless:
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] netfilter: ipset: Replace strlcpy with strscpy
2023-06-13 0:34 [PATCH] netfilter: ipset: Replace strlcpy with strscpy Azeem Shaikh
2023-06-13 19:26 ` Kees Cook
@ 2023-06-14 12:10 ` Simon Horman
2023-06-16 10:51 ` Jozsef Kadlecsik
2023-06-20 20:28 ` Kees Cook
3 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2023-06-14 12:10 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
linux-hardening, netfilter-devel, coreteam, linux-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
On Tue, Jun 13, 2023 at 12:34:37AM +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().
>
> Direct replacement is safe here since return value from all
> callers of STRLCPY macro were ignored.
>
> [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: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] netfilter: ipset: Replace strlcpy with strscpy
2023-06-13 0:34 [PATCH] netfilter: ipset: Replace strlcpy with strscpy Azeem Shaikh
2023-06-13 19:26 ` Kees Cook
2023-06-14 12:10 ` Simon Horman
@ 2023-06-16 10:51 ` Jozsef Kadlecsik
2023-06-20 20:28 ` Kees Cook
3 siblings, 0 replies; 5+ messages in thread
From: Jozsef Kadlecsik @ 2023-06-16 10:51 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Pablo Neira Ayuso, Florian Westphal, linux-hardening,
netfilter-devel, coreteam, linux-kernel, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev
On Tue, 13 Jun 2023, 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().
>
> Direct replacement is safe here since return value from all
> callers of STRLCPY macro were ignored.
>
> [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>
Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Best regards,
Jozsef
> ---
> net/netfilter/ipset/ip_set_hash_netiface.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
> index 031073286236..95aeb31c60e0 100644
> --- a/net/netfilter/ipset/ip_set_hash_netiface.c
> +++ b/net/netfilter/ipset/ip_set_hash_netiface.c
> @@ -40,7 +40,7 @@ MODULE_ALIAS("ip_set_hash:net,iface");
> #define IP_SET_HASH_WITH_MULTI
> #define IP_SET_HASH_WITH_NET0
>
> -#define STRLCPY(a, b) strlcpy(a, b, IFNAMSIZ)
> +#define STRSCPY(a, b) strscpy(a, b, IFNAMSIZ)
>
> /* IPv4 variant */
>
> @@ -182,11 +182,11 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
>
> if (!eiface)
> return -EINVAL;
> - STRLCPY(e.iface, eiface);
> + STRSCPY(e.iface, eiface);
> e.physdev = 1;
> #endif
> } else {
> - STRLCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
> + STRSCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
> }
>
> if (strlen(e.iface) == 0)
> @@ -400,11 +400,11 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
>
> if (!eiface)
> return -EINVAL;
> - STRLCPY(e.iface, eiface);
> + STRSCPY(e.iface, eiface);
> e.physdev = 1;
> #endif
> } else {
> - STRLCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
> + STRSCPY(e.iface, SRCDIR ? IFACE(in) : IFACE(out));
> }
>
> if (strlen(e.iface) == 0)
> --
> 2.41.0.162.gfafddb0af9-goog
>
>
>
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] netfilter: ipset: Replace strlcpy with strscpy
2023-06-13 0:34 [PATCH] netfilter: ipset: Replace strlcpy with strscpy Azeem Shaikh
` (2 preceding siblings ...)
2023-06-16 10:51 ` Jozsef Kadlecsik
@ 2023-06-20 20:28 ` Kees Cook
3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2023-06-20 20:28 UTC (permalink / raw)
To: fw, pablo, azeemshaikh38, kadlec
Cc: Kees Cook, netfilter-devel, kuba, pabeni, coreteam,
linux-hardening, davem, edumazet, netdev, linux-kernel
On Tue, 13 Jun 2023 00:34:37 +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().
>
> [...]
Since this got Acked and it's a trivial change, I'll take this via the
hardening tree. Thanks!
Applied to for-next/hardening, thanks!
[1/1] netfilter: ipset: Replace strlcpy with strscpy
https://git.kernel.org/kees/c/0b2fa86361f4
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-20 20:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-13 0:34 [PATCH] netfilter: ipset: Replace strlcpy with strscpy Azeem Shaikh
2023-06-13 19:26 ` Kees Cook
2023-06-14 12:10 ` Simon Horman
2023-06-16 10:51 ` Jozsef Kadlecsik
2023-06-20 20:28 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.