On Mon, Nov 27, 2023 at 4:00 PM Dmitry Antipov <dmantipov@xxxxxxxxx> 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>
> ---
> drivers/pnp/pnpacpi/rsparser.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
> index 4f05f610391b..af51f893525e 100644
> --- a/drivers/pnp/pnpacpi/rsparser.c
> +++ b/drivers/pnp/pnpacpi/rsparser.c
> @@ -152,12 +152,12 @@ static void pnpacpi_parse_allocated_vendor(struct pnp_dev *dev,
> struct acpi_resource_vendor_typed *vendor)
> {
> if (vendor_resource_matches(dev, vendor, &hp_ccsr_uuid, 16)) {
> - u64 start, length;
> + struct { u64 start, length; } range;
>
> - memcpy(&start, vendor->byte_data, sizeof(start));
> - memcpy(&length, vendor->byte_data + 8, sizeof(length));
> + memcpy(&range, vendor->byte_data, 16);
sizeof(range) instead of 16 here?
>
> - pnp_add_mem_resource(dev, start, start + length - 1, 0);
> + pnp_add_mem_resource(dev, range.start, range.start +
> + range.length - 1, 0);
> }
> }
>
> --
