| 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: <fbdc3741b28a0174e3058a998e253439@codethink.co.uk>
Date: Fri, 23 Jun 2023 10:43:12 +0100
From: Ben Dooks <ben.dooks@...ethink.co.uk>
To: Simon Horman <simon.horman@...igine.com>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, kuba@...nel.org,
edumazet@...gle.com, davem@...emloft.net,
linux-kernel@...r.kernel.org, claudiu.beznea@...rochip.com,
nicolas.ferre@...rochip.com
Subject: Re: [PATCH 3/3] net: macb: fix __be32 warnings in debug code
On 2023-06-22 16:44, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 02:05:07PM +0100, Ben Dooks wrote:
>> The netdev_dbg() calls in gem_add_flow_filter() and
>> gem_del_flow_filter()
>> call ntohl() on the ipv4 addresses, which will put them into the host
>> order
>> but not the right type (returns a __be32, not an u32 as would be
>> expected).
>>
>> Chaning the htonl() to nthol() should still do the right conversion,
>> return
>> the correct u32 type and should not change any functional to remove
>> the
>> following sparse warnings:
>>
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: incorrect
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: expected unsigned
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: got restricted
>> __be32 [usertype] ip4src
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: incorrect
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: expected unsigned
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: got restricted
>> __be32 [usertype] ip4dst
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from
>> restricted __be32
>> d
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: incorrect
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: expected unsigned
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: got restricted
>> __be32 [usertype] ip4src
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: incorrect
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: expected unsigned
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: got restricted
>> __be32 [usertype] ip4dst
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from
>> restricted __be32
>>
>> Signed-off-by: Ben Dooks <ben.dooks@...ethink.co.uk>
>
> Hi Ben,
>
> this code-change looks good to me, but I have a few minor nits for your
> consideration.
>
> 1. Please specify the target tree, in this case net-next, for patch
> sets
> for Networking code.
>
> Subject: [PATCH net-next ...] ...
Ah, was using net, but I assume net-next is probably ok
> 2. It might be nicer to write '.../macb_main.c' or similar,
> rather tha nthe full path, in the patch description.
>
> 3. checkpatch --codespell says: 'Chaning' -> 'Chaining'
Ok, thank you. I didn't know about that.
Since there's another patch that needs work I'll re-send this early next
week
with the fixes in.
>> ---
>> drivers/net/ethernet/cadence/macb_main.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index 56e202b74bd7..59a90c2b307f 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -3568,8 +3568,8 @@ static int gem_add_flow_filter(struct net_device
>> *netdev,
>> netdev_dbg(netdev,
>> "Adding flow filter
>> entry,type=%u,queue=%u,loc=%u,src=%08X,dst=%08X,ps=%u,pd=%u\n",
>> fs->flow_type, (int)fs->ring_cookie, fs->location,
>> - htonl(fs->h_u.tcp_ip4_spec.ip4src),
>> - htonl(fs->h_u.tcp_ip4_spec.ip4dst),
>> + ntohl(fs->h_u.tcp_ip4_spec.ip4src),
>> + ntohl(fs->h_u.tcp_ip4_spec.ip4dst),
>> be16_to_cpu(fs->h_u.tcp_ip4_spec.psrc),
>> be16_to_cpu(fs->h_u.tcp_ip4_spec.pdst));
>>
>> @@ -3622,8 +3622,8 @@ static int gem_del_flow_filter(struct net_device
>> *netdev,
>> netdev_dbg(netdev,
>> "Deleting flow filter
>> entry,type=%u,queue=%u,loc=%u,src=%08X,dst=%08X,ps=%u,pd=%u\n",
>> fs->flow_type, (int)fs->ring_cookie, fs->location,
>> - htonl(fs->h_u.tcp_ip4_spec.ip4src),
>> - htonl(fs->h_u.tcp_ip4_spec.ip4dst),
>> + ntohl(fs->h_u.tcp_ip4_spec.ip4src),
>> + ntohl(fs->h_u.tcp_ip4_spec.ip4dst),
>> be16_to_cpu(fs->h_u.tcp_ip4_spec.psrc),
>> be16_to_cpu(fs->h_u.tcp_ip4_spec.pdst));
>>
>> --
>> 2.40.1
>>
>>
Powered by blists - more mailing lists