diff mbox

[GIT] Networking

Message ID 20170710140048.GA15408@gondor.apana.org.au
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Herbert Xu July 10, 2017, 2 p.m. UTC
On Mon, Jul 10, 2017 at 08:10:02AM -0400, Sowmini Varadhan wrote:
>
> The reason that the WARN_ON is triggered is that af_alg_accept() calls
> sock_init_data() which does 
> 
>    2636         if (sock) {
>     :
>    2639                 sock->sk        =       sk;

Oh yes.  This started out with just sock_init_data which would
not have triggered your warning.  Then someone came along and
added sock_graft which basically duplicates work already done in
sock_init_data.

In fact the reason they wanted sock_graft was purely to call
security_sock_graft to initialise some SELinux state.  So we
could avoid your warning by calling that directly.

---8<---
crypto: af_alg - Avoid sock_graft call warning

The newly added sock_graft warning triggers in af_alg_accept.
It's harmless as we're essentially doing sock->sk = sock->sk.

The sock_graft call is actually redundant because all the work
it does is subsumed by sock_init_data.  However, it was added
to placate SELinux as it uses it to initialise its internal state.

This patch avoisd the warning by making the SELinux call directly.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

David Miller July 11, 2017, 8:31 p.m. UTC | #1
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 10 Jul 2017 22:00:48 +0800

> crypto: af_alg - Avoid sock_graft call warning
> 
> The newly added sock_graft warning triggers in af_alg_accept.
> It's harmless as we're essentially doing sock->sk = sock->sk.
> 
> The sock_graft call is actually redundant because all the work
> it does is subsumed by sock_init_data.  However, it was added
> to placate SELinux as it uses it to initialise its internal state.
> 
> This patch avoisd the warning by making the SELinux call directly.
> 
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: David S. Miller <davem@davemloft.net>

Looks good, is this going via my tree or your's?
Herbert Xu July 12, 2017, 1:44 a.m. UTC | #2
On Tue, Jul 11, 2017 at 01:31:14PM -0700, David Miller wrote:
>
> Acked-by: David S. Miller <davem@davemloft.net>
> 
> Looks good, is this going via my tree or your's?

I'll push it along.  Thanks.
diff mbox

Patch

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 3556d8e..92a3d54 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -287,7 +287,7 @@  int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 		goto unlock;
 
 	sock_init_data(newsock, sk2);
-	sock_graft(sk2, newsock);
+	security_sock_graft(sk2, newsock);
 	security_sk_clone(sk, sk2);
 
 	err = type->accept(ask->private, sk2);