lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 31 Mar 2016 15:36:30 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	hannes@...essinduktion.org
Cc:	daniel@...earbox.net, eric.dumazet@...il.com,
	alexei.starovoitov@...il.com, mkubecek@...e.cz,
	sasha.levin@...cle.com, jslaby@...e.cz, mst@...hat.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH net] tun, bpf: fix suspicious RCU usage in
 tun_{attach,detach}_filter

From: Hannes Frederic Sowa <hannes@...essinduktion.org>
Date: Thu, 31 Mar 2016 21:24:12 +0200

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4b81b71171b4ce..8ab270d5ce5507 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1166,7 +1166,8 @@ static int __sk_attach_prog(struct bpf_prog
> *prog, struct sock *sk)
>  	}
> 
>  	old_fp = rcu_dereference_protected(sk->sk_filter,
> -					   sock_owned_by_user(sk));
> +					   lockdep_rtnl_is_held() ||
> +					   lockdep_sock_is_held(sk));
>  	rcu_assign_pointer(sk->sk_filter, fp);
> 
>  	if (old_fp)

I have the same objections Daniel did.

Not all socket filter clients use RTNL as the synchornization
mechanism.  The caller, or some descriptive element, should tell us
what that synchronizing element is.

Yes, I understand how these RTNL checks can pass "accidently" but
the opposite is true too.  A socket locking synchornizing user,
who didn't lock the socket, might now pass because RTNL happens
to be held elsewhere.

Constraining the test properly, based upon the user, makes this less
likely to happen.  And that's desirable.

Powered by blists - more mailing lists