linux-kernel - Re: [PATCH 2/3] media: dvb: Fix dtvs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f7ca4107-0341-4631-8d8d-b9677782ac2f@xs4all.nl>
Date: Fri, 12 Apr 2024 16:21:40 +0200
From: Hans Verkuil <hverkuil-cisco@...all.nl>
To: Ricardo Ribalda <ribalda@...omium.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] media: dvb: Fix dtvs_stats packing.

On 10/04/2024 14:24, Ricardo Ribalda wrote:
> The structure is packed, which requires that all its fields need to be
> also packed.
> 
> ./include/uapi/linux/dvb/frontend.h:854:2: warning: field  within 'struct dtv_stats' is less aligned than 'union dtv_stats::(anonymous at ./include/uapi/linux/dvb/frontend.h:854:2)' and is usually due to 'struct dtv_stats' being packed, which can lead to unaligned accesses [-Wunaligned-access]
> 
> Explicitly set the inner union as packed.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
>  include/uapi/linux/dvb/frontend.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
> index 7e0983b987c2d..8d38c6befda8d 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -854,7 +854,7 @@ struct dtv_stats {
>  	union {
>  		__u64 uvalue;	/* for counters and relative scales */
>  		__s64 svalue;	/* for 0.001 dB measures */
> -	};
> +	}  __attribute__ ((packed));
>  } __attribute__ ((packed));

This is used in the public API, and I think this change can cause ABI changes.

Can you compare the layouts? Also between gcc and llvm since gcc never warned
about this.

I'm not going to accept this unless it is clear that there are no ABI changes.

Note that the ABI test in the build scripts only tests V4L2 at the moment,
not the DVB API.

Regards,

	Hans