Re: [PATCH 0/9] xen/arm: Enable UBSAN support

[Julien Grall <julien@xxxxxxx>]

(+ The rest)

Hi Jan,

On 26/06/2023 06:45, Jan Beulich wrote:
> On 25.06.2023 22:48, Julien Grall wrote:
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> >
> > Hi all,
> >
> > At the moment, we are not able to enable UBSAN on Arm because the
> > final binary will be over the maximum size of Xen we currently support
> > (i.e. 2MB).
> >
> > This patch series aim to lift the restrictions and also
> > enable UBSAN. Lastly, at the end of the series, there is the first
> > issue found by USBAN.
> >
> > There are a few of others. One will be fixed by the MISRA work
> > in [1] and the other is a bit tricky. One the splat is (the
> > others seems to be for similar reasons)
> >
> > (XEN) 
> > ================================================================================
> > (XEN) UBSAN: Undefined behaviour in common/sched/credit2.c:2437:5
> > (XEN) member access within misaligned address 43feefbc for type 'struct 
> > csched2_runqueue_data'
> > (XEN) which requires 8 byte alignment
> > (XEN) Xen WARN at common/ubsan/ubsan.c:172
> >
> > This is on 32-bit and UBSAN seems to complain about the check in
> > list_for_each_entry. I haven't yet dived into the issue yet.
> At a guess this is because the list head struct, living in
> struct csched2_private, has only 32-bit alignment, while on the
> last loop iteration pos doesn't hold a real struct
> csched2_runqueue_data * (which ought to b 64-bit aligned, but
> isn't in the special case of having reached the list head again).
> If I'm not mistaken rwlock_t is 12 bytes for Arm32, which would
> match with the address above ending in c, assuming that xmalloc()
> returns at least 16-byte-aligned space on Arm32 as well. If so,
> in this particular case some rearrangement of struct
> csched2_private ought to help, but as you say this is a more
> general issue and hence likely wants addressing in a generic way.
Your analysis is correct. So far I have only seen the splats in the 
credit2 code. But it feels a little be odd to fix only there.
Our list implementation is based on the Linux one. I checked a recent 
version but I couldn't find any hints that they may have fixed the issue.
For a more generic fix, I was thinking to force the alignment of the 
list head to 8-bytes. This is not perfect but would cover most of the 
use-cases where list head is used.
Cheers,

--
Julien Grall