Skip to content

Commit 7ae836a

Browse files
Phil Sutterummakynes
authored andcommitted
netfilter: xt_owner: Fix for unsafe access of sk->sk_socket
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: 0265ab4 ("[NETFILTER]: merge ipt_owner/ip6t_owner in xt_owner") Reported-by: Jann Horn <[email protected]> Signed-off-by: Phil Sutter <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent f6e1532 commit 7ae836a

File tree

1 file changed

+12
-4
lines changed

1 file changed

+12
-4
lines changed

net/netfilter/xt_owner.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,23 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
7676
*/
7777
return false;
7878

79-
filp = sk->sk_socket->file;
80-
if (filp == NULL)
79+
read_lock_bh(&sk->sk_callback_lock);
80+
filp = sk->sk_socket ? sk->sk_socket->file : NULL;
81+
if (filp == NULL) {
82+
read_unlock_bh(&sk->sk_callback_lock);
8183
return ((info->match ^ info->invert) &
8284
(XT_OWNER_UID | XT_OWNER_GID)) == 0;
85+
}
8386

8487
if (info->match & XT_OWNER_UID) {
8588
kuid_t uid_min = make_kuid(net->user_ns, info->uid_min);
8689
kuid_t uid_max = make_kuid(net->user_ns, info->uid_max);
8790
if ((uid_gte(filp->f_cred->fsuid, uid_min) &&
8891
uid_lte(filp->f_cred->fsuid, uid_max)) ^
89-
!(info->invert & XT_OWNER_UID))
92+
!(info->invert & XT_OWNER_UID)) {
93+
read_unlock_bh(&sk->sk_callback_lock);
9094
return false;
95+
}
9196
}
9297

9398
if (info->match & XT_OWNER_GID) {
@@ -112,10 +117,13 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
112117
}
113118
}
114119

115-
if (match ^ !(info->invert & XT_OWNER_GID))
120+
if (match ^ !(info->invert & XT_OWNER_GID)) {
121+
read_unlock_bh(&sk->sk_callback_lock);
116122
return false;
123+
}
117124
}
118125

126+
read_unlock_bh(&sk->sk_callback_lock);
119127
return true;
120128
}
121129

0 commit comments

Comments
 (0)