[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87il28h7zs.wl-tiwai@suse.de>
Date: Wed, 28 Feb 2024 17:24:39 +0100
From: Takashi Iwai <tiwai@...e.de>
To: "Arnd Bergmann" <arnd@...db.de>
Cc: "Takashi Iwai" <tiwai@...e.de>,
"Arnd Bergmann" <arnd@...nel.org>,
"Kees Cook" <keescook@...omium.org>,
"Jaroslav Kysela" <perex@...ex.cz>,
"Takashi Iwai" <tiwai@...e.com>,
"Nathan Chancellor" <nathan@...nel.org>,
"Nick Desaulniers" <ndesaulniers@...gle.com>,
"Bill Wendling" <morbo@...gle.com>,
"Justin Stitt" <justinstitt@...gle.com>,
linux-sound@...r.kernel.org,
linux-kernel@...r.kernel.org,
llvm@...ts.linux.dev
Subject: Re: [PATCH] ALSA: asihpi: work around clang-17+ false positive fortify-string warning
On Wed, 28 Feb 2024 16:03:56 +0100,
Arnd Bergmann wrote:
>
> 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@...db.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.
Ah, I see. Then from the logical POV, it's better to have a check
before that assignment; otherwise it'd overflow silently there.
Does putting the check beforehand (like the one below) fix similarly?
thanks,
Takashi
--- a/sound/pci/asihpi/hpimsgx.c
+++ b/sound/pci/asihpi/hpimsgx.c
@@ -708,7 +708,8 @@ static u16 HPIMSGX__init(struct hpi_message *phm,
phr->error = HPI_ERROR_PROCESSING_MESSAGE;
return phr->error;
}
- if (hr.error == 0) {
+ if (hr.error == 0 &&
+ hr.u.s.adapter_index < ARRAY_SIZE(rESP_HPI_ADAPTER_OPEN)) {
/* the adapter was created successfully
save the mapping for future use */
hpi_entry_points[hr.u.s.adapter_index] = entry_point_func;
Powered by blists - more mailing lists