Skip to content

Commit

Permalink
net/sched: act_ipt: add sanity checks on table name and hook locations
Browse files Browse the repository at this point in the history
Looks like "tc" hard-codes "mangle" as the only supported table
name, but on kernel side there are no checks.

This is wrong.  Not all xtables targets are safe to call from tc.
E.g. "nat" targets assume skb has a conntrack object assigned to it.
Normally those get called from netfilter nat core which consults the
nat table to obtain the address mapping.

"tc" userspace either sets PRE or POSTROUTING as hook number, but there
is no validation of this on kernel side, so update netlink policy to
reject bogus numbers.  Some targets may assume skb_dst is set for
input/forward hooks, so prevent those from being used.

act_ipt uses the hook number in two places:
1. the state hook number, this is fine as-is
2. to set par.hook_mask

The latter is a bit mask, so update the assignment to make
xt_check_target() to the right thing.

Followup patch adds required checks for the skb/packet headers before
calling the targets evaluation function.

Fixes: 1da177e ("Linux-2.6.12-rc2")
Signed-off-by: Florian Westphal <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
Acked-by: Jamal Hadi Salim <[email protected]>
Signed-off-by: Paolo Abeni <[email protected]>
  • Loading branch information
Florian Westphal authored and Paolo Abeni committed Jun 29, 2023
1 parent 6feb37b commit b4ee933
Showing 1 changed file with 20 additions and 7 deletions.
27 changes: 20 additions & 7 deletions net/sched/act_ipt.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ static int ipt_init_target(struct net *net, struct xt_entry_target *t,
par.entryinfo = &e;
par.target = target;
par.targinfo = t->data;
par.hook_mask = hook;
par.hook_mask = 1 << hook;
par.family = NFPROTO_IPV4;

ret = xt_check_target(&par, t->u.target_size - sizeof(*t), 0, false);
Expand Down Expand Up @@ -85,7 +85,8 @@ static void tcf_ipt_release(struct tc_action *a)

static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = {
[TCA_IPT_TABLE] = { .type = NLA_STRING, .len = IFNAMSIZ },
[TCA_IPT_HOOK] = { .type = NLA_U32 },
[TCA_IPT_HOOK] = NLA_POLICY_RANGE(NLA_U32, NF_INET_PRE_ROUTING,
NF_INET_NUMHOOKS),
[TCA_IPT_INDEX] = { .type = NLA_U32 },
[TCA_IPT_TARG] = { .len = sizeof(struct xt_entry_target) },
};
Expand Down Expand Up @@ -158,15 +159,27 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
return -EEXIST;
}
}

err = -EINVAL;
hook = nla_get_u32(tb[TCA_IPT_HOOK]);
switch (hook) {
case NF_INET_PRE_ROUTING:
break;
case NF_INET_POST_ROUTING:
break;
default:
goto err1;
}

if (tb[TCA_IPT_TABLE]) {
/* mangle only for now */
if (nla_strcmp(tb[TCA_IPT_TABLE], "mangle"))
goto err1;
}

err = -ENOMEM;
tname = kmalloc(IFNAMSIZ, GFP_KERNEL);
tname = kstrdup("mangle", GFP_KERNEL);
if (unlikely(!tname))
goto err1;
if (tb[TCA_IPT_TABLE] == NULL ||
nla_strscpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
strcpy(tname, "mangle");

t = kmemdup(td, td->u.target_size, GFP_KERNEL);
if (unlikely(!t))
Expand Down

0 comments on commit b4ee933

Please sign in to comment.