Commit 3610cda53f247e176bcbb7a7cca64bc53b12acdb

Authored by David S. Miller
1 parent 44b8288308

af_unix: Avoid socket->sk NULL OOPS in stream connect security hooks.

unix_release() can asynchornously set socket->sk to NULL, and
it does so without holding the unix_state_lock() on "other"
during stream connects.

However, the reverse mapping, sk->sk_socket, is only transitioned
to NULL under the unix_state_lock().

Therefore make the security hooks follow the reverse mapping instead
of the forward mapping.

Reported-by: Jeremy Fitzhardinge <jeremy@goop.org>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 6 changed files with 22 additions and 24 deletions Side-by-side Diff

include/linux/security.h
... ... @@ -796,8 +796,9 @@
796 796 * @unix_stream_connect:
797 797 * Check permissions before establishing a Unix domain stream connection
798 798 * between @sock and @other.
799   - * @sock contains the socket structure.
800   - * @other contains the peer socket structure.
  799 + * @sock contains the sock structure.
  800 + * @other contains the peer sock structure.
  801 + * @newsk contains the new sock structure.
801 802 * Return 0 if permission is granted.
802 803 * @unix_may_send:
803 804 * Check permissions before connecting or sending datagrams from @sock to
... ... @@ -1568,8 +1569,7 @@
1568 1569 int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
1569 1570  
1570 1571 #ifdef CONFIG_SECURITY_NETWORK
1571   - int (*unix_stream_connect) (struct socket *sock,
1572   - struct socket *other, struct sock *newsk);
  1572 + int (*unix_stream_connect) (struct sock *sock, struct sock *other, struct sock *newsk);
1573 1573 int (*unix_may_send) (struct socket *sock, struct socket *other);
1574 1574  
1575 1575 int (*socket_create) (int family, int type, int protocol, int kern);
... ... @@ -2525,8 +2525,7 @@
2525 2525  
2526 2526 #ifdef CONFIG_SECURITY_NETWORK
2527 2527  
2528   -int security_unix_stream_connect(struct socket *sock, struct socket *other,
2529   - struct sock *newsk);
  2528 +int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
2530 2529 int security_unix_may_send(struct socket *sock, struct socket *other);
2531 2530 int security_socket_create(int family, int type, int protocol, int kern);
2532 2531 int security_socket_post_create(struct socket *sock, int family,
... ... @@ -2567,8 +2566,8 @@
2567 2566 int security_tun_dev_attach(struct sock *sk);
2568 2567  
2569 2568 #else /* CONFIG_SECURITY_NETWORK */
2570   -static inline int security_unix_stream_connect(struct socket *sock,
2571   - struct socket *other,
  2569 +static inline int security_unix_stream_connect(struct sock *sock,
  2570 + struct sock *other,
2572 2571 struct sock *newsk)
2573 2572 {
2574 2573 return 0;
... ... @@ -1157,7 +1157,7 @@
1157 1157 goto restart;
1158 1158 }
1159 1159  
1160   - err = security_unix_stream_connect(sock, other->sk_socket, newsk);
  1160 + err = security_unix_stream_connect(sk, other, newsk);
1161 1161 if (err) {
1162 1162 unix_state_unlock(sk);
1163 1163 goto out_unlock;
security/capability.c
... ... @@ -548,7 +548,7 @@
548 548 }
549 549  
550 550 #ifdef CONFIG_SECURITY_NETWORK
551   -static int cap_unix_stream_connect(struct socket *sock, struct socket *other,
  551 +static int cap_unix_stream_connect(struct sock *sock, struct sock *other,
552 552 struct sock *newsk)
553 553 {
554 554 return 0;
... ... @@ -977,8 +977,7 @@
977 977  
978 978 #ifdef CONFIG_SECURITY_NETWORK
979 979  
980   -int security_unix_stream_connect(struct socket *sock, struct socket *other,
981   - struct sock *newsk)
  980 +int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk)
982 981 {
983 982 return security_ops->unix_stream_connect(sock, other, newsk);
984 983 }
security/selinux/hooks.c
... ... @@ -3921,18 +3921,18 @@
3921 3921 return sock_has_perm(current, sock->sk, SOCKET__SHUTDOWN);
3922 3922 }
3923 3923  
3924   -static int selinux_socket_unix_stream_connect(struct socket *sock,
3925   - struct socket *other,
  3924 +static int selinux_socket_unix_stream_connect(struct sock *sock,
  3925 + struct sock *other,
3926 3926 struct sock *newsk)
3927 3927 {
3928   - struct sk_security_struct *sksec_sock = sock->sk->sk_security;
3929   - struct sk_security_struct *sksec_other = other->sk->sk_security;
  3928 + struct sk_security_struct *sksec_sock = sock->sk_security;
  3929 + struct sk_security_struct *sksec_other = other->sk_security;
3930 3930 struct sk_security_struct *sksec_new = newsk->sk_security;
3931 3931 struct common_audit_data ad;
3932 3932 int err;
3933 3933  
3934 3934 COMMON_AUDIT_DATA_INIT(&ad, NET);
3935   - ad.u.net.sk = other->sk;
  3935 + ad.u.net.sk = other;
3936 3936  
3937 3937 err = avc_has_perm(sksec_sock->sid, sksec_other->sid,
3938 3938 sksec_other->sclass,
security/smack/smack_lsm.c
... ... @@ -2408,22 +2408,22 @@
2408 2408  
2409 2409 /**
2410 2410 * smack_unix_stream_connect - Smack access on UDS
2411   - * @sock: one socket
2412   - * @other: the other socket
  2411 + * @sock: one sock
  2412 + * @other: the other sock
2413 2413 * @newsk: unused
2414 2414 *
2415 2415 * Return 0 if a subject with the smack of sock could access
2416 2416 * an object with the smack of other, otherwise an error code
2417 2417 */
2418   -static int smack_unix_stream_connect(struct socket *sock,
2419   - struct socket *other, struct sock *newsk)
  2418 +static int smack_unix_stream_connect(struct sock *sock,
  2419 + struct sock *other, struct sock *newsk)
2420 2420 {
2421   - struct inode *sp = SOCK_INODE(sock);
2422   - struct inode *op = SOCK_INODE(other);
  2421 + struct inode *sp = SOCK_INODE(sock->sk_socket);
  2422 + struct inode *op = SOCK_INODE(other->sk_socket);
2423 2423 struct smk_audit_info ad;
2424 2424  
2425 2425 smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NET);
2426   - smk_ad_setfield_u_net_sk(&ad, other->sk);
  2426 + smk_ad_setfield_u_net_sk(&ad, other);
2427 2427 return smk_access(smk_of_inode(sp), smk_of_inode(op),
2428 2428 MAY_READWRITE, &ad);
2429 2429 }