On 5/8/19 10:12 AM, Lukasz Pawelczyk wrote:
> The XT_SUPPL_GROUPS flag causes GIDs specified with XT_OWNER_GID to
> be also checked in the supplementary groups of a process.
>
> Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@xxxxxxxxxxx>
> ---
> include/uapi/linux/netfilter/xt_owner.h | 1 +
> net/netfilter/xt_owner.c | 23 ++++++++++++++++++++---
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/netfilter/xt_owner.h b/include/uapi/linux/netfilter/xt_owner.h
> index fa3ad84957d5..d646f0dc3466 100644
> --- a/include/uapi/linux/netfilter/xt_owner.h
> +++ b/include/uapi/linux/netfilter/xt_owner.h
> @@ -8,6 +8,7 @@ enum {
> XT_OWNER_UID = 1 << 0,
> XT_OWNER_GID = 1 << 1,
> XT_OWNER_SOCKET = 1 << 2,
> + XT_SUPPL_GROUPS = 1 << 3,
> };
>
> struct xt_owner_match_info {
> diff --git a/net/netfilter/xt_owner.c b/net/netfilter/xt_owner.c
> index 46686fb73784..283a1fb5cc52 100644
> --- a/net/netfilter/xt_owner.c
> +++ b/net/netfilter/xt_owner.c
> @@ -91,11 +91,28 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
> }
>
> if (info->match & XT_OWNER_GID) {
> + unsigned int i, match = false;
> kgid_t gid_min = make_kgid(net->user_ns, info->gid_min);
> kgid_t gid_max = make_kgid(net->user_ns, info->gid_max);
> - if ((gid_gte(filp->f_cred->fsgid, gid_min) &&
> - gid_lte(filp->f_cred->fsgid, gid_max)) ^
> - !(info->invert & XT_OWNER_GID))
> + struct group_info *gi = filp->f_cred->group_info;
> +
> + if (gid_gte(filp->f_cred->fsgid, gid_min) &&
> + gid_lte(filp->f_cred->fsgid, gid_max))
> + match = true;
> +
> + if (!match && (info->match & XT_SUPPL_GROUPS) && gi) {
> + for (i = 0; i < gi->ngroups; ++i) {
> + kgid_t group = gi->gid[i];
> +
> + if (gid_gte(group, gid_min) &&
> + gid_lte(group, gid_max)) {
> + match = true;
> + break;
> + }
> + }
> + }
> +
> + if (match ^ !(info->invert & XT_OWNER_GID))
> return false;
> }
>
>
How can this be safe on SMP ?
