On Fri, Dec 1, 2023 at 12:29 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> On Tue, Nov 28, 2023 at 05:52:10AM +0300, Dmitry Antipov wrote:
> > When compiling with gcc version 14.0.0 20231126 (experimental)
> > and CONFIG_FORTIFY_SOURCE=y, I've noticed the following:
> >
> > In file included from ./include/linux/string.h:295,
> > from ./include/linux/bitmap.h:12,
> > from ./include/linux/cpumask.h:12,
> > from ./arch/x86/include/asm/paravirt.h:17,
> > from ./arch/x86/include/asm/cpuid.h:62,
> > from ./arch/x86/include/asm/processor.h:19,
> > from ./arch/x86/include/asm/cpufeature.h:5,
> > from ./arch/x86/include/asm/thread_info.h:53,
> > from ./include/linux/thread_info.h:60,
> > from ./arch/x86/include/asm/preempt.h:9,
> > from ./include/linux/preempt.h:79,
> > from ./include/linux/spinlock.h:56,
> > from ./include/linux/mmzone.h:8,
> > from ./include/linux/gfp.h:7,
> > from ./include/linux/slab.h:16,
> > from ./include/linux/resource_ext.h:11,
> > from ./include/linux/acpi.h:13,
> > from drivers/pnp/pnpacpi/rsparser.c:11:
> > In function 'fortify_memcpy_chk',
> > inlined from 'pnpacpi_parse_allocated_vendor' at drivers/pnp/pnpacpi/rsparser.c:158:3,
> > inlined from 'pnpacpi_allocated_resource' at drivers/pnp/pnpacpi/rsparser.c:249:3:
> > ./include/linux/fortify-string.h:588:25: warning: call to '__read_overflow2_field'
> > declared with attribute warning: detected read beyond size of field (2nd parameter);
> > maybe use struct_group()? [-Wattribute-warning]
> > 588 | __read_overflow2_field(q_size_field, size);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > According to the comments in include/linux/fortify-string.h, 'memcpy()',
> > 'memmove()' and 'memset()' must not be used beyond individual struct
> > members to ensure that the compiler can enforce protection against
> > buffer overflows, and, IIUC, this also applies to partial copies from
> > the particular member ('vendor->byte_data' in this case). So it should
> > be better (and safer) to do both copies at once (and 'byte_data' of
> > 'struct acpi_resource_vendor_typed' seems to be a good candidate for
> > '__counted_by(byte_length)' as well).
> >
> > Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx>
> > ---
> > v2: prefer sizeof(range) over hardcoded constant (Rafael J. Wysocki)
>
> Yeah, this looks good to me. Thanks for the fix!
>
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
Applied as 6.8 material, thanks!
