linux-kernel - Re: [PATCH 10/12] sata: sx4: fix pdc20621_get_from

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d7fd5998-8813-4f29-a8d2-b82adbdec11c@kernel.org>
Date: Wed, 27 Mar 2024 10:36:28 +0900
From: Damien Le Moal <dlemoal@...nel.org>
To: Arnd Bergmann <arnd@...nel.org>, linux-kbuild@...r.kernel.org,
 Masahiro Yamada <masahiroy@...nel.org>, Niklas Cassel <cassel@...nel.org>
Cc: Nicolas Schier <nicolas@...sle.eu>, Nathan Chancellor
 <nathan@...nel.org>, Arnd Bergmann <arnd@...db.de>,
 John Garry <john.g.garry@...cle.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>,
 linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/12] sata: sx4: fix pdc20621_get_from_dimm() on 64-bit

On 3/26/24 23:53, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> gcc warns about a memcpy() with overlapping pointers because of an
> incorrect size calculation:
> 
> In file included from include/linux/string.h:369,
>                  from drivers/ata/sata_sx4.c:66:
> In function 'memcpy_fromio',
>     inlined from 'pdc20621_get_from_dimm.constprop' at drivers/ata/sata_sx4.c:962:2:
> include/linux/fortify-string.h:97:33: error: '__builtin_memcpy' accessing 4294934464 bytes at offsets 0 and [16, 16400] overlaps 6442385281 bytes at offset -2147450817 [-Werror=restrict]
>    97 | #define __underlying_memcpy     __builtin_memcpy
>       |                                 ^
> include/linux/fortify-string.h:620:9: note: in expansion of macro '__underlying_memcpy'
>   620 |         __underlying_##op(p, q, __fortify_size);                        \
>       |         ^~~~~~~~~~~~~
> include/linux/fortify-string.h:665:26: note: in expansion of macro '__fortify_memcpy_chk'
>   665 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
>       |                          ^~~~~~~~~~~~~~~~~~~~
> include/asm-generic/io.h:1184:9: note: in expansion of macro 'memcpy'
>  1184 |         memcpy(buffer, __io_virt(addr), size);
>       |         ^~~~~~
> 
> The problem here is the overflow of an unsigned 32-bit number to a
> negative that gets converted into a signed 'long', keeping a large
> positive number.
> 
> Replace the complex calculation with a more readable min() variant
> that avoids the warning.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

That is old :)

> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Looks good to me. I can take the patch through libata tree, unless you prefer
taking the whole series ?

In case it is the latter:

Acked-by: Damien Le Moal <dlemoal@...nel.org>

-- 
Damien Le Moal
Western Digital Research