Hi Hans,
On 9/26/23 11:27, Hans Verkuil wrote:
> If CONFIG_FORTIFY_SOURCE=y, then this warning is produced:
>
> In file included from ./include/linux/string.h:254,
> from ./include/linux/bitmap.h:11,
> from ./include/linux/cpumask.h:12,
> from ./arch/x86/include/asm/cpumask.h:5,
> from ./arch/x86/include/asm/msr.h:11,
> from ./arch/x86/include/asm/processor.h:23,
> 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 ./drivers/staging/media/atomisp//include/hmm/hmm.h:26,
> from drivers/staging/media/atomisp/pci/sh_css_params.c:26:
> In function ‘fortify_memcpy_chk’,
> inlined from ‘sh_css_store_sp_group_to_ddr’ at drivers/staging/media/atomisp/pci/sh_css_params.c:3736:3:
> ./include/linux/fortify-string.h:592: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]
> 592 | __read_overflow2_field(q_size_field, size);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The reason is that the memcpy copies two fields (each a u8), when the source
> pointer points to the first field. It's a bit unexpected, so just make this
> explicit.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> Hans, can you verify that it is indeed the intention of the original code
> to write both bytes?
Yes I believe so. I'm currently travelling so I cannot test this, but I think it should
be fine, see feel free to merge this and if it breaks things I'll get back to you once
I can actually run some tests.
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
>
> Note that I think that the initial memcpy is equally dubious. I think that
> should be replaced by '*buf_ptr++ = ...' lines as well, rather than just
> copying the first three fields with a memcpy. If you want I can make a v2
> that does that.
> ---
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> index 5667e855da76..232744973ab8 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> @@ -3733,8 +3733,8 @@ ia_css_ptr sh_css_store_sp_group_to_ddr(void)
> if (IS_ISP2401) {
> memcpy(buf_ptr, &sh_css_sp_group.config, 3);
> buf_ptr += 3;
> - memcpy(buf_ptr, &sh_css_sp_group.config.enable_isys_event_queue, 2);
> - buf_ptr += 2;
> + *buf_ptr++ = sh_css_sp_group.config.enable_isys_event_queue;
> + *buf_ptr++ = sh_css_sp_group.config.disable_cont_vf;
> memset(buf_ptr, 0, 3);
> buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
> } else {
>
