* [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle
@ 2019-12-03 16:03 Laura Abbott
2019-12-03 17:01 ` Marcelo Ricardo Leitner
0 siblings, 1 reply; 6+ messages in thread
From: Laura Abbott @ 2019-12-03 16:03 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S. Miller
Cc: Laura Abbott, netfilter-devel, coreteam, netdev, linux-kernel, Kees Cook
The sizes for memcpy in flow_offload_mangle don't match
the source variables, leading to overflow errors on some
build configurations:
In function 'memcpy',
inlined from 'flow_offload_mangle' at net/netfilter/nf_flow_table_offload.c:112:2,
inlined from 'flow_offload_port_dnat' at net/netfilter/nf_flow_table_offload.c:373:2,
inlined from 'nf_flow_rule_route_ipv4' at net/netfilter/nf_flow_table_offload.c:424:3:
./include/linux/string.h:376:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
376 | __read_overflow2();
| ^~~~~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:266: net/netfilter/nf_flow_table_offload.o] Error 1
Fix this by using the corresponding type.
Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
Seen on a Fedora powerpc little endian build with -O3 but it looks like
it is correctly catching an error with doing a memcpy outside the source
variable.
---
net/netfilter/nf_flow_table_offload.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index c54c9a6cc981..526f894d0bdb 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -108,8 +108,8 @@ static void flow_offload_mangle(struct flow_action_entry *entry,
entry->id = FLOW_ACTION_MANGLE;
entry->mangle.htype = htype;
entry->mangle.offset = offset;
- memcpy(&entry->mangle.mask, mask, sizeof(u32));
- memcpy(&entry->mangle.val, value, sizeof(u32));
+ memcpy(&entry->mangle.mask, mask, sizeof(u8));
+ memcpy(&entry->mangle.val, value, sizeof(u8));
}
static inline struct flow_action_entry *
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle
2019-12-03 16:03 [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle Laura Abbott
@ 2019-12-03 17:01 ` Marcelo Ricardo Leitner
2019-12-03 20:49 ` Laura Abbott
0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-12-03 17:01 UTC (permalink / raw)
To: Laura Abbott
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel,
Kees Cook
On Tue, Dec 03, 2019 at 11:03:45AM -0500, Laura Abbott wrote:
> The sizes for memcpy in flow_offload_mangle don't match
> the source variables, leading to overflow errors on some
> build configurations:
>
> In function 'memcpy',
> inlined from 'flow_offload_mangle' at net/netfilter/nf_flow_table_offload.c:112:2,
> inlined from 'flow_offload_port_dnat' at net/netfilter/nf_flow_table_offload.c:373:2,
> inlined from 'nf_flow_rule_route_ipv4' at net/netfilter/nf_flow_table_offload.c:424:3:
> ./include/linux/string.h:376:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> 376 | __read_overflow2();
> | ^~~~~~~~~~~~~~~~~~
> make[2]: *** [scripts/Makefile.build:266: net/netfilter/nf_flow_table_offload.o] Error 1
>
> Fix this by using the corresponding type.
>
> Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> Seen on a Fedora powerpc little endian build with -O3 but it looks like
> it is correctly catching an error with doing a memcpy outside the source
> variable.
Hi,
It is right but the fix is not. In that call trace:
flow_offload_port_dnat() {
...
u32 mask = ~htonl(0xffff);
__be16 port;
...
flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
(u8 *)&port, (u8 *)&mask);
}
port should have a 32b storage as well, and aligned with the mask.
> ---
> net/netfilter/nf_flow_table_offload.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index c54c9a6cc981..526f894d0bdb 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -108,8 +108,8 @@ static void flow_offload_mangle(struct flow_action_entry *entry,
> entry->id = FLOW_ACTION_MANGLE;
> entry->mangle.htype = htype;
> entry->mangle.offset = offset;
> - memcpy(&entry->mangle.mask, mask, sizeof(u32));
> - memcpy(&entry->mangle.val, value, sizeof(u32));
^^^^^ ^^^ which is &port in the call above
> + memcpy(&entry->mangle.mask, mask, sizeof(u8));
> + memcpy(&entry->mangle.val, value, sizeof(u8));
This fix would cause it to copy only the first byte, which is not the
intention.
> }
>
> static inline struct flow_action_entry *
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle
2019-12-03 17:01 ` Marcelo Ricardo Leitner
@ 2019-12-03 20:49 ` Laura Abbott
2019-12-06 22:58 ` Justin Forbes
0 siblings, 1 reply; 6+ messages in thread
From: Laura Abbott @ 2019-12-03 20:49 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel,
Kees Cook
On 12/3/19 12:01 PM, Marcelo Ricardo Leitner wrote:
> On Tue, Dec 03, 2019 at 11:03:45AM -0500, Laura Abbott wrote:
>> The sizes for memcpy in flow_offload_mangle don't match
>> the source variables, leading to overflow errors on some
>> build configurations:
>>
>> In function 'memcpy',
>> inlined from 'flow_offload_mangle' at net/netfilter/nf_flow_table_offload.c:112:2,
>> inlined from 'flow_offload_port_dnat' at net/netfilter/nf_flow_table_offload.c:373:2,
>> inlined from 'nf_flow_rule_route_ipv4' at net/netfilter/nf_flow_table_offload.c:424:3:
>> ./include/linux/string.h:376:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
>> 376 | __read_overflow2();
>> | ^~~~~~~~~~~~~~~~~~
>> make[2]: *** [scripts/Makefile.build:266: net/netfilter/nf_flow_table_offload.o] Error 1
>>
>> Fix this by using the corresponding type.
>>
>> Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> Seen on a Fedora powerpc little endian build with -O3 but it looks like
>> it is correctly catching an error with doing a memcpy outside the source
>> variable.
>
> Hi,
>
> It is right but the fix is not. In that call trace:
>
> flow_offload_port_dnat() {
> ...
> u32 mask = ~htonl(0xffff);
> __be16 port;
> ...
> flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
> (u8 *)&port, (u8 *)&mask);
> }
>
> port should have a 32b storage as well, and aligned with the mask.
>
>> ---
>> net/netfilter/nf_flow_table_offload.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
>> index c54c9a6cc981..526f894d0bdb 100644
>> --- a/net/netfilter/nf_flow_table_offload.c
>> +++ b/net/netfilter/nf_flow_table_offload.c
>> @@ -108,8 +108,8 @@ static void flow_offload_mangle(struct flow_action_entry *entry,
>> entry->id = FLOW_ACTION_MANGLE;
>> entry->mangle.htype = htype;
>> entry->mangle.offset = offset;
>> - memcpy(&entry->mangle.mask, mask, sizeof(u32));
>> - memcpy(&entry->mangle.val, value, sizeof(u32));
> ^^^^^ ^^^ which is &port in the call above
>> + memcpy(&entry->mangle.mask, mask, sizeof(u8));
>> + memcpy(&entry->mangle.val, value, sizeof(u8));
>
> This fix would cause it to copy only the first byte, which is not the
> intention.
>
Thanks for the review. I took another look at fixing this and I
think it might be better for the maintainer or someone who is more
familiar with the code to fix this. I ended up down a rabbit
hole trying to get the types to work and I wasn't confident about
the casting.
Thanks,
Laura
>> }
>>
>> static inline struct flow_action_entry *
>> --
>> 2.21.0
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle
2019-12-03 20:49 ` Laura Abbott
@ 2019-12-06 22:58 ` Justin Forbes
2019-12-07 17:38 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Justin Forbes @ 2019-12-06 22:58 UTC (permalink / raw)
To: Laura Abbott
Cc: Marcelo Ricardo Leitner, Pablo Neira Ayuso, Jozsef Kadlecsik,
Florian Westphal, David S. Miller, netfilter-devel, coreteam,
netdev, LKML, Kees Cook
On Tue, Dec 3, 2019 at 2:50 PM Laura Abbott <labbott@redhat.com> wrote:
>
> On 12/3/19 12:01 PM, Marcelo Ricardo Leitner wrote:
> > On Tue, Dec 03, 2019 at 11:03:45AM -0500, Laura Abbott wrote:
> >> The sizes for memcpy in flow_offload_mangle don't match
> >> the source variables, leading to overflow errors on some
> >> build configurations:
> >>
> >> In function 'memcpy',
> >> inlined from 'flow_offload_mangle' at net/netfilter/nf_flow_table_offload.c:112:2,
> >> inlined from 'flow_offload_port_dnat' at net/netfilter/nf_flow_table_offload.c:373:2,
> >> inlined from 'nf_flow_rule_route_ipv4' at net/netfilter/nf_flow_table_offload.c:424:3:
> >> ./include/linux/string.h:376:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> >> 376 | __read_overflow2();
> >> | ^~~~~~~~~~~~~~~~~~
> >> make[2]: *** [scripts/Makefile.build:266: net/netfilter/nf_flow_table_offload.o] Error 1
> >>
> >> Fix this by using the corresponding type.
> >>
> >> Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
> >> Signed-off-by: Laura Abbott <labbott@redhat.com>
> >> ---
> >> Seen on a Fedora powerpc little endian build with -O3 but it looks like
> >> it is correctly catching an error with doing a memcpy outside the source
> >> variable.
> >
> > Hi,
> >
> > It is right but the fix is not. In that call trace:
> >
> > flow_offload_port_dnat() {
> > ...
> > u32 mask = ~htonl(0xffff);
> > __be16 port;
> > ...
> > flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
> > (u8 *)&port, (u8 *)&mask);
> > }
> >
> > port should have a 32b storage as well, and aligned with the mask.
> >
> >> ---
> >> net/netfilter/nf_flow_table_offload.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> >> index c54c9a6cc981..526f894d0bdb 100644
> >> --- a/net/netfilter/nf_flow_table_offload.c
> >> +++ b/net/netfilter/nf_flow_table_offload.c
> >> @@ -108,8 +108,8 @@ static void flow_offload_mangle(struct flow_action_entry *entry,
> >> entry->id = FLOW_ACTION_MANGLE;
> >> entry->mangle.htype = htype;
> >> entry->mangle.offset = offset;
> >> - memcpy(&entry->mangle.mask, mask, sizeof(u32));
> >> - memcpy(&entry->mangle.val, value, sizeof(u32));
> > ^^^^^ ^^^ which is &port in the call above
> >> + memcpy(&entry->mangle.mask, mask, sizeof(u8));
> >> + memcpy(&entry->mangle.val, value, sizeof(u8));
> >
> > This fix would cause it to copy only the first byte, which is not the
> > intention.
> >
>
> Thanks for the review. I took another look at fixing this and I
> think it might be better for the maintainer or someone who is more
> familiar with the code to fix this. I ended up down a rabbit
> hole trying to get the types to work and I wasn't confident about
> the casting.
>
Any update on this? It is definitely a problem on PPC LE.
Thanks,
Justin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle
2019-12-06 22:58 ` Justin Forbes
@ 2019-12-07 17:38 ` Pablo Neira Ayuso
2019-12-09 17:56 ` Justin Forbes
0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-07 17:38 UTC (permalink / raw)
To: Justin Forbes
Cc: Laura Abbott, Marcelo Ricardo Leitner, Jozsef Kadlecsik,
Florian Westphal, David S. Miller, netfilter-devel, coreteam,
netdev, LKML, Kees Cook
[-- Attachment #1: Type: text/plain, Size: 3380 bytes --]
On Fri, Dec 06, 2019 at 04:58:30PM -0600, Justin Forbes wrote:
> On Tue, Dec 3, 2019 at 2:50 PM Laura Abbott <labbott@redhat.com> wrote:
> >
> > On 12/3/19 12:01 PM, Marcelo Ricardo Leitner wrote:
> > > On Tue, Dec 03, 2019 at 11:03:45AM -0500, Laura Abbott wrote:
> > >> The sizes for memcpy in flow_offload_mangle don't match
> > >> the source variables, leading to overflow errors on some
> > >> build configurations:
> > >>
> > >> In function 'memcpy',
> > >> inlined from 'flow_offload_mangle' at net/netfilter/nf_flow_table_offload.c:112:2,
> > >> inlined from 'flow_offload_port_dnat' at net/netfilter/nf_flow_table_offload.c:373:2,
> > >> inlined from 'nf_flow_rule_route_ipv4' at net/netfilter/nf_flow_table_offload.c:424:3:
> > >> ./include/linux/string.h:376:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> > >> 376 | __read_overflow2();
> > >> | ^~~~~~~~~~~~~~~~~~
> > >> make[2]: *** [scripts/Makefile.build:266: net/netfilter/nf_flow_table_offload.o] Error 1
> > >>
> > >> Fix this by using the corresponding type.
> > >>
> > >> Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
> > >> Signed-off-by: Laura Abbott <labbott@redhat.com>
> > >> ---
> > >> Seen on a Fedora powerpc little endian build with -O3 but it looks like
> > >> it is correctly catching an error with doing a memcpy outside the source
> > >> variable.
> > >
> > > Hi,
> > >
> > > It is right but the fix is not. In that call trace:
> > >
> > > flow_offload_port_dnat() {
> > > ...
> > > u32 mask = ~htonl(0xffff);
> > > __be16 port;
> > > ...
> > > flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
> > > (u8 *)&port, (u8 *)&mask);
> > > }
> > >
> > > port should have a 32b storage as well, and aligned with the mask.
> > >
> > >> ---
> > >> net/netfilter/nf_flow_table_offload.c | 4 ++--
> > >> 1 file changed, 2 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > >> index c54c9a6cc981..526f894d0bdb 100644
> > >> --- a/net/netfilter/nf_flow_table_offload.c
> > >> +++ b/net/netfilter/nf_flow_table_offload.c
> > >> @@ -108,8 +108,8 @@ static void flow_offload_mangle(struct flow_action_entry *entry,
> > >> entry->id = FLOW_ACTION_MANGLE;
> > >> entry->mangle.htype = htype;
> > >> entry->mangle.offset = offset;
> > >> - memcpy(&entry->mangle.mask, mask, sizeof(u32));
> > >> - memcpy(&entry->mangle.val, value, sizeof(u32));
> > > ^^^^^ ^^^ which is &port in the call above
> > >> + memcpy(&entry->mangle.mask, mask, sizeof(u8));
> > >> + memcpy(&entry->mangle.val, value, sizeof(u8));
> > >
> > > This fix would cause it to copy only the first byte, which is not the
> > > intention.
> > >
> >
> > Thanks for the review. I took another look at fixing this and I
> > think it might be better for the maintainer or someone who is more
> > familiar with the code to fix this. I ended up down a rabbit
> > hole trying to get the types to work and I wasn't confident about
> > the casting.
>
> Any update on this? It is definitely a problem on PPC LE.
I'm attaching a tentative patch to address this problem.
Thanks.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2089 bytes --]
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 30205d57226d..cfa5602f54f5 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -336,23 +336,22 @@ static void flow_offload_port_snat(struct net *net,
struct nf_flow_rule *flow_rule)
{
struct flow_action_entry *entry = flow_action_entry_next(flow_rule);
- u32 mask = ~htonl(0xffff0000);
- __be16 port;
+ u32 mask = ~htonl(0xffff0000), port;
u32 offset;
switch (dir) {
case FLOW_OFFLOAD_DIR_ORIGINAL:
- port = flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_port;
+ port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_port);
offset = 0; /* offsetof(struct tcphdr, source); */
break;
case FLOW_OFFLOAD_DIR_REPLY:
- port = flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_port;
+ port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_port);
offset = 0; /* offsetof(struct tcphdr, dest); */
break;
default:
return;
}
-
+ port = htonl(port << 16);
flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
(u8 *)&port, (u8 *)&mask);
}
@@ -363,23 +362,22 @@ static void flow_offload_port_dnat(struct net *net,
struct nf_flow_rule *flow_rule)
{
struct flow_action_entry *entry = flow_action_entry_next(flow_rule);
- u32 mask = ~htonl(0xffff);
- __be16 port;
+ u32 mask = ~htonl(0xffff), port;
u32 offset;
switch (dir) {
case FLOW_OFFLOAD_DIR_ORIGINAL:
- port = flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_port;
+ port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_port);
offset = 0; /* offsetof(struct tcphdr, source); */
break;
case FLOW_OFFLOAD_DIR_REPLY:
- port = flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_port;
+ port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_port);
offset = 0; /* offsetof(struct tcphdr, dest); */
break;
default:
return;
}
-
+ port = htonl(port);
flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
(u8 *)&port, (u8 *)&mask);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle
2019-12-07 17:38 ` Pablo Neira Ayuso
@ 2019-12-09 17:56 ` Justin Forbes
0 siblings, 0 replies; 6+ messages in thread
From: Justin Forbes @ 2019-12-09 17:56 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Laura Abbott, Marcelo Ricardo Leitner, Jozsef Kadlecsik,
Florian Westphal, David S. Miller, netfilter-devel, coreteam,
netdev, LKML, Kees Cook
On Sat, Dec 7, 2019 at 11:38 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Fri, Dec 06, 2019 at 04:58:30PM -0600, Justin Forbes wrote:
> > On Tue, Dec 3, 2019 at 2:50 PM Laura Abbott <labbott@redhat.com> wrote:
> > >
> > > On 12/3/19 12:01 PM, Marcelo Ricardo Leitner wrote:
> > > > On Tue, Dec 03, 2019 at 11:03:45AM -0500, Laura Abbott wrote:
> > > >> The sizes for memcpy in flow_offload_mangle don't match
> > > >> the source variables, leading to overflow errors on some
> > > >> build configurations:
> > > >>
> > > >> In function 'memcpy',
> > > >> inlined from 'flow_offload_mangle' at net/netfilter/nf_flow_table_offload.c:112:2,
> > > >> inlined from 'flow_offload_port_dnat' at net/netfilter/nf_flow_table_offload.c:373:2,
> > > >> inlined from 'nf_flow_rule_route_ipv4' at net/netfilter/nf_flow_table_offload.c:424:3:
> > > >> ./include/linux/string.h:376:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> > > >> 376 | __read_overflow2();
> > > >> | ^~~~~~~~~~~~~~~~~~
> > > >> make[2]: *** [scripts/Makefile.build:266: net/netfilter/nf_flow_table_offload.o] Error 1
> > > >>
> > > >> Fix this by using the corresponding type.
> > > >>
> > > >> Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
> > > >> Signed-off-by: Laura Abbott <labbott@redhat.com>
> > > >> ---
> > > >> Seen on a Fedora powerpc little endian build with -O3 but it looks like
> > > >> it is correctly catching an error with doing a memcpy outside the source
> > > >> variable.
> > > >
> > > > Hi,
> > > >
> > > > It is right but the fix is not. In that call trace:
> > > >
> > > > flow_offload_port_dnat() {
> > > > ...
> > > > u32 mask = ~htonl(0xffff);
> > > > __be16 port;
> > > > ...
> > > > flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
> > > > (u8 *)&port, (u8 *)&mask);
> > > > }
> > > >
> > > > port should have a 32b storage as well, and aligned with the mask.
> > > >
> > > >> ---
> > > >> net/netfilter/nf_flow_table_offload.c | 4 ++--
> > > >> 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > > >> index c54c9a6cc981..526f894d0bdb 100644
> > > >> --- a/net/netfilter/nf_flow_table_offload.c
> > > >> +++ b/net/netfilter/nf_flow_table_offload.c
> > > >> @@ -108,8 +108,8 @@ static void flow_offload_mangle(struct flow_action_entry *entry,
> > > >> entry->id = FLOW_ACTION_MANGLE;
> > > >> entry->mangle.htype = htype;
> > > >> entry->mangle.offset = offset;
> > > >> - memcpy(&entry->mangle.mask, mask, sizeof(u32));
> > > >> - memcpy(&entry->mangle.val, value, sizeof(u32));
> > > > ^^^^^ ^^^ which is &port in the call above
> > > >> + memcpy(&entry->mangle.mask, mask, sizeof(u8));
> > > >> + memcpy(&entry->mangle.val, value, sizeof(u8));
> > > >
> > > > This fix would cause it to copy only the first byte, which is not the
> > > > intention.
> > > >
> > >
> > > Thanks for the review. I took another look at fixing this and I
> > > think it might be better for the maintainer or someone who is more
> > > familiar with the code to fix this. I ended up down a rabbit
> > > hole trying to get the types to work and I wasn't confident about
> > > the casting.
> >
> > Any update on this? It is definitely a problem on PPC LE.
>
> I'm attaching a tentative patch to address this problem.
>
This patch does in fact fix the build issue.
Thanks,
Justin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-09 17:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-03 16:03 [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle Laura Abbott
2019-12-03 17:01 ` Marcelo Ricardo Leitner
2019-12-03 20:49 ` Laura Abbott
2019-12-06 22:58 ` Justin Forbes
2019-12-07 17:38 ` Pablo Neira Ayuso
2019-12-09 17:56 ` Justin Forbes
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).