Commit 79e88659 authored by Daniel Borkmann's avatar Daniel Borkmann Committed by Herbert Xu
Browse files

crypto: algif - add and use sock_kzfree_s() instead of memzero_explicit() 



Commit e1bd95bf ("crypto: algif - zeroize IV buffer") and
2a6af25b ("crypto: algif - zeroize message digest buffer")
added memzero_explicit() calls on buffers that are later on
passed back to sock_kfree_s().

This is a discussed follow-up that, instead, extends the sock
API and adds sock_kzfree_s(), which internally uses kzfree()
instead of kfree() for passing the buffers back to slab.

Having sock_kzfree_s() allows to keep the changes more minimal
by just having a drop-in replacement instead of adding
memzero_explicit() calls everywhere before sock_kfree_s().

In kzfree(), the compiler is not allowed to optimize the memset()
away and thus there's no need for memzero_explicit(). Both,
sock_kfree_s() and sock_kzfree_s() are wrappers for
__sock_kfree_s() and call into kfree() resp. kzfree(); here,
__sock_kfree_s() needs to be explicitly inlined as we want the
compiler to optimize the call and condition away and thus it
produces e.g. on x86_64 the _same_ assembler output for
sock_kfree_s() before and after, and thus also allows for
avoiding code duplication.

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: default avatarDaniel Borkmann <dborkman@redhat.com>
Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
parent 5d26a105

crypto/algif_hash.c

+2 −4
Original line number Diff line number Diff line
@@ -258,9 +258,7 @@ static void hash_sock_destruct(struct sock *sk) 
	struct alg_sock *ask = alg_sk(sk);

	struct hash_ctx *ctx = ask->private;



	memzero_explicit(ctx->result,

		     crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)));

	sock_kfree_s(sk, ctx->result,

	sock_kzfree_s(sk, ctx->result,

		      crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)));

	sock_kfree_s(sk, ctx, ctx->len);

	af_alg_release_parent(sk);

crypto/algif_skcipher.c

+1 −2
Original line number Diff line number Diff line
@@ -566,8 +566,7 @@ static void skcipher_sock_destruct(struct sock *sk) 
	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(&ctx->req);



	skcipher_free_sgl(sk);

	memzero_explicit(ctx->iv, crypto_ablkcipher_ivsize(tfm));

	sock_kfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));

	sock_kzfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));

	sock_kfree_s(sk, ctx, ctx->len);

	af_alg_release_parent(sk);

}

include/net/sock.h

+1 −0
Original line number Diff line number Diff line
@@ -1588,6 +1588,7 @@ struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len, 
				     int *errcode, int max_page_order);

void *sock_kmalloc(struct sock *sk, int size, gfp_t priority);

void sock_kfree_s(struct sock *sk, void *mem, int size);

void sock_kzfree_s(struct sock *sk, void *mem, int size);

void sk_send_sigurg(struct sock *sk);



/*

net/core/sock.c

+20 −4
Original line number Diff line number Diff line
@@ -1713,18 +1713,34 @@ void *sock_kmalloc(struct sock *sk, int size, gfp_t priority) 
}

EXPORT_SYMBOL(sock_kmalloc);



/*

 * Free an option memory block.

/* Free an option memory block. Note, we actually want the inline

 * here as this allows gcc to detect the nullify and fold away the

 * condition entirely.

 */

void sock_kfree_s(struct sock *sk, void *mem, int size)

static inline void __sock_kfree_s(struct sock *sk, void *mem, int size,

				  const bool nullify)

{

	if (WARN_ON_ONCE(!mem))

		return;

	if (nullify)

		kzfree(mem);

	else

		kfree(mem);

	atomic_sub(size, &sk->sk_omem_alloc);

}



void sock_kfree_s(struct sock *sk, void *mem, int size)

{

	__sock_kfree_s(sk, mem, size, false);

}

EXPORT_SYMBOL(sock_kfree_s);



void sock_kzfree_s(struct sock *sk, void *mem, int size)

{

	__sock_kfree_s(sk, mem, size, true);

}

EXPORT_SYMBOL(sock_kzfree_s);



/* It is almost wait_for_tcp_memory minus release_sock/lock_sock.

   I think, these locks should be removed for datagram sockets.

 */

CRA Git | Maintained and supported by SUSTech CRA and CCSE