exec/memory: Experimental API to catch unaligned accesses

[RFC PATCH 00/12] exec/memory: Experimental API to catch unaligned accesses

Posted by Philippe Mathieu-Daudé 2 years, 11 months ago

Hi,

This series is an experiment after chatting with Stefan and having
received review from Peter / Richard on an orthogonal series aiming
to handle unaligned pointers (atomically):
https://www.mail-archive.com/qemu-devel@nongnu.org/msg808954.html

Here we don't aim to fix unatomic accesses, however we are interested
in catching malicious unaligned accesses from guests.

For that we introduce the MemTxAttrs::aligned field which allow
accessors to return MEMTX_UNALIGNED_ERROR early enough, instead
of trying the unaligned access which can potentially trigger a
SIGBUS and abort the process.

To be able to pass/return alignment information we modify the
memory load/store cached API, then add the
virtio_ld/st*_phys_cached_with_attrs() handler (we only implement
lduw for this experiment).

Finaly we modify vring_avail_flags() to return whether the guest
requested an illegal (unaligned) memory access.

Note: The current virtio_ld/st*_phys_cached_with_attrs() API returns
the value, and take the MemTxResult as argument, so I choose to
return -1 (marked with /* XXX */ comment. We should switch to using
an API which returns a MemTxResult and takes the value accessed as
argument, this way we don't have to return random meaningless value.

But this is beyond the scope of this experiment, here we want to
emphasize the introduction of the MemTxAttrs::aligned field and the
MEMTX_UNALIGNED_ERROR return value.

Regards,

Phil.

Philippe Mathieu-Daudé (12):
  exec/memory_ldst: Use correct type sizes
  exec/memattrs: Add attribute/error for address alignment
  exec/memory_ldst: Return MEMTX_UNALIGNED_ERROR for unaligned addresses
  exec/memory_ldst_cached: Sort declarations
  exec/memory_ldst_cached: Use correct type size
  exec/memory_ldst_cached: Set MemTxResult on success
  exec/memory_ldst_cached: Document aligned addresses are expected
  exec/memory_ldst_cached: Check address alignment if requested
  hw/virtio: Use correct type sizes
  hw/virtio: Extract virtio_lduw_phys_cached_with_attrs()
  hw/virtio: Have vring_avail_flags() return a boolean value
  hw/virtio: Display error if vring flag field is not aligned

 include/exec/memattrs.h               |   3 +
 include/hw/virtio/virtio-access.h     |  39 +++++++--
 include/exec/memory_ldst.h.inc        |  16 ++--
 include/exec/memory_ldst_cached.h.inc | 114 ++++++++++++++++++++------
 hw/virtio/virtio.c                    |  22 ++++-
 memory_ldst.c.inc                     |  69 +++++++++++++---
 6 files changed, 211 insertions(+), 52 deletions(-)

-- 
2.26.3

Re: [RFC PATCH 00/12] exec/memory: Experimental API to catch unaligned accesses

Posted by Philippe Mathieu-Daudé 2 years, 10 months ago

Hi,

If there is any feedback, should I discard this experiment?

On 5/20/21 1:09 PM, Philippe Mathieu-Daudé wrote:
> This series is an experiment after chatting with Stefan and having
> received review from Peter / Richard on an orthogonal series aiming
> to handle unaligned pointers (atomically):
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg808954.html
> 
> Here we don't aim to fix unatomic accesses, however we are interested
> in catching malicious unaligned accesses from guests.
> 
> For that we introduce the MemTxAttrs::aligned field which allow
> accessors to return MEMTX_UNALIGNED_ERROR early enough, instead
> of trying the unaligned access which can potentially trigger a
> SIGBUS and abort the process.
> 
> To be able to pass/return alignment information we modify the
> memory load/store cached API, then add the
> virtio_ld/st*_phys_cached_with_attrs() handler (we only implement
> lduw for this experiment).
> 
> Finaly we modify vring_avail_flags() to return whether the guest
> requested an illegal (unaligned) memory access.
> 
> Note: The current virtio_ld/st*_phys_cached_with_attrs() API returns
> the value, and take the MemTxResult as argument, so I choose to
> return -1 (marked with /* XXX */ comment. We should switch to using
> an API which returns a MemTxResult and takes the value accessed as
> argument, this way we don't have to return random meaningless value.
> 
> But this is beyond the scope of this experiment, here we want to
> emphasize the introduction of the MemTxAttrs::aligned field and the
> MEMTX_UNALIGNED_ERROR return value.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (12):
>   exec/memory_ldst: Use correct type sizes
>   exec/memattrs: Add attribute/error for address alignment
>   exec/memory_ldst: Return MEMTX_UNALIGNED_ERROR for unaligned addresses
>   exec/memory_ldst_cached: Sort declarations
>   exec/memory_ldst_cached: Use correct type size
>   exec/memory_ldst_cached: Set MemTxResult on success
>   exec/memory_ldst_cached: Document aligned addresses are expected
>   exec/memory_ldst_cached: Check address alignment if requested
>   hw/virtio: Use correct type sizes
>   hw/virtio: Extract virtio_lduw_phys_cached_with_attrs()
>   hw/virtio: Have vring_avail_flags() return a boolean value
>   hw/virtio: Display error if vring flag field is not aligned
> 
>  include/exec/memattrs.h               |   3 +
>  include/hw/virtio/virtio-access.h     |  39 +++++++--
>  include/exec/memory_ldst.h.inc        |  16 ++--
>  include/exec/memory_ldst_cached.h.inc | 114 ++++++++++++++++++++------
>  hw/virtio/virtio.c                    |  22 ++++-
>  memory_ldst.c.inc                     |  69 +++++++++++++---
>  6 files changed, 211 insertions(+), 52 deletions(-)
>