[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



(+ 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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.