[PATCH nf-next 0/4] netfilter: nftables: tcp mss mangling support
This series adds the needed kernel parts to support tcp mss mangling. First two patches rework exthdr so we don't have to copy-paste too much, patch 3 adds tcp option mangling support. Last patch allows to retrieve path tcpmss via rt expression, this is so we can support iptables TCPMSS --clamp-to-pmtu by combining the two, i.e.: nft add rule inet mangle forward tcp option mss set rt mss -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH nf-next 1/4] netfilter: exthdr: factor out tcp option access
Signed-off-by: Florian Westphal
---
net/netfilter/nft_exthdr.c | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 1ec49fe5845f..921c95f2c583 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -61,6 +61,26 @@ static void nft_exthdr_ipv6_eval(const struct nft_expr *expr,
regs->verdict.code = NFT_BREAK;
}
+static void *
+nft_tcp_header_pointer(const struct nft_pktinfo *pkt,
+ unsigned int len, void *buffer, unsigned int *tcphdr_len)
+{
+ struct tcphdr *tcph;
+
+ if (!pkt->tprot_set || pkt->tprot != IPPROTO_TCP)
+ return NULL;
+
+ tcph = skb_header_pointer(pkt->skb, pkt->xt.thoff, sizeof(*tcph),
buffer);
+ if (!tcph)
+ return NULL;
+
+ *tcphdr_len = __tcp_hdrlen(tcph);
+ if (*tcphdr_len < sizeof(*tcph) || *tcphdr_len > len)
+ return NULL;
+
+ return skb_header_pointer(pkt->skb, pkt->xt.thoff, *tcphdr_len, buffer);
+}
+
static void nft_exthdr_tcp_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
@@ -72,18 +92,7 @@ static void nft_exthdr_tcp_eval(const struct nft_expr *expr,
struct tcphdr *tcph;
u8 *opt;
- if (!pkt->tprot_set || pkt->tprot != IPPROTO_TCP)
- goto err;
-
- tcph = skb_header_pointer(pkt->skb, pkt->xt.thoff, sizeof(*tcph), buff);
- if (!tcph)
- goto err;
-
- tcphdr_len = __tcp_hdrlen(tcph);
- if (tcphdr_len < sizeof(*tcph))
- goto err;
-
- tcph = skb_header_pointer(pkt->skb, pkt->xt.thoff, tcphdr_len, buff);
+ tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
if (!tcph)
goto err;
--
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH nf-next 4/4] netfilter: rt: add support to fetch path mss
to be used in combination with tcp option set support to mimic
iptables TCPMSS --clamp-mss-to-pmtu.
Signed-off-by: Florian Westphal
---
include/uapi/linux/netfilter/nf_tables.h | 2 +
net/netfilter/nft_rt.c | 65
2 files changed, 67 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h
b/include/uapi/linux/netfilter/nf_tables.h
index 40fd199f7531..b49da72efa68 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -811,11 +811,13 @@ enum nft_meta_keys {
* @NFT_RT_CLASSID: realm value of packet's route (skb->dst->tclassid)
* @NFT_RT_NEXTHOP4: routing nexthop for IPv4
* @NFT_RT_NEXTHOP6: routing nexthop for IPv6
+ * @NFT_RT_TCPMSS: fetch current path tcp mss
*/
enum nft_rt_keys {
NFT_RT_CLASSID,
NFT_RT_NEXTHOP4,
NFT_RT_NEXTHOP6,
+ NFT_RT_TCPMSS,
};
/**
diff --git a/net/netfilter/nft_rt.c b/net/netfilter/nft_rt.c
index c7383d8f88d0..69ed601d6fc6 100644
--- a/net/netfilter/nft_rt.c
+++ b/net/netfilter/nft_rt.c
@@ -23,6 +23,41 @@ struct nft_rt {
enum nft_registers dreg:8;
};
+static u16 get_tcpmss(const struct nft_pktinfo *pkt, const struct dst_entry
*skbdst)
+{
+ u32 minlen = sizeof(struct ipv6hdr), mtu = dst_mtu(skbdst);
+ const struct sk_buff *skb = pkt->skb;
+ const struct nf_afinfo *ai;
+ struct dst_entry *dst;
+ struct flowi fl;
+
+ memset(&fl, 0, sizeof(fl));
+
+ switch (nft_pf(pkt)) {
+ case NFPROTO_IPV4:
+ fl.u.ip4.daddr = ip_hdr(skb)->saddr;
+ minlen = sizeof(struct iphdr);
+ break;
+ case NFPROTO_IPV6:
+ fl.u.ip6.daddr = ipv6_hdr(skb)->saddr;
+ break;
+ }
+
+ ai = nf_get_afinfo(nft_pf(pkt));
+ if (ai)
+ ai->route(nft_net(pkt), &dst, &fl, false);
+
+ if (dst) {
+ mtu = min(mtu, dst_mtu(dst));
+ dst_release(dst);
+ }
+
+ if (mtu <= minlen || mtu > 0x)
+ return TCP_MSS_DEFAULT;
+
+ return mtu - minlen;
+}
+
static void nft_rt_get_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
@@ -57,6 +92,9 @@ static void nft_rt_get_eval(const struct nft_expr *expr,
&ipv6_hdr(skb)->daddr),
sizeof(struct in6_addr));
break;
+ case NFT_RT_TCPMSS:
+ nft_reg_store16(dest, get_tcpmss(pkt, dst));
+ break;
default:
WARN_ON(1);
goto err;
@@ -94,6 +132,9 @@ static int nft_rt_get_init(const struct nft_ctx *ctx,
case NFT_RT_NEXTHOP6:
len = sizeof(struct in6_addr);
break;
+ case NFT_RT_TCPMSS:
+ len = sizeof(u16);
+ break;
default:
return -EOPNOTSUPP;
}
@@ -118,6 +159,29 @@ static int nft_rt_get_dump(struct sk_buff *skb,
return -1;
}
+static int nft_rt_validate(const struct nft_ctx *ctx, const struct nft_expr
*expr,
+ const struct nft_data **data)
+{
+ const struct nft_rt *priv = nft_expr_priv(expr);
+ unsigned int hooks;
+
+ switch (priv->key) {
+ case NFT_RT_NEXTHOP4:
+ case NFT_RT_NEXTHOP6:
+ case NFT_RT_CLASSID:
+ return 0;
+ case NFT_RT_TCPMSS:
+ hooks = (1 << NF_INET_FORWARD) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return nft_chain_validate_hooks(ctx->chain, hooks);
+}
+
static struct nft_expr_type nft_rt_type;
static const struct nft_expr_ops nft_rt_get_ops = {
.type = &nft_rt_type,
@@ -125,6 +189,7 @@ static const struct nft_expr_ops nft_rt_get_ops = {
.eval = nft_rt_get_eval,
.init = nft_rt_get_init,
.dump = nft_rt_get_dump,
+ .validate = nft_rt_validate,
};
static struct nft_expr_type nft_rt_type __read_mostly = {
--
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH nf-next 2/4] netfilter: exthdr: split netlink dump function
so eval and uncoming eval_set versions can reuse a common helper.
Signed-off-by: Florian Westphal
---
net/netfilter/nft_exthdr.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 921c95f2c583..e3a6eebe7e0c 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -180,12 +180,8 @@ static int nft_exthdr_init(const struct nft_ctx *ctx,
NFT_DATA_VALUE, priv->len);
}
-static int nft_exthdr_dump(struct sk_buff *skb, const struct nft_expr *expr)
+static int nft_exthdr_dump_common(struct sk_buff *skb, const struct nft_exthdr
*priv)
{
- const struct nft_exthdr *priv = nft_expr_priv(expr);
-
- if (nft_dump_register(skb, NFTA_EXTHDR_DREG, priv->dreg))
- goto nla_put_failure;
if (nla_put_u8(skb, NFTA_EXTHDR_TYPE, priv->type))
goto nla_put_failure;
if (nla_put_be32(skb, NFTA_EXTHDR_OFFSET, htonl(priv->offset)))
@@ -202,6 +198,16 @@ static int nft_exthdr_dump(struct sk_buff *skb, const
struct nft_expr *expr)
return -1;
}
+static int nft_exthdr_dump(struct sk_buff *skb, const struct nft_expr *expr)
+{
+ const struct nft_exthdr *priv = nft_expr_priv(expr);
+
+ if (nft_dump_register(skb, NFTA_EXTHDR_DREG, priv->dreg))
+ return -1;
+
+ return nft_exthdr_dump_common(skb, priv);
+}
+
static struct nft_expr_type nft_exthdr_type;
static const struct nft_expr_ops nft_exthdr_ipv6_ops = {
.type = &nft_exthdr_type,
--
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH nf-next 3/4] netfilter: exthdr: tcp option set support
This allows setting 2 and 4 byte quantities in the tcp option space.
Main purpose is to allow native replacement for xt_TCPMSS to
work around pmtu blackholes.
Writes to kind and len are now allowed at the moment, it does not seem
useful to do this as it causes corruption of the tcp option space.
We can always lift this restriction later if a use-case appears.
Signed-off-by: Florian Westphal
---
include/uapi/linux/netfilter/nf_tables.h | 4 +-
net/netfilter/nft_exthdr.c | 164 ++-
2 files changed, 165 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/netfilter/nf_tables.h
b/include/uapi/linux/netfilter/nf_tables.h
index be25cf69295b..40fd199f7531 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -732,7 +732,8 @@ enum nft_exthdr_op {
* @NFTA_EXTHDR_OFFSET: extension header offset (NLA_U32)
* @NFTA_EXTHDR_LEN: extension header length (NLA_U32)
* @NFTA_EXTHDR_FLAGS: extension header flags (NLA_U32)
- * @NFTA_EXTHDR_OP: option match type (NLA_U8)
+ * @NFTA_EXTHDR_OP: option match type (NLA_U32)
+ * @NFTA_EXTHDR_SREG: option match type (NLA_U32)
*/
enum nft_exthdr_attributes {
NFTA_EXTHDR_UNSPEC,
@@ -742,6 +743,7 @@ enum nft_exthdr_attributes {
NFTA_EXTHDR_LEN,
NFTA_EXTHDR_FLAGS,
NFTA_EXTHDR_OP,
+ NFTA_EXTHDR_SREG,
__NFTA_EXTHDR_MAX
};
#define NFTA_EXTHDR_MAX(__NFTA_EXTHDR_MAX - 1)
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index e3a6eebe7e0c..f5a0bf5e3bdd 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -8,6 +8,7 @@
* Development of this code funded by Astaro AG (http://www.astaro.com/)
*/
+#include
#include
#include
#include
@@ -23,6 +24,7 @@ struct nft_exthdr {
u8 len;
u8 op;
enum nft_registers dreg:8;
+ enum nft_registers sreg:8;
u8 flags;
};
@@ -124,6 +126,88 @@ static void nft_exthdr_tcp_eval(const struct nft_expr
*expr,
regs->verdict.code = NFT_BREAK;
}
+static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt)
+{
+ u8 buff[sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE];
+ struct nft_exthdr *priv = nft_expr_priv(expr);
+ unsigned int i, optl, tcphdr_len, offset;
+ struct tcphdr *tcph;
+ u8 *opt;
+ u32 src;
+
+ tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
+ if (!tcph)
+ return;
+
+ opt = (u8 *)tcph;
+ for (i = sizeof(*tcph); i < tcphdr_len - 1; i += optl) {
+ union {
+ u8 octet;
+ __be16 v16;
+ __be32 v32;
+ } old, new;
+
+ optl = optlen(opt, i);
+
+ if (priv->type != opt[i])
+ continue;
+
+ if (i + optl > tcphdr_len || priv->len + priv->offset > optl)
+ return;
+
+ if (!skb_make_writable(pkt->skb, pkt->xt.thoff + i + priv->len))
+ return;
+
+ tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff,
+ &tcphdr_len);
+ if (!tcph)
+ return;
+
+ src = regs->data[priv->sreg];
+ offset = i + priv->offset;
+
+ switch (priv->len) {
+ case 2:
+ old.v16 = get_unaligned((u16 *)(opt + offset));
+ new.v16 = src;
+
+ switch (priv->type) {
+ case TCPOPT_MSS:
+ /* increase can cause connection to stall */
+ if (ntohs(old.v16) <= ntohs(new.v16))
+ return;
+ break;
+ }
+
+ if (old.v16 == new.v16)
+ return;
+
+ put_unaligned(new.v16, (u16*)(opt + offset));
+ inet_proto_csum_replace2(&tcph->check, pkt->skb,
+old.v16, new.v16, false);
+ break;
+ case 4:
+ new.v32 = src;
+ old.v32 = get_unaligned((u32 *)(opt + offset));
+
+ if (old.v32 == new.v32)
+ return;
+
+ put_unaligned(new.v32, (u32*)(opt + offset));
+ inet_proto_csum_replace4(&tcph->check, pkt->skb,
+old.v32, new.v32, false);
+ break;
+ default:
+ WARN_ON_ONCE(1)
Re: [PATCH nf-next 4/4] netfilter: rt: add support to fetch path mss
On Tue, 2017-08-08 at 15:15 +0200, Florian Westphal wrote:
> to be used in combination with tcp option set support to mimic
> iptables TCPMSS --clamp-mss-to-pmtu.
>
> Signed-off-by: Florian Westphal
> ---
> include/uapi/linux/netfilter/nf_tables.h | 2 +
> net/netfilter/nft_rt.c | 65
>
> 2 files changed, 67 insertions(+)
>
> diff --git a/include/uapi/linux/netfilter/nf_tables.h
> b/include/uapi/linux/netfilter/nf_tables.h
> index 40fd199f7531..b49da72efa68 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -811,11 +811,13 @@ enum nft_meta_keys {
> * @NFT_RT_CLASSID: realm value of packet's route (skb->dst->tclassid)
> * @NFT_RT_NEXTHOP4: routing nexthop for IPv4
> * @NFT_RT_NEXTHOP6: routing nexthop for IPv6
> + * @NFT_RT_TCPMSS: fetch current path tcp mss
> */
> enum nft_rt_keys {
> NFT_RT_CLASSID,
> NFT_RT_NEXTHOP4,
> NFT_RT_NEXTHOP6,
> + NFT_RT_TCPMSS,
> };
>
> /**
> diff --git a/net/netfilter/nft_rt.c b/net/netfilter/nft_rt.c
> index c7383d8f88d0..69ed601d6fc6 100644
> --- a/net/netfilter/nft_rt.c
> +++ b/net/netfilter/nft_rt.c
> @@ -23,6 +23,41 @@ struct nft_rt {
> enum nft_registers dreg:8;
> };
>
> +static u16 get_tcpmss(const struct nft_pktinfo *pkt, const struct dst_entry
> *skbdst)
> +{
> + u32 minlen = sizeof(struct ipv6hdr), mtu = dst_mtu(skbdst);
> + const struct sk_buff *skb = pkt->skb;
> + const struct nf_afinfo *ai;
> + struct dst_entry *dst;
> + struct flowi fl;
> +
> + memset(&fl, 0, sizeof(fl));
> +
> + switch (nft_pf(pkt)) {
> + case NFPROTO_IPV4:
> + fl.u.ip4.daddr = ip_hdr(skb)->saddr;
> + minlen = sizeof(struct iphdr);
> + break;
> + case NFPROTO_IPV6:
> + fl.u.ip6.daddr = ipv6_hdr(skb)->saddr;
> + break;
> + }
> +
> + ai = nf_get_afinfo(nft_pf(pkt));
> + if (ai)
> + ai->route(nft_net(pkt), &dst, &fl, false);
> +
if ai is NULL,
dst is not initialized and might contain garbage.
> + if (dst) {
> + mtu = min(mtu, dst_mtu(dst));
> + dst_release(dst);
> + }
> +
> + if (mtu <= minlen || mtu > 0x)
> + return TCP_MSS_DEFAULT;
> +
> + return mtu - minlen;
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 nf-next 4/4] netfilter: rt: add support to fetch path mss
to be used in combination with tcp option set support to mimic
iptables TCPMSS --clamp-mss-to-pmtu.
v2: Eric Dumazet points out dst must be initialized.
Signed-off-by: Florian Westphal
---
include/uapi/linux/netfilter/nf_tables.h | 2 +
net/netfilter/nft_rt.c | 66
2 files changed, 68 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h
b/include/uapi/linux/netfilter/nf_tables.h
index 40fd199f7531..b49da72efa68 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -811,11 +811,13 @@ enum nft_meta_keys {
* @NFT_RT_CLASSID: realm value of packet's route (skb->dst->tclassid)
* @NFT_RT_NEXTHOP4: routing nexthop for IPv4
* @NFT_RT_NEXTHOP6: routing nexthop for IPv6
+ * @NFT_RT_TCPMSS: fetch current path tcp mss
*/
enum nft_rt_keys {
NFT_RT_CLASSID,
NFT_RT_NEXTHOP4,
NFT_RT_NEXTHOP6,
+ NFT_RT_TCPMSS,
};
/**
diff --git a/net/netfilter/nft_rt.c b/net/netfilter/nft_rt.c
index c7383d8f88d0..e142e65d3176 100644
--- a/net/netfilter/nft_rt.c
+++ b/net/netfilter/nft_rt.c
@@ -23,6 +23,42 @@ struct nft_rt {
enum nft_registers dreg:8;
};
+static u16 get_tcpmss(const struct nft_pktinfo *pkt, const struct dst_entry
*skbdst)
+{
+ u32 minlen = sizeof(struct ipv6hdr), mtu = dst_mtu(skbdst);
+ const struct sk_buff *skb = pkt->skb;
+ const struct nf_afinfo *ai;
+ struct flowi fl;
+
+ memset(&fl, 0, sizeof(fl));
+
+ switch (nft_pf(pkt)) {
+ case NFPROTO_IPV4:
+ fl.u.ip4.daddr = ip_hdr(skb)->saddr;
+ minlen = sizeof(struct iphdr);
+ break;
+ case NFPROTO_IPV6:
+ fl.u.ip6.daddr = ipv6_hdr(skb)->saddr;
+ break;
+ }
+
+ ai = nf_get_afinfo(nft_pf(pkt));
+ if (ai) {
+ struct dst_entry *dst = NULL;
+
+ ai->route(nft_net(pkt), &dst, &fl, false);
+ if (dst) {
+ mtu = min(mtu, dst_mtu(dst));
+ dst_release(dst);
+ }
+ }
+
+ if (mtu <= minlen || mtu > 0x)
+ return TCP_MSS_DEFAULT;
+
+ return mtu - minlen;
+}
+
static void nft_rt_get_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
@@ -57,6 +93,9 @@ static void nft_rt_get_eval(const struct nft_expr *expr,
&ipv6_hdr(skb)->daddr),
sizeof(struct in6_addr));
break;
+ case NFT_RT_TCPMSS:
+ nft_reg_store16(dest, get_tcpmss(pkt, dst));
+ break;
default:
WARN_ON(1);
goto err;
@@ -94,6 +133,9 @@ static int nft_rt_get_init(const struct nft_ctx *ctx,
case NFT_RT_NEXTHOP6:
len = sizeof(struct in6_addr);
break;
+ case NFT_RT_TCPMSS:
+ len = sizeof(u16);
+ break;
default:
return -EOPNOTSUPP;
}
@@ -118,6 +160,29 @@ static int nft_rt_get_dump(struct sk_buff *skb,
return -1;
}
+static int nft_rt_validate(const struct nft_ctx *ctx, const struct nft_expr
*expr,
+ const struct nft_data **data)
+{
+ const struct nft_rt *priv = nft_expr_priv(expr);
+ unsigned int hooks;
+
+ switch (priv->key) {
+ case NFT_RT_NEXTHOP4:
+ case NFT_RT_NEXTHOP6:
+ case NFT_RT_CLASSID:
+ return 0;
+ case NFT_RT_TCPMSS:
+ hooks = (1 << NF_INET_FORWARD) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return nft_chain_validate_hooks(ctx->chain, hooks);
+}
+
static struct nft_expr_type nft_rt_type;
static const struct nft_expr_ops nft_rt_get_ops = {
.type = &nft_rt_type,
@@ -125,6 +190,7 @@ static const struct nft_expr_ops nft_rt_get_ops = {
.eval = nft_rt_get_eval,
.init = nft_rt_get_init,
.dump = nft_rt_get_dump,
+ .validate = nft_rt_validate,
};
static struct nft_expr_type nft_rt_type __read_mostly = {
--
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nf-next 4/4] netfilter: rt: add support to fetch path mss
Eric Dumazet wrote: > On Tue, 2017-08-08 at 15:15 +0200, Florian Westphal wrote: > > + struct dst_entry *dst; > > + struct flowi fl; [..] > > + ai = nf_get_afinfo(nft_pf(pkt)); > > + if (ai) > > + ai->route(nft_net(pkt), &dst, &fl, false); > > + > > if ai is NULL, > > dst is not initialized and might contain garbage. Right, thanks for pointing this out, I sent a v2. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH iptables 1/2] xtables-compat-restore: fix several memory leaks
The following memory leaks are detected by valgrind when ip[6]tables-compat-restore is executed: valgrind --leak-check=full iptables-compat-restore test-ruleset ==2548== 16 bytes in 1 blocks are definitely lost in loss record 1 of 20 ==2548==at 0x4C2DBC5: calloc (vg_replace_malloc.c:711) ==2548==by 0x4E39D67: __mnl_socket_open (socket.c:110) ==2548==by 0x4E39DDE: mnl_socket_open (socket.c:133) ==2548==by 0x11A48E: nft_init (nft.c:765) ==2548==by 0x11589F: xtables_restore_main (xtables-restore.c:463) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) ==2548== ==2548== 16 bytes in 1 blocks are definitely lost in loss record 2 of 20 ==2548==at 0x4C2DBC5: calloc (vg_replace_malloc.c:711) ==2548==by 0x504C7CD: nftnl_chain_list_alloc (chain.c:874) ==2548==by 0x11B2DB: nftnl_chain_list_get (nft.c:1194) ==2548==by 0x11B377: nft_chain_dump (nft.c:1210) ==2548==by 0x114DF9: get_chain_list (xtables-restore.c:167) ==2548==by 0x114EF8: xtables_restore_parse (xtables-restore.c:217) ==2548==by 0x115B43: xtables_restore_main (xtables-restore.c:526) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) ==2548== ==2548== 40 bytes in 1 blocks are definitely lost in loss record 5 of 20 ==2548==at 0x4C2DBC5: calloc (vg_replace_malloc.c:711) ==2548==by 0x56ABB99: xtables_calloc (xtables.c:291) ==2548==by 0x116DA7: command_jump (xtables.c:623) ==2548==by 0x117D5B: do_parse (xtables.c:923) ==2548==by 0x1188BA: do_commandx (xtables.c:1183) ==2548==by 0x115655: xtables_restore_parse (xtables-restore.c:405) ==2548==by 0x115B43: xtables_restore_main (xtables-restore.c:526) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) ==2548== ==2548== 40 bytes in 1 blocks are definitely lost in loss record 6 of 20 ==2548==at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2548==by 0x4E3AE07: mnl_nlmsg_batch_start (nlmsg.c:441) ==2548==by 0x1192B7: mnl_nftnl_batch_alloc (nft.c:106) ==2548==by 0x11931A: mnl_nftnl_batch_page_add (nft.c:122) ==2548==by 0x11DB0C: nft_action (nft.c:2402) ==2548==by 0x11DB65: nft_commit (nft.c:2413) ==2548==by 0x114FBB: xtables_restore_parse (xtables-restore.c:238) ==2548==by 0x115B43: xtables_restore_main (xtables-restore.c:526) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) ==2548== ==2548== 80 bytes in 5 blocks are definitely lost in loss record 8 of 20 ==2548==at 0x4C2DBC5: calloc (vg_replace_malloc.c:711) ==2548==by 0x50496FE: nftnl_table_list_alloc (table.c:433) ==2548==by 0x11DF88: nft_xtables_config_load (nft.c:2539) ==2548==by 0x11B037: nft_rule_append (nft.c:1116) ==2548==by 0x116639: add_entry (xtables.c:429) ==2548==by 0x118A3B: do_commandx (xtables.c:1187) ==2548==by 0x115655: xtables_restore_parse (xtables-restore.c:405) ==2548==by 0x115B43: xtables_restore_main (xtables-restore.c:526) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) ==2548== ==2548== 80 bytes in 5 blocks are definitely lost in loss record 9 of 20 ==2548==at 0x4C2DBC5: calloc (vg_replace_malloc.c:711) ==2548==by 0x504C7CD: nftnl_chain_list_alloc (chain.c:874) ==2548==by 0x11DF91: nft_xtables_config_load (nft.c:2540) ==2548==by 0x11B037: nft_rule_append (nft.c:1116) ==2548==by 0x116639: add_entry (xtables.c:429) ==2548==by 0x118A3B: do_commandx (xtables.c:1187) ==2548==by 0x115655: xtables_restore_parse (xtables-restore.c:405) ==2548==by 0x115B43: xtables_restore_main (xtables-restore.c:526) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) ==2548== ==2548== 135,168 bytes in 1 blocks are definitely lost in loss record 19 of 20 ==2548==at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2548==by 0x119280: mnl_nftnl_batch_alloc (nft.c:102) ==2548==by 0x11A51F: nft_init (nft.c:777) ==2548==by 0x11589F: xtables_restore_main (xtables-restore.c:463) ==2548==by 0x115B88: xtables_ip4_restore_main (xtables-restore.c:534) ==2548==by 0x12FF39: subcmd_main (xshared.c:211) ==2548==by 0x10F63C: main (xtables-compat-multi.c:41) An additional leak occurs if a rule-set already exits: ==2735== 375 (312 direct, 63 indirect) bytes in 3 blocks are definitely lost in loss
[PATCH iptables 2/2] xtables-compat: fix memory leak when listing
The following memory leaks are detected by valgrind when
ip[6]tables-compat is used for listing operations:
==1604== 1,064 (120 direct, 944 indirect) bytes in 5 blocks are definitely lost
in loss record 21 of 27
==1604==at 0x4C2BBEF: malloc (vg_replace_malloc.c:299)
==1604==by 0x56ABB78: xtables_malloc (in
/usr/local/lib/libxtables.so.12.0.0)
==1604==by 0x56AC7D3: xtables_find_match (in
/usr/local/lib/libxtables.so.12.0.0)
==1604==by 0x11F502: nft_parse_match (in
/usr/local/sbin/xtables-compat-multi)
==1604==by 0x11FC7B: nft_rule_to_iptables_command_state (in
/usr/local/sbin/xtables-compat-multi)
==1604==by 0x1218C0: nft_ipv4_print_firewall (nft-ipv4.c:301)
==1604==by 0x11CBEB: __nft_rule_list (nft.c:2042)
==1604==by 0x11CEA4: nft_rule_list (nft.c:2126)
==1604==by 0x116A7F: list_entries (xtables.c:592)
==1604==by 0x118B26: do_commandx (xtables.c:1233)
==1604==by 0x115AE8: xtables_main (in /usr/local/sbin/xtables-compat-multi)
==1604==by 0x115BCB: xtables_ip4_main (in
/usr/local/sbin/xtables-compat-multi)
==1604==
==1604== 135,168 bytes in 1 blocks are definitely lost in loss record 25 of 27
==1604==at 0x4C2BBEF: malloc (vg_replace_malloc.c:299)
==1604==by 0x119072: mnl_nftnl_batch_alloc (nft.c:102)
==1604==by 0x11A311: nft_init (nft.c:777)
==1604==by 0x115A71: xtables_main (in /usr/local/sbin/xtables-compat-multi)
==1604==by 0x115BCB: xtables_ip4_main (in
/usr/local/sbin/xtables-compat-multi)
==1604==by 0x12F911: subcmd_main (in /usr/local/sbin/xtables-compat-multi)
==1604==by 0x10F636: main (in /usr/local/sbin/xtables-compat-multi)
==1604==
==1604== 135,168 bytes in 1 blocks are definitely lost in loss record 26 of 27
==1604==at 0x4C2BBEF: malloc (vg_replace_malloc.c:299)
==1604==by 0x119072: mnl_nftnl_batch_alloc (nft.c:102)
==1604==by 0x11910C: mnl_nftnl_batch_page_add (nft.c:122)
==1604==by 0x11D8FE: nft_action (nft.c:2402)
==1604==by 0x11D957: nft_commit (nft.c:2413)
==1604==by 0x11CCB7: nft_rule_list (nft.c:2076)
==1604==by 0x116A7F: list_entries (xtables.c:592)
==1604==by 0x118B26: do_commandx (xtables.c:1233)
==1604==by 0x115AE8: xtables_main (in /usr/local/sbin/xtables-compat-multi)
==1604==by 0x115BCB: xtables_ip4_main (in
/usr/local/sbin/xtables-compat-multi)
==1604==by 0x12F911: subcmd_main (in /usr/local/sbin/xtables-compat-multi)
==1604==by 0x10F636: main (in /usr/local/sbin/xtables-compat-multi)
Fix these memory leaks.
Signed-off-by: Pablo M. Bermudo Garay
---
iptables/nft-ipv4.c | 2 ++
iptables/nft-ipv6.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index cf311513..00dd3e93 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -320,6 +320,8 @@ static void nft_ipv4_print_firewall(struct nftnl_rule *r,
unsigned int num,
if (!(format & FMT_NONEWLINE))
fputc('\n', stdout);
+
+ xtables_rule_matches_free(&cs.matches);
}
static void save_ipv4_addr(char letter, const struct in_addr *addr,
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 53526369..9867d1ee 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -251,6 +251,8 @@ static void nft_ipv6_print_firewall(struct nftnl_rule *r,
unsigned int num,
if (!(format & FMT_NONEWLINE))
fputc('\n', stdout);
+
+ xtables_rule_matches_free(&cs.matches);
}
static void save_ipv6_addr(char letter, const struct in6_addr *addr,
--
2.13.2
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net] net: sched: fix NULL pointer dereference when action calls some targets
On Mon, Aug 7, 2017 at 7:33 PM, Xin Long wrote: > On Tue, Aug 8, 2017 at 9:15 AM, Cong Wang wrote: >> This looks like a completely API burden? > netfilter xt targets are not really compatible with netsched action. > I've got to say, the patch is just a way to make checkentry return > false and avoid panic. like [1] said I don't doubt you fix a crash, I am thinking if we can "fix" the API instead of fixing the caller. I am not familiar with this API, so just my 2 cents... -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] netfilter: nf_nat_h323: fix logical-not-parentheses warning
bumping for review
On Mon, Jul 31, 2017 at 11:39 AM, Nick Desaulniers
wrote:
> Clang produces the following warning:
>
> net/ipv4/netfilter/nf_nat_h323.c:553:6: error:
> logical not is only applied to the left hand side of this comparison
> [-Werror,-Wlogical-not-parentheses]
> if (!set_h225_addr(skb, protoff, data, dataoff, taddr,
> ^
> add parentheses after the '!' to evaluate the comparison first
> add parentheses around left hand side expression to silence this warning
>
> There's not necessarily a bug here, but it's cleaner to use the form:
>
> if (x != 0)
>
> rather than:
>
> if (!x == 0)
>
> Signed-off-by: Nick Desaulniers
> ---
> Also, it's even cleaner to use the form:
>
> if (x)
>
> but then if the return codes change from treating 0 as success (unlikely),
> then all call sites must be updated.
>
> I'm happy to send v2 that changes to that form, and updates the other call
> sites to be:
>
> if (set_h225_addr())
> handle_failures()
> else
> handle_success()
>
> net/ipv4/netfilter/nf_nat_h323.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/netfilter/nf_nat_h323.c
> b/net/ipv4/netfilter/nf_nat_h323.c
> index 574f7ebba0b6..d8fb251fa6e3 100644
> --- a/net/ipv4/netfilter/nf_nat_h323.c
> +++ b/net/ipv4/netfilter/nf_nat_h323.c
> @@ -550,9 +550,9 @@ static int nat_callforwarding(struct sk_buff *skb, struct
> nf_conn *ct,
> }
>
> /* Modify signal */
> - if (!set_h225_addr(skb, protoff, data, dataoff, taddr,
> - &ct->tuplehash[!dir].tuple.dst.u3,
> - htons(nated_port)) == 0) {
> + if (set_h225_addr(skb, protoff, data, dataoff, taddr,
> + &ct->tuplehash[!dir].tuple.dst.u3,
> + htons(nated_port)) != 0) {
> nf_ct_unexpect_related(exp);
> return -1;
> }
> --
> 2.14.0.rc0.400.g1c36432dff-goog
>
--
Thanks,
~Nick Desaulniers
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
