netdev - Re: [PATCH net] net: fix the address copy size to flow

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3be7af8bd3c768c691f368300f98db9c355b5018.camel@redhat.com>
Date:   Thu, 10 Nov 2022 13:15:23 +0100
From:   Paolo Abeni <pabeni@...hat.com>
To:     Hangbin Liu <liuhangbin@...il.com>, netdev@...r.kernel.org
Cc:     Jay Vosburgh <j.vosburgh@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Toppins <jtoppins@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Tom Herbert <tom@...bertland.com>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH net] net: fix the address copy size to flow_keys

Hello,

On Wed, 2022-11-09 at 10:44 +0800, Hangbin Liu wrote:
> kernel test robot reported a warning when build bonding module:
> 
>                  from ../drivers/net/bonding/bond_main.c:35:
> In function ‘fortify_memcpy_chk’,
>     inlined from ‘iph_to_flow_copy_v4addrs’ at ../include/net/ip.h:566:2,
>     inlined from ‘bond_flow_ip’ at ../drivers/net/bonding/bond_main.c:3984:3:
> ../include/linux/fortify-string.h:413:25: warning: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of f
> ield (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
>   413 |                         __read_overflow2_field(q_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘fortify_memcpy_chk’,
>     inlined from ‘iph_to_flow_copy_v6addrs’ at ../include/net/ipv6.h:900:2,
>     inlined from ‘bond_flow_ip’ at ../drivers/net/bonding/bond_main.c:3994:3:
> ../include/linux/fortify-string.h:413:25: warning: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of f
> ield (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
>   413 |                         __read_overflow2_field(q_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This is because we try to memcpy the whole ip/ip6 address to the flow_key,
> while we only point the to ip/ip6 saddr. It is efficient since we only need
> to do copy once for both saddr and daddr. But to fix the build warning,
> let's break the memcpy to 2 parts. This may affect bonding's performance
> slightly, but shouldn't too much.

The compiler should be able to unroll the memcpy loop in both cases,
but if it fails to do that, the performance hit could be measurable,
see commit 236222d39347e0e486010f10c1493e83dbbdfba8.

This looks like a straight struct_group() use-case, I suggest to use
the latter construct instead.

Cheers,

Paolo