ACK: [PATCH 1/1] net: Set sk_prot_creator when cloning sockets to the right proto

[Kleber Souza]

On 12/6/18 10:59 PM, Tyler Hicks wrote:
> From: Christoph Paasch <cpaasch at apple.com>
>
> sk->sk_prot and sk->sk_prot_creator can differ when the app uses
> IPV6_ADDRFORM (transforming an IPv6-socket to an IPv4-one).
> Which is why sk_prot_creator is there to make sure that sk_prot_free()
> does the kmem_cache_free() on the right kmem_cache slab.
>
> Now, if such a socket gets transformed back to a listening socket (using
> connect() with AF_UNSPEC) we will allocate an IPv4 tcp_sock through
> sk_clone_lock() when a new connection comes in. But sk_prot_creator will
> still point to the IPv6 kmem_cache (as everything got copied in
> sk_clone_lock()). When freeing, we will thus put this
> memory back into the IPv6 kmem_cache although it was allocated in the
> IPv4 cache. I have seen memory corruption happening because of this.
>
> With slub-debugging and MEMCG_KMEM enabled this gives the warning
> 	"cache_from_obj: Wrong slab cache. TCPv6 but object is from TCP"
>
> A C-program to trigger this:
>
> void main(void)
> {
>         int fd = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP);
>         int new_fd, newest_fd, client_fd;
>         struct sockaddr_in6 bind_addr;
>         struct sockaddr_in bind_addr4, client_addr1, client_addr2;
>         struct sockaddr unsp;
>         int val;
>
>         memset(&bind_addr, 0, sizeof(bind_addr));
>         bind_addr.sin6_family = AF_INET6;
>         bind_addr.sin6_port = ntohs(42424);
>
>         memset(&client_addr1, 0, sizeof(client_addr1));
>         client_addr1.sin_family = AF_INET;
>         client_addr1.sin_port = ntohs(42424);
>         client_addr1.sin_addr.s_addr = inet_addr("127.0.0.1");
>
>         memset(&client_addr2, 0, sizeof(client_addr2));
>         client_addr2.sin_family = AF_INET;
>         client_addr2.sin_port = ntohs(42421);
>         client_addr2.sin_addr.s_addr = inet_addr("127.0.0.1");
>
>         memset(&unsp, 0, sizeof(unsp));
>         unsp.sa_family = AF_UNSPEC;
>
>         bind(fd, (struct sockaddr *)&bind_addr, sizeof(bind_addr));
>
>         listen(fd, 5);
>
>         client_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
>         connect(client_fd, (struct sockaddr *)&client_addr1, sizeof(client_addr1));
>         new_fd = accept(fd, NULL, NULL);
>         close(fd);
>
>         val = AF_INET;
>         setsockopt(new_fd, SOL_IPV6, IPV6_ADDRFORM, &val, sizeof(val));
>
>         connect(new_fd, &unsp, sizeof(unsp));
>
>         memset(&bind_addr4, 0, sizeof(bind_addr4));
>         bind_addr4.sin_family = AF_INET;
>         bind_addr4.sin_port = ntohs(42421);
>         bind(new_fd, (struct sockaddr *)&bind_addr4, sizeof(bind_addr4));
>
>         listen(new_fd, 5);
>
>         client_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
>         connect(client_fd, (struct sockaddr *)&client_addr2, sizeof(client_addr2));
>
>         newest_fd = accept(new_fd, NULL, NULL);
>         close(new_fd);
>
>         close(client_fd);
>         close(new_fd);
> }
>
> As far as I can see, this bug has been there since the beginning of the
> git-days.
>
> Signed-off-by: Christoph Paasch <cpaasch at apple.com>
> Reviewed-by: Eric Dumazet <edumazet at google.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
>
> CVE-2018-9568
>
> (cherry picked from commit 9d538fa60bad4f7b23193c89e843797a1cf71ef3)
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> ---
>  net/core/sock.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index a2702d5d03bf..871e98eb8dd1 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1511,6 +1511,8 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
>  
>  		sock_copy(newsk, sk);
>  
> +		newsk->sk_prot_creator = sk->sk_prot;
> +
>  		/* SANITY */
>  		get_net(sock_net(newsk));
>  		sk_node_init(&newsk->sk_node);