Hi Michal,
I have one question.
On Wed, Mar 05, 2025 at 06:09:35PM +0100, Michal Koutný wrote:
> diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
> index c0f5e9a4f3c65..c3055e74aa0ea 100644
> --- a/net/netfilter/xt_cgroup.c
> +++ b/net/netfilter/xt_cgroup.c
> @@ -23,6 +23,13 @@ MODULE_DESCRIPTION("Xtables: process control group matching");
> MODULE_ALIAS("ipt_cgroup");
> MODULE_ALIAS("ip6t_cgroup");
>
> +#define NET_CLS_CLASSID_INVALID_MSG "xt_cgroup: classid invalid without net_cls cgroups\n"
> +
> +static bool possible_classid(u32 classid)
> +{
> + return IS_ENABLED(CONFIG_CGROUP_NET_CLASSID) || classid == 0;
> +}
> +
> static int cgroup_mt_check_v0(const struct xt_mtchk_param *par)
> {
> struct xt_cgroup_info_v0 *info = par->matchinfo;
> @@ -30,6 +37,11 @@ static int cgroup_mt_check_v0(const struct xt_mtchk_param *par)
> if (info->invert & ~1)
> return -EINVAL;
>
> + if (!possible_classid(info->id)) {
why classid != 0 is accepted for cgroup_mt_check_v0()?
cgroup_mt_check_v0 represents revision 0 of this match, and this match
only supports for clsid (groupsv1).
History of revisions of cgroupsv2:
- cgroup_mt_check_v0 added to match on clsid (initial version of this match)
- cgroup_mt_check_v1 is added to support cgroupsv2 matching
- cgroup_mt_check_v2 is added to make cgroupsv2 matching more flexible
I mean, if !IS_ENABLED(CONFIG_CGROUP_NET_CLASSID) then xt_cgroup
should fail for cgroup_mt_check_v0.
But a more general question: why this check for classid == 0 in
cgroup_mt_check_v1 and cgroup_mt_check_v2?
> + pr_info(NET_CLS_CLASSID_INVALID_MSG);
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -51,6 +63,11 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
> return -EINVAL;
> }
>
> + if (!possible_classid(info->classid)) {
> + pr_info(NET_CLS_CLASSID_INVALID_MSG);
> + return -EINVAL;
> + }
> +
> info->priv = NULL;
> if (info->has_path) {
> cgrp = cgroup_get_from_path(info->path);
> @@ -83,6 +100,11 @@ static int cgroup_mt_check_v2(const struct xt_mtchk_param *par)
> return -EINVAL;
> }
>
> + if (info->has_classid && !possible_classid(info->classid)) {
> + pr_info(NET_CLS_CLASSID_INVALID_MSG);
> + return -EINVAL;
> + }
> +
> info->priv = NULL;
> if (info->has_path) {
> cgrp = cgroup_get_from_path(info->path);
>
> base-commit: dd83757f6e686a2188997cb58b5975f744bb7786
> --
> 2.48.1
>
