Commit 2158cfb0 authored by David S. Miller's avatar David S. Miller

Merge git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf

Pablo Neira Ayuso says:

====================
The following patchset contains late Netfilter fixes for net:

1) Use READ_ONCE()/WRITE_ONCE() to update ct->mark, from Daniel Xu.
   Not reported by syzbot, but I presume KASAN would trigger post
   a splat on this. This is a rather old issue, predating git history.

2) Do not set up extensions for set element with end interval flag
   set on. This leads to bogusly skipping this elements as expired
   when listing the set/map to userspace as well as increasing
   memory consumpton when stateful expressions are used. This issue
   has been present since 4.18, when timeout support for rbtree set
   was added.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 19d04a94 33c7aba0
...@@ -296,7 +296,7 @@ skb_flow_dissect_ct(const struct sk_buff *skb, ...@@ -296,7 +296,7 @@ skb_flow_dissect_ct(const struct sk_buff *skb,
key->ct_zone = ct->zone.id; key->ct_zone = ct->zone.id;
#endif #endif
#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
key->ct_mark = ct->mark; key->ct_mark = READ_ONCE(ct->mark);
#endif #endif
cl = nf_ct_labels_find(ct); cl = nf_ct_labels_find(ct);
......
...@@ -435,7 +435,7 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par) ...@@ -435,7 +435,7 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par)
switch (ctinfo) { switch (ctinfo) {
case IP_CT_NEW: case IP_CT_NEW:
ct->mark = hash; WRITE_ONCE(ct->mark, hash);
break; break;
case IP_CT_RELATED: case IP_CT_RELATED:
case IP_CT_RELATED_REPLY: case IP_CT_RELATED_REPLY:
...@@ -452,7 +452,7 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par) ...@@ -452,7 +452,7 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par)
#ifdef DEBUG #ifdef DEBUG
nf_ct_dump_tuple_ip(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple); nf_ct_dump_tuple_ip(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
#endif #endif
pr_debug("hash=%u ct_hash=%u ", hash, ct->mark); pr_debug("hash=%u ct_hash=%u ", hash, READ_ONCE(ct->mark));
if (!clusterip_responsible(cipinfo->config, hash)) { if (!clusterip_responsible(cipinfo->config, hash)) {
pr_debug("not responsible\n"); pr_debug("not responsible\n");
return NF_DROP; return NF_DROP;
......
...@@ -1781,7 +1781,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl, ...@@ -1781,7 +1781,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
} }
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
ct->mark = exp->master->mark; ct->mark = READ_ONCE(exp->master->mark);
#endif #endif
#ifdef CONFIG_NF_CONNTRACK_SECMARK #ifdef CONFIG_NF_CONNTRACK_SECMARK
ct->secmark = exp->master->secmark; ct->secmark = exp->master->secmark;
......
...@@ -328,9 +328,9 @@ ctnetlink_dump_timestamp(struct sk_buff *skb, const struct nf_conn *ct) ...@@ -328,9 +328,9 @@ ctnetlink_dump_timestamp(struct sk_buff *skb, const struct nf_conn *ct)
} }
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
static int ctnetlink_dump_mark(struct sk_buff *skb, const struct nf_conn *ct) static int ctnetlink_dump_mark(struct sk_buff *skb, u32 mark)
{ {
if (nla_put_be32(skb, CTA_MARK, htonl(ct->mark))) if (nla_put_be32(skb, CTA_MARK, htonl(mark)))
goto nla_put_failure; goto nla_put_failure;
return 0; return 0;
...@@ -543,7 +543,7 @@ static int ctnetlink_dump_extinfo(struct sk_buff *skb, ...@@ -543,7 +543,7 @@ static int ctnetlink_dump_extinfo(struct sk_buff *skb,
static int ctnetlink_dump_info(struct sk_buff *skb, struct nf_conn *ct) static int ctnetlink_dump_info(struct sk_buff *skb, struct nf_conn *ct)
{ {
if (ctnetlink_dump_status(skb, ct) < 0 || if (ctnetlink_dump_status(skb, ct) < 0 ||
ctnetlink_dump_mark(skb, ct) < 0 || ctnetlink_dump_mark(skb, READ_ONCE(ct->mark)) < 0 ||
ctnetlink_dump_secctx(skb, ct) < 0 || ctnetlink_dump_secctx(skb, ct) < 0 ||
ctnetlink_dump_id(skb, ct) < 0 || ctnetlink_dump_id(skb, ct) < 0 ||
ctnetlink_dump_use(skb, ct) < 0 || ctnetlink_dump_use(skb, ct) < 0 ||
...@@ -722,6 +722,7 @@ ctnetlink_conntrack_event(unsigned int events, const struct nf_ct_event *item) ...@@ -722,6 +722,7 @@ ctnetlink_conntrack_event(unsigned int events, const struct nf_ct_event *item)
struct sk_buff *skb; struct sk_buff *skb;
unsigned int type; unsigned int type;
unsigned int flags = 0, group; unsigned int flags = 0, group;
u32 mark;
int err; int err;
if (events & (1 << IPCT_DESTROY)) { if (events & (1 << IPCT_DESTROY)) {
...@@ -826,8 +827,9 @@ ctnetlink_conntrack_event(unsigned int events, const struct nf_ct_event *item) ...@@ -826,8 +827,9 @@ ctnetlink_conntrack_event(unsigned int events, const struct nf_ct_event *item)
} }
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
if ((events & (1 << IPCT_MARK) || ct->mark) mark = READ_ONCE(ct->mark);
&& ctnetlink_dump_mark(skb, ct) < 0) if ((events & (1 << IPCT_MARK) || mark) &&
ctnetlink_dump_mark(skb, mark) < 0)
goto nla_put_failure; goto nla_put_failure;
#endif #endif
nlmsg_end(skb, nlh); nlmsg_end(skb, nlh);
...@@ -1154,7 +1156,7 @@ static int ctnetlink_filter_match(struct nf_conn *ct, void *data) ...@@ -1154,7 +1156,7 @@ static int ctnetlink_filter_match(struct nf_conn *ct, void *data)
} }
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
if ((ct->mark & filter->mark.mask) != filter->mark.val) if ((READ_ONCE(ct->mark) & filter->mark.mask) != filter->mark.val)
goto ignore_entry; goto ignore_entry;
#endif #endif
status = (u32)READ_ONCE(ct->status); status = (u32)READ_ONCE(ct->status);
...@@ -2002,9 +2004,9 @@ static void ctnetlink_change_mark(struct nf_conn *ct, ...@@ -2002,9 +2004,9 @@ static void ctnetlink_change_mark(struct nf_conn *ct,
mask = ~ntohl(nla_get_be32(cda[CTA_MARK_MASK])); mask = ~ntohl(nla_get_be32(cda[CTA_MARK_MASK]));
mark = ntohl(nla_get_be32(cda[CTA_MARK])); mark = ntohl(nla_get_be32(cda[CTA_MARK]));
newmark = (ct->mark & mask) ^ mark; newmark = (READ_ONCE(ct->mark) & mask) ^ mark;
if (newmark != ct->mark) if (newmark != READ_ONCE(ct->mark))
ct->mark = newmark; WRITE_ONCE(ct->mark, newmark);
} }
#endif #endif
...@@ -2669,6 +2671,7 @@ static int __ctnetlink_glue_build(struct sk_buff *skb, struct nf_conn *ct) ...@@ -2669,6 +2671,7 @@ static int __ctnetlink_glue_build(struct sk_buff *skb, struct nf_conn *ct)
{ {
const struct nf_conntrack_zone *zone; const struct nf_conntrack_zone *zone;
struct nlattr *nest_parms; struct nlattr *nest_parms;
u32 mark;
zone = nf_ct_zone(ct); zone = nf_ct_zone(ct);
...@@ -2730,7 +2733,8 @@ static int __ctnetlink_glue_build(struct sk_buff *skb, struct nf_conn *ct) ...@@ -2730,7 +2733,8 @@ static int __ctnetlink_glue_build(struct sk_buff *skb, struct nf_conn *ct)
goto nla_put_failure; goto nla_put_failure;
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
if (ct->mark && ctnetlink_dump_mark(skb, ct) < 0) mark = READ_ONCE(ct->mark);
if (mark && ctnetlink_dump_mark(skb, mark) < 0)
goto nla_put_failure; goto nla_put_failure;
#endif #endif
if (ctnetlink_dump_labels(skb, ct) < 0) if (ctnetlink_dump_labels(skb, ct) < 0)
......
...@@ -366,7 +366,7 @@ static int ct_seq_show(struct seq_file *s, void *v) ...@@ -366,7 +366,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
goto release; goto release;
#if defined(CONFIG_NF_CONNTRACK_MARK) #if defined(CONFIG_NF_CONNTRACK_MARK)
seq_printf(s, "mark=%u ", ct->mark); seq_printf(s, "mark=%u ", READ_ONCE(ct->mark));
#endif #endif
ct_show_secctx(s, ct); ct_show_secctx(s, ct);
......
...@@ -5958,7 +5958,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, ...@@ -5958,7 +5958,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
&timeout); &timeout);
if (err) if (err)
return err; return err;
} else if (set->flags & NFT_SET_TIMEOUT) { } else if (set->flags & NFT_SET_TIMEOUT &&
!(flags & NFT_SET_ELEM_INTERVAL_END)) {
timeout = set->timeout; timeout = set->timeout;
} }
...@@ -6024,7 +6025,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, ...@@ -6024,7 +6025,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
err = -EOPNOTSUPP; err = -EOPNOTSUPP;
goto err_set_elem_expr; goto err_set_elem_expr;
} }
} else if (set->num_exprs > 0) { } else if (set->num_exprs > 0 &&
!(flags & NFT_SET_ELEM_INTERVAL_END)) {
err = nft_set_elem_expr_clone(ctx, set, expr_array); err = nft_set_elem_expr_clone(ctx, set, expr_array);
if (err < 0) if (err < 0)
goto err_set_elem_expr_clone; goto err_set_elem_expr_clone;
......
...@@ -98,7 +98,7 @@ static void nft_ct_get_eval(const struct nft_expr *expr, ...@@ -98,7 +98,7 @@ static void nft_ct_get_eval(const struct nft_expr *expr,
return; return;
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
case NFT_CT_MARK: case NFT_CT_MARK:
*dest = ct->mark; *dest = READ_ONCE(ct->mark);
return; return;
#endif #endif
#ifdef CONFIG_NF_CONNTRACK_SECMARK #ifdef CONFIG_NF_CONNTRACK_SECMARK
...@@ -297,8 +297,8 @@ static void nft_ct_set_eval(const struct nft_expr *expr, ...@@ -297,8 +297,8 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
switch (priv->key) { switch (priv->key) {
#ifdef CONFIG_NF_CONNTRACK_MARK #ifdef CONFIG_NF_CONNTRACK_MARK
case NFT_CT_MARK: case NFT_CT_MARK:
if (ct->mark != value) { if (READ_ONCE(ct->mark) != value) {
ct->mark = value; WRITE_ONCE(ct->mark, value);
nf_conntrack_event_cache(IPCT_MARK, ct); nf_conntrack_event_cache(IPCT_MARK, ct);
} }
break; break;
......
...@@ -30,6 +30,7 @@ connmark_tg_shift(struct sk_buff *skb, const struct xt_connmark_tginfo2 *info) ...@@ -30,6 +30,7 @@ connmark_tg_shift(struct sk_buff *skb, const struct xt_connmark_tginfo2 *info)
u_int32_t new_targetmark; u_int32_t new_targetmark;
struct nf_conn *ct; struct nf_conn *ct;
u_int32_t newmark; u_int32_t newmark;
u_int32_t oldmark;
ct = nf_ct_get(skb, &ctinfo); ct = nf_ct_get(skb, &ctinfo);
if (ct == NULL) if (ct == NULL)
...@@ -37,14 +38,15 @@ connmark_tg_shift(struct sk_buff *skb, const struct xt_connmark_tginfo2 *info) ...@@ -37,14 +38,15 @@ connmark_tg_shift(struct sk_buff *skb, const struct xt_connmark_tginfo2 *info)
switch (info->mode) { switch (info->mode) {
case XT_CONNMARK_SET: case XT_CONNMARK_SET:
newmark = (ct->mark & ~info->ctmask) ^ info->ctmark; oldmark = READ_ONCE(ct->mark);
newmark = (oldmark & ~info->ctmask) ^ info->ctmark;
if (info->shift_dir == D_SHIFT_RIGHT) if (info->shift_dir == D_SHIFT_RIGHT)
newmark >>= info->shift_bits; newmark >>= info->shift_bits;
else else
newmark <<= info->shift_bits; newmark <<= info->shift_bits;
if (ct->mark != newmark) { if (READ_ONCE(ct->mark) != newmark) {
ct->mark = newmark; WRITE_ONCE(ct->mark, newmark);
nf_conntrack_event_cache(IPCT_MARK, ct); nf_conntrack_event_cache(IPCT_MARK, ct);
} }
break; break;
...@@ -55,15 +57,15 @@ connmark_tg_shift(struct sk_buff *skb, const struct xt_connmark_tginfo2 *info) ...@@ -55,15 +57,15 @@ connmark_tg_shift(struct sk_buff *skb, const struct xt_connmark_tginfo2 *info)
else else
new_targetmark <<= info->shift_bits; new_targetmark <<= info->shift_bits;
newmark = (ct->mark & ~info->ctmask) ^ newmark = (READ_ONCE(ct->mark) & ~info->ctmask) ^
new_targetmark; new_targetmark;
if (ct->mark != newmark) { if (READ_ONCE(ct->mark) != newmark) {
ct->mark = newmark; WRITE_ONCE(ct->mark, newmark);
nf_conntrack_event_cache(IPCT_MARK, ct); nf_conntrack_event_cache(IPCT_MARK, ct);
} }
break; break;
case XT_CONNMARK_RESTORE: case XT_CONNMARK_RESTORE:
new_targetmark = (ct->mark & info->ctmask); new_targetmark = (READ_ONCE(ct->mark) & info->ctmask);
if (info->shift_dir == D_SHIFT_RIGHT) if (info->shift_dir == D_SHIFT_RIGHT)
new_targetmark >>= info->shift_bits; new_targetmark >>= info->shift_bits;
else else
...@@ -126,7 +128,7 @@ connmark_mt(const struct sk_buff *skb, struct xt_action_param *par) ...@@ -126,7 +128,7 @@ connmark_mt(const struct sk_buff *skb, struct xt_action_param *par)
if (ct == NULL) if (ct == NULL)
return false; return false;
return ((ct->mark & info->mask) == info->mark) ^ info->invert; return ((READ_ONCE(ct->mark) & info->mask) == info->mark) ^ info->invert;
} }
static int connmark_mt_check(const struct xt_mtchk_param *par) static int connmark_mt_check(const struct xt_mtchk_param *par)
......
...@@ -152,7 +152,7 @@ static u8 ovs_ct_get_state(enum ip_conntrack_info ctinfo) ...@@ -152,7 +152,7 @@ static u8 ovs_ct_get_state(enum ip_conntrack_info ctinfo)
static u32 ovs_ct_get_mark(const struct nf_conn *ct) static u32 ovs_ct_get_mark(const struct nf_conn *ct)
{ {
#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
return ct ? ct->mark : 0; return ct ? READ_ONCE(ct->mark) : 0;
#else #else
return 0; return 0;
#endif #endif
...@@ -340,9 +340,9 @@ static int ovs_ct_set_mark(struct nf_conn *ct, struct sw_flow_key *key, ...@@ -340,9 +340,9 @@ static int ovs_ct_set_mark(struct nf_conn *ct, struct sw_flow_key *key,
#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
u32 new_mark; u32 new_mark;
new_mark = ct_mark | (ct->mark & ~(mask)); new_mark = ct_mark | (READ_ONCE(ct->mark) & ~(mask));
if (ct->mark != new_mark) { if (READ_ONCE(ct->mark) != new_mark) {
ct->mark = new_mark; WRITE_ONCE(ct->mark, new_mark);
if (nf_ct_is_confirmed(ct)) if (nf_ct_is_confirmed(ct))
nf_conntrack_event_cache(IPCT_MARK, ct); nf_conntrack_event_cache(IPCT_MARK, ct);
key->ct.mark = new_mark; key->ct.mark = new_mark;
......
...@@ -61,7 +61,7 @@ static int tcf_connmark_act(struct sk_buff *skb, const struct tc_action *a, ...@@ -61,7 +61,7 @@ static int tcf_connmark_act(struct sk_buff *skb, const struct tc_action *a,
c = nf_ct_get(skb, &ctinfo); c = nf_ct_get(skb, &ctinfo);
if (c) { if (c) {
skb->mark = c->mark; skb->mark = READ_ONCE(c->mark);
/* using overlimits stats to count how many packets marked */ /* using overlimits stats to count how many packets marked */
ca->tcf_qstats.overlimits++; ca->tcf_qstats.overlimits++;
goto out; goto out;
...@@ -81,7 +81,7 @@ static int tcf_connmark_act(struct sk_buff *skb, const struct tc_action *a, ...@@ -81,7 +81,7 @@ static int tcf_connmark_act(struct sk_buff *skb, const struct tc_action *a,
c = nf_ct_tuplehash_to_ctrack(thash); c = nf_ct_tuplehash_to_ctrack(thash);
/* using overlimits stats to count how many packets marked */ /* using overlimits stats to count how many packets marked */
ca->tcf_qstats.overlimits++; ca->tcf_qstats.overlimits++;
skb->mark = c->mark; skb->mark = READ_ONCE(c->mark);
nf_ct_put(c); nf_ct_put(c);
out: out:
......
...@@ -178,7 +178,7 @@ static void tcf_ct_flow_table_add_action_meta(struct nf_conn *ct, ...@@ -178,7 +178,7 @@ static void tcf_ct_flow_table_add_action_meta(struct nf_conn *ct,
entry = tcf_ct_flow_table_flow_action_get_next(action); entry = tcf_ct_flow_table_flow_action_get_next(action);
entry->id = FLOW_ACTION_CT_METADATA; entry->id = FLOW_ACTION_CT_METADATA;
#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
entry->ct_metadata.mark = ct->mark; entry->ct_metadata.mark = READ_ONCE(ct->mark);
#endif #endif
ctinfo = dir == IP_CT_DIR_ORIGINAL ? IP_CT_ESTABLISHED : ctinfo = dir == IP_CT_DIR_ORIGINAL ? IP_CT_ESTABLISHED :
IP_CT_ESTABLISHED_REPLY; IP_CT_ESTABLISHED_REPLY;
...@@ -936,9 +936,9 @@ static void tcf_ct_act_set_mark(struct nf_conn *ct, u32 mark, u32 mask) ...@@ -936,9 +936,9 @@ static void tcf_ct_act_set_mark(struct nf_conn *ct, u32 mark, u32 mask)
if (!mask) if (!mask)
return; return;
new_mark = mark | (ct->mark & ~(mask)); new_mark = mark | (READ_ONCE(ct->mark) & ~(mask));
if (ct->mark != new_mark) { if (READ_ONCE(ct->mark) != new_mark) {
ct->mark = new_mark; WRITE_ONCE(ct->mark, new_mark);
if (nf_ct_is_confirmed(ct)) if (nf_ct_is_confirmed(ct))
nf_conntrack_event_cache(IPCT_MARK, ct); nf_conntrack_event_cache(IPCT_MARK, ct);
} }
......
...@@ -32,7 +32,7 @@ static void tcf_ctinfo_dscp_set(struct nf_conn *ct, struct tcf_ctinfo *ca, ...@@ -32,7 +32,7 @@ static void tcf_ctinfo_dscp_set(struct nf_conn *ct, struct tcf_ctinfo *ca,
{ {
u8 dscp, newdscp; u8 dscp, newdscp;
newdscp = (((ct->mark & cp->dscpmask) >> cp->dscpmaskshift) << 2) & newdscp = (((READ_ONCE(ct->mark) & cp->dscpmask) >> cp->dscpmaskshift) << 2) &
~INET_ECN_MASK; ~INET_ECN_MASK;
switch (proto) { switch (proto) {
...@@ -72,7 +72,7 @@ static void tcf_ctinfo_cpmark_set(struct nf_conn *ct, struct tcf_ctinfo *ca, ...@@ -72,7 +72,7 @@ static void tcf_ctinfo_cpmark_set(struct nf_conn *ct, struct tcf_ctinfo *ca,
struct sk_buff *skb) struct sk_buff *skb)
{ {
ca->stats_cpmark_set++; ca->stats_cpmark_set++;
skb->mark = ct->mark & cp->cpmarkmask; skb->mark = READ_ONCE(ct->mark) & cp->cpmarkmask;
} }
static int tcf_ctinfo_act(struct sk_buff *skb, const struct tc_action *a, static int tcf_ctinfo_act(struct sk_buff *skb, const struct tc_action *a,
...@@ -130,7 +130,7 @@ static int tcf_ctinfo_act(struct sk_buff *skb, const struct tc_action *a, ...@@ -130,7 +130,7 @@ static int tcf_ctinfo_act(struct sk_buff *skb, const struct tc_action *a,
} }
if (cp->mode & CTINFO_MODE_DSCP) if (cp->mode & CTINFO_MODE_DSCP)
if (!cp->dscpstatemask || (ct->mark & cp->dscpstatemask)) if (!cp->dscpstatemask || (READ_ONCE(ct->mark) & cp->dscpstatemask))
tcf_ctinfo_dscp_set(ct, ca, cp, skb, wlen, proto); tcf_ctinfo_dscp_set(ct, ca, cp, skb, wlen, proto);
if (cp->mode & CTINFO_MODE_CPMARK) if (cp->mode & CTINFO_MODE_CPMARK)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment