From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org
Subject: [PATCH 27/28] netfilter: nft_exthdr: fix endianness of tcp option cast
Date: Mon, 30 Mar 2020 21:21:35 +0200 [thread overview]
Message-ID: <20200330192136.230459-28-pablo@netfilter.org> (raw)
In-Reply-To: <20200330192136.230459-1-pablo@netfilter.org>
From: Sergey Marinkevich <sergey.marinkevich@eltex-co.ru>
I got a problem on MIPS with Big-Endian is turned on: every time when
NF trying to change TCP MSS it returns because of new.v16 was greater
than old.v16. But real MSS was 1460 and my rule was like this:
add rule table chain tcp option maxseg size set 1400
And 1400 is lesser that 1460, not greater.
Later I founded that main causer is cast from u32 to __be16.
Debugging:
In example MSS = 1400(HEX: 0x578). Here is representation of each byte
like it is in memory by addresses from left to right(e.g. [0x0 0x1 0x2
0x3]). LE — Little-Endian system, BE — Big-Endian, left column is type.
LE BE
u32: [78 05 00 00] [00 00 05 78]
As you can see, u32 representation will be casted to u16 from different
half of 4-byte address range. But actually nf_tables uses registers and
store data of various size. Actually TCP MSS stored in 2 bytes. But
registers are still u32 in definition:
struct nft_regs {
union {
u32 data[20];
struct nft_verdict verdict;
};
};
So, access like regs->data[priv->sreg] exactly u32. So, according to
table presents above, per-byte representation of stored TCP MSS in
register will be:
LE BE
(u32)regs->data[]: [78 05 00 00] [05 78 00 00]
^^ ^^
We see that register uses just half of u32 and other 2 bytes may be
used for some another data. But in nft_exthdr_tcp_set_eval() it casted
just like u32 -> __be16:
new.v16 = src
But u32 overfill __be16, so it get 2 low bytes. For clarity draw
one more table(<xx xx> means that bytes will be used for cast).
LE BE
u32: [<78 05> 00 00] [00 00 <05 78>]
(u32)regs->data[]: [<78 05> 00 00] [05 78 <00 00>]
As you can see, for Little-Endian nothing changes, but for Big-endian we
take the wrong half. In my case there is some other data instead of
zeros, so new MSS was wrongly greater.
For shooting this bug I used solution for ports ranges. Applying of this
patch does not affect Little-Endian systems.
Signed-off-by: Sergey Marinkevich <sergey.marinkevich@eltex-co.ru>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_exthdr.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index a5e8469859e3..07782836fad6 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -228,7 +228,6 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *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)
@@ -237,7 +236,6 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
opt = (u8 *)tcph;
for (i = sizeof(*tcph); i < tcphdr_len - 1; i += optl) {
union {
- u8 octet;
__be16 v16;
__be32 v32;
} old, new;
@@ -259,13 +257,13 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
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;
+ new.v16 = (__force __be16)nft_reg_load16(
+ ®s->data[priv->sreg]);
switch (priv->type) {
case TCPOPT_MSS:
@@ -283,7 +281,7 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
old.v16, new.v16, false);
break;
case 4:
- new.v32 = src;
+ new.v32 = regs->data[priv->sreg];
old.v32 = get_unaligned((u32 *)(opt + offset));
if (old.v32 == new.v32)
--
2.11.0
next prev parent reply other threads:[~2020-03-30 19:22 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-30 19:21 [PATCH 00/28] Netfilter/IPVS updates for net-next Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 01/28] netfilter: nf_tables: move nft_expr_clone() to nf_tables_api.c Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 02/28] netfilter: nf_tables: pass context to nft_set_destroy() Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 03/28] netfilter: nf_tables: allow to specify stateful expression in set definition Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 04/28] netfilter: nf_tables: fix double-free on set expression from the error path Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 05/28] netfilter: nf_tables: add nft_set_elem_expr_destroy() and use it Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 06/28] netfilter: flowtable: fix NULL pointer dereference in tunnel offload support Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 07/28] netfilter: ctnetlink: Add missing annotation for ctnetlink_parse_nat_setup() Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 08/28] netfilter: conntrack: Add missing annotations for nf_conntrack_all_lock() and nf_conntrack_all_unlock() Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 09/28] ipvs: optimize tunnel dumps for icmp errors Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 10/28] netfilter: conntrack: export nf_ct_acct_update() Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 11/28] netfilter: nf_tables: add enum nft_flowtable_flags to uapi Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 12/28] netfilter: flowtable: add counter support Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 13/28] netfilter: flowtable: Fix incorrect tc_setup_type type Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 14/28] netfilter: nf_tables: silence a RCU-list warning in nft_table_lookup() Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 15/28] netfilter: flowtable: Use rw sem as flow block lock Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 16/28] netfilter: flowtable: Use work entry per offload command Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 17/28] netfilter: nf_queue: make nf_queue_entry_release_refs static Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 18/28] netfilter: nf_queue: place bridge physports into queue_entry struct Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 19/28] netfilter: nf_queue: do not release refcouts until nf_reinject is done Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 20/28] netfilter: nf_queue: prefer nf_queue_entry_free Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 21/28] netfilter: ctnetlink: be more strict when NF_CONNTRACK_MARK is not set Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 22/28] netfilter: nft_set_bitmap: initialize set element extension in lookups Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 23/28] netfilter: nft_dynset: validate set expression definition Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 24/28] netfilter: nf_tables: skip set types that do not support for expressions Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 25/28] netfilter: conntrack: add nf_ct_acct_add() Pablo Neira Ayuso
2020-03-30 19:21 ` [PATCH 26/28] netfilter: flowtable: add counter support in HW offload Pablo Neira Ayuso
2020-03-30 19:21 ` Pablo Neira Ayuso [this message]
2020-03-30 19:21 ` [PATCH 28/28] ipvs: fix uninitialized variable warning Pablo Neira Ayuso
2020-03-31 3:11 ` [PATCH 00/28] Netfilter/IPVS updates for net-next David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200330192136.230459-28-pablo@netfilter.org \
--to=pablo@netfilter.org \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).