linux-kernel - Re: [PATCH] [v2] scsi: ipr: work around fortify-string warning

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: <63ed4f65.170a0220.b2f75.8a8f@mx.google.com>
Date:   Wed, 15 Feb 2023 13:32:20 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Brian King <brking@...ibm.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        James Bottomley <James.Bottomley@...eleye.com>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        Arnd Bergmann <arnd@...db.de>,
        Niklas Cassel <niklas.cassel@....com>,
        John Garry <john.g.garry@...cle.com>,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH] [v2] scsi: ipr: work around fortify-string warning

On Tue, Feb 14, 2023 at 02:28:08PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> The ipr_log_vpd_compact() function triggers a fortified memcpy() warning
> about a potential string overflow with all versions of clang:
> 
> In file included from drivers/scsi/ipr.c:43:
> In file included from include/linux/string.h:254:
> include/linux/fortify-string.h:520:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
>                         __write_overflow_field(p_size_field, size);
>                         ^
> include/linux/fortify-string.h:520:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
> 2 errors generated.
> 
> I don't see anything actually wrong with the function, but this is the
> only instance I can reproduce of the fortification going wrong in the
> kernel at the moment, so the easiest solution may be to rewrite the
> function into something that does not trigger the warning.
> 
> Instead of having a combined buffer for vendor/device/serial strings,
> use three separate local variables and just truncate the whitespace
> individually.
> 
> Fixes: 8cf093e275d0 ("[SCSI] ipr: Improved dual adapter errors")
> Cc: Kees Cook <keescook@...omium.org>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Reviewed-by: Kees Cook <keescook@...omium.org>

Reproduced this locally -- I agree your fix looks like the best
approach. I think Clang was seeing the old "i + 2" return as potentially
overflowing in the case where there was no space-padding on any strings.

-- 
Kees Cook