Re: [PATCH] ALSA: asihpi: work around clang-17+ false positive fortify-string warning

From: "Arnd Bergmann" <arnd@arndb.de>
To: "Takashi Iwai" <tiwai@suse.de>, "Arnd Bergmann" <arnd@kernel.org>
Cc: "Kees Cook" <keescook@chromium.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH] ALSA: asihpi: work around clang-17+ false positive fortify-string warning
Date: Wed, 28 Feb 2024 16:03:56 +0100	[thread overview]
Message-ID: <1265517d-b98a-4ec6-9215-10cd988ff78f@app.fastmail.com> (raw)
In-Reply-To: <87msrkhcz6.wl-tiwai@suse.de>

On Wed, Feb 28, 2024, at 15:37, Takashi Iwai wrote:
> On Wed, 28 Feb 2024 15:01:45 +0100,
> Arnd Bergmann wrote:
>> 
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> One of the memory copies in this driver triggers a warning about a
>> possible out of range access:
>> 
>> In file included from /home/arnd/arm-soc/sound/pci/asihpi/hpimsgx.c:13:
>> /home/arnd/arm-soc/include/linux/fortify-string.h:553: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]
>>   553 |                         __write_overflow_field(p_size_field, size);
>>       |                         ^
>
> Hmm, I can't see the relevance of those messages with the code you
> touched.  Do you have more info?

Not much. The warning is caused by this line:

        memcpy(&rESP_HPI_ADAPTER_OPEN[adapter], &hr,
                sizeof(rESP_HPI_ADAPTER_OPEN[0]));

rESP_HPI_ADAPTER_OPEN[] is a global array with a fixed
length of 20 elements, and 'adapter' is a 16-bit index
into that array. The warning is intended to trigger when
there is a code path that will overflow, but it normally
warns only if there is a known value range that the
variable can take, not for an unrestricted index.

My first thought was that clang warns about it here because
the 'u16 adapter' declaration limits the index to something
smaller than an 'int' or 'long', but changing the type
did not get rid of the warning.

>> Adding a range check avoids the problem, though I don't quite see
>> why it warns in the first place if clang has no knowledge of the
>> actual range of the type, or why I never saw the warning in previous
>> randconfig tests.
>
> It's indeed puzzling.  If it's really about adapter_prepare() call,
> the caller is only HPIMSGX__init(), and there is already an assignment
> with that index value beforehand:
>   hpi_entry_points[hr.u.s.adapter_index] = entry_point_func;
>
> and this array is also the size of HPI_MAX_ADAPTERS.  That is, the
> same check should have caught here...

The fortified-string warning only triggers for string.h operations
(memset, memcpy, memcmp, strn*...), not for a direct assignment.

      Arnd