lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFxkdAqaOu98H5uaSr2DHrFoa+woB0ypt02y2rtau8x_gFj_eg@mail.gmail.com> Date: Mon, 9 Dec 2019 11:56:17 -0600 From: Justin Forbes <jmforbes@...uxtx.org> To: Pablo Neira Ayuso <pablo@...filter.org> Cc: Laura Abbott <labbott@...hat.com>, Marcelo Ricardo Leitner <marcelo.leitner@...il.com>, Jozsef Kadlecsik <kadlec@...filter.org>, Florian Westphal <fw@...len.de>, "David S. Miller" <davem@...emloft.net>, netfilter-devel@...r.kernel.org, coreteam@...filter.org, netdev@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>, Kees Cook <keescook@...omium.org> Subject: Re: [PATCH] netfilter: nf_flow_table_offload: Correct memcpy size for flow_overload_mangle On Sat, Dec 7, 2019 at 11:38 AM Pablo Neira Ayuso <pablo@...filter.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@...hat.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@...hat.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
Powered by blists - more mailing lists