netfilter: xt_owner: Fix for unsafe access of sk->sk_socket

[ Upstream commit 7ae836a3d630e146b732fe8ef7d86b243748751f ]

A concurrently running sock_orphan() may NULL the sk_socket pointer in
between check and deref. Follow other users (like nft_meta.c for
instance) and acquire sk_callback_lock before dereferencing sk_socket.

Fixes: 0265ab44bacc ("[NETFILTER]: merge ipt_owner/ip6t_owner in xt_owner")
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/netfilter/xt_owner.c b/net/netfilter/xt_owner.c
index e85ce69..5033288 100644 (file)
--- a/net/netfilter/xt_owner.c
+++ b/net/netfilter/xt_owner.c
@@ -76,18 +76,23 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
                 */
                return false;
 
-       filp = sk->sk_socket->file;
-       if (filp == NULL)
+       read_lock_bh(&sk->sk_callback_lock);
+       filp = sk->sk_socket ? sk->sk_socket->file : NULL;
+       if (filp == NULL) {
+               read_unlock_bh(&sk->sk_callback_lock);
                return ((info->match ^ info->invert) &
                       (XT_OWNER_UID | XT_OWNER_GID)) == 0;
+       }
 
        if (info->match & XT_OWNER_UID) {
                kuid_t uid_min = make_kuid(net->user_ns, info->uid_min);
                kuid_t uid_max = make_kuid(net->user_ns, info->uid_max);
                if ((uid_gte(filp->f_cred->fsuid, uid_min) &&
                     uid_lte(filp->f_cred->fsuid, uid_max)) ^
-                   !(info->invert & XT_OWNER_UID))
+                   !(info->invert & XT_OWNER_UID)) {
+                       read_unlock_bh(&sk->sk_callback_lock);
                        return false;
+               }
        }
 
        if (info->match & XT_OWNER_GID) {
@@ -112,10 +117,13 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
                        }
                }
 
-               if (match ^ !(info->invert & XT_OWNER_GID))
+               if (match ^ !(info->invert & XT_OWNER_GID)) {
+                       read_unlock_bh(&sk->sk_callback_lock);
                        return false;
+               }
        }
 
+       read_unlock_bh(&sk->sk_callback_lock);
        return true;
 }