* [PATCH] sock: Use unsafe_memcpy() for sock_copy()
@ 2024-02-16 20:44 Kees Cook
2024-02-16 22:06 ` Kuniyuki Iwashima
0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2024-02-16 20:44 UTC (permalink / raw
To: Jakub Kicinski
Cc: Kees Cook, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
Kuniyuki Iwashima, Abel Wu, Breno Leitao, Alexander Mikhalitsyn,
David Howells, linux-kernel, linux-hardening
While testing for places where zero-sized destinations were still
showing up in the kernel, sock_copy() was found, which is using very
specific memcpy() offsets for both avoiding a portion of struct sock,
and copying beyond the end of it (since struct sock is really just a
common header before the protocol-specific allocation). Instead of
trying to unravel this historical lack of container_of(), just switch
to unsafe_memcpy(), since that's effectively what was happening already
(memcpy() wasn't checking 0-sized destinations while the code base was
being converted away from fake flexible arrays).
Avoid the following false positive warning with future changes to
CONFIG_FORTIFY_SOURCE:
memcpy: detected field-spanning write (size 3068) of destination "&nsk->__sk_common.skc_dontcopy_end" at net/core/sock.c:2057 (size 0)
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org
---
net/core/sock.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index 0a7f46c37f0c..b7ea358eb18f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2053,8 +2053,9 @@ static void sock_copy(struct sock *nsk, const struct sock *osk)
memcpy(nsk, osk, offsetof(struct sock, sk_dontcopy_begin));
- memcpy(&nsk->sk_dontcopy_end, &osk->sk_dontcopy_end,
- prot->obj_size - offsetof(struct sock, sk_dontcopy_end));
+ unsafe_memcpy(&nsk->sk_dontcopy_end, &osk->sk_dontcopy_end,
+ prot->obj_size - offsetof(struct sock, sk_dontcopy_end),
+ /* alloc is larger than struct, see sk_prot_alloc() */);
#ifdef CONFIG_SECURITY_NETWORK
nsk->sk_security = sptr;
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] sock: Use unsafe_memcpy() for sock_copy()
2024-02-16 20:44 [PATCH] sock: Use unsafe_memcpy() for sock_copy() Kees Cook
@ 2024-02-16 22:06 ` Kuniyuki Iwashima
0 siblings, 0 replies; 2+ messages in thread
From: Kuniyuki Iwashima @ 2024-02-16 22:06 UTC (permalink / raw
To: keescook
Cc: alexander, davem, dhowells, edumazet, kuba, kuniyu, leitao,
linux-hardening, linux-kernel, netdev, pabeni, wuyun.abel
From: Kees Cook <keescook@chromium.org>
Date: Fri, 16 Feb 2024 12:44:24 -0800
> While testing for places where zero-sized destinations were still
> showing up in the kernel, sock_copy() was found, which is using very
> specific memcpy() offsets for both avoiding a portion of struct sock,
> and copying beyond the end of it (since struct sock is really just a
> common header before the protocol-specific allocation). Instead of
> trying to unravel this historical lack of container_of(), just switch
> to unsafe_memcpy(), since that's effectively what was happening already
> (memcpy() wasn't checking 0-sized destinations while the code base was
> being converted away from fake flexible arrays).
>
> Avoid the following false positive warning with future changes to
> CONFIG_FORTIFY_SOURCE:
>
> memcpy: detected field-spanning write (size 3068) of destination "&nsk->__sk_common.skc_dontcopy_end" at net/core/sock.c:2057 (size 0)
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
I confirmed unsafe_memcpy() is just memcpy() without fortified checks.
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: netdev@vger.kernel.org
> ---
> net/core/sock.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 0a7f46c37f0c..b7ea358eb18f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2053,8 +2053,9 @@ static void sock_copy(struct sock *nsk, const struct sock *osk)
>
> memcpy(nsk, osk, offsetof(struct sock, sk_dontcopy_begin));
>
> - memcpy(&nsk->sk_dontcopy_end, &osk->sk_dontcopy_end,
> - prot->obj_size - offsetof(struct sock, sk_dontcopy_end));
> + unsafe_memcpy(&nsk->sk_dontcopy_end, &osk->sk_dontcopy_end,
> + prot->obj_size - offsetof(struct sock, sk_dontcopy_end),
> + /* alloc is larger than struct, see sk_prot_alloc() */);
>
> #ifdef CONFIG_SECURITY_NETWORK
> nsk->sk_security = sptr;
> --
> 2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-02-16 22:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-16 20:44 [PATCH] sock: Use unsafe_memcpy() for sock_copy() Kees Cook
2024-02-16 22:06 ` Kuniyuki Iwashima
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).