mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-08-25 02:09:42 +00:00
netfilter: nft_exthdr: fix endianness of tcp option cast
[ Upstream commit 2e34328b39
]
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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
e4fd994f02
commit
4bf1941581
1 changed files with 3 additions and 5 deletions
|
@ -231,7 +231,6 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
|
||||||
unsigned int i, optl, tcphdr_len, offset;
|
unsigned int i, optl, tcphdr_len, offset;
|
||||||
struct tcphdr *tcph;
|
struct tcphdr *tcph;
|
||||||
u8 *opt;
|
u8 *opt;
|
||||||
u32 src;
|
|
||||||
|
|
||||||
tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
|
tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
|
||||||
if (!tcph)
|
if (!tcph)
|
||||||
|
@ -240,7 +239,6 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
|
||||||
opt = (u8 *)tcph;
|
opt = (u8 *)tcph;
|
||||||
for (i = sizeof(*tcph); i < tcphdr_len - 1; i += optl) {
|
for (i = sizeof(*tcph); i < tcphdr_len - 1; i += optl) {
|
||||||
union {
|
union {
|
||||||
u8 octet;
|
|
||||||
__be16 v16;
|
__be16 v16;
|
||||||
__be32 v32;
|
__be32 v32;
|
||||||
} old, new;
|
} old, new;
|
||||||
|
@ -262,13 +260,13 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
|
||||||
if (!tcph)
|
if (!tcph)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
src = regs->data[priv->sreg];
|
|
||||||
offset = i + priv->offset;
|
offset = i + priv->offset;
|
||||||
|
|
||||||
switch (priv->len) {
|
switch (priv->len) {
|
||||||
case 2:
|
case 2:
|
||||||
old.v16 = get_unaligned((u16 *)(opt + offset));
|
old.v16 = get_unaligned((u16 *)(opt + offset));
|
||||||
new.v16 = src;
|
new.v16 = (__force __be16)nft_reg_load16(
|
||||||
|
®s->data[priv->sreg]);
|
||||||
|
|
||||||
switch (priv->type) {
|
switch (priv->type) {
|
||||||
case TCPOPT_MSS:
|
case TCPOPT_MSS:
|
||||||
|
@ -286,7 +284,7 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
|
||||||
old.v16, new.v16, false);
|
old.v16, new.v16, false);
|
||||||
break;
|
break;
|
||||||
case 4:
|
case 4:
|
||||||
new.v32 = src;
|
new.v32 = regs->data[priv->sreg];
|
||||||
old.v32 = get_unaligned((u32 *)(opt + offset));
|
old.v32 = get_unaligned((u32 *)(opt + offset));
|
||||||
|
|
||||||
if (old.v32 == new.v32)
|
if (old.v32 == new.v32)
|
||||||
|
|
Loading…
Reference in a new issue