another tst-arm-mte bug: qemu-system segfaults

another tst-arm-mte bug: qemu-system segfaults

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

again i'm using the branch at

https://github.com/rth7680/qemu/tree/tgt-arm-mte

to test armv8.5-a mte, now qemu-system-aarch64 segfaults
and it's easy to reproduce: minimal source and static
linked binary is attached (should be executed on linux
with mte support, i used mte-v4 kernel with reverted
memory property check).

the __aarch64_memchr function referenced is from
https://github.com/ARM-software/optimized-routines

and the bug was originally observed when running the
test executables built with gcc-10 and a config.mk like

SUBS = string
ARCH = aarch64
srcdir = .
CROSS_COMPILE = aarch64-none-linux-gnu-
CC = $(CROSS_COMPILE)gcc
CFLAGS = -std=c99 -pipe -O3
CFLAGS += -march=armv8.5-a+memtag -DWANT_MTE_TEST=1
CFLAGS += -Wall -Wno-missing-braces
CFLAGS += -Werror=implicit-function-declaration
CFLAGS += -g

[-- Attachment #2: bug.c --]
[-- Type: text/x-csrc, Size: 682 bytes --]

#include <stddef.h>
#include <sys/mman.h>
#include <sys/prctl.h>

void *__memchr_aarch64 (const void *, int, size_t);

#define PR_SET_TAGGED_ADDR_CTRL 55
#define PR_TAGGED_ADDR_ENABLE (1UL << 0)
#define PR_MTE_TCF_SHIFT 1
#define PR_MTE_TCF_SYNC (1UL << PR_MTE_TCF_SHIFT)
#define PR_MTE_TAG_SHIFT 3
#define PROT_MTE 0x20

int main (void)
{
  int r = prctl (PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | (0xfffe << PR_MTE_TAG_SHIFT), 0, 0, 0);
  if (r < 0) return -1;
  char *s = mmap (NULL, 4096, PROT_READ | PROT_WRITE | PROT_MTE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  if (s == MAP_FAILED) return -1;
  s[0] = 1;
  __memchr_aarch64(s, 1, 4096);
  return 0;
}

[-- Attachment #3: bug --]
[-- Type: application/octet-stream, Size: 9072 bytes --]

Re: another tst-arm-mte bug: qemu-system segfaults

[Richard Henderson]

On 5/29/20 5:04 AM, Szabolcs Nagy wrote:
> again i'm using the branch at
> 
> https://github.com/rth7680/qemu/tree/tgt-arm-mte
> 
> to test armv8.5-a mte, now qemu-system-aarch64 segfaults
> and it's easy to reproduce: minimal source and static
> linked binary is attached (should be executed on linux
> with mte support, i used mte-v4 kernel with reverted
> memory property check).
> 
> the __aarch64_memchr function referenced is from
> https://github.com/ARM-software/optimized-routines
> 
> and the bug was originally observed when running the
> test executables built with gcc-10 and a config.mk like

Thanks.  I've been able to reliably reproduce.


r~

Re: another tst-arm-mte bug: qemu-system segfaults

[Richard Henderson]

On 5/29/20 5:04 AM, Szabolcs Nagy wrote:
> again i'm using the branch at
> 
> https://github.com/rth7680/qemu/tree/tgt-arm-mte
> 
> to test armv8.5-a mte, now qemu-system-aarch64 segfaults
> and it's easy to reproduce: minimal source and static
> linked binary is attached (should be executed on linux
> with mte support, i used mte-v4 kernel with reverted
> memory property check).

Now fixed on that branch.


r~

Re: another tst-arm-mte bug: qemu-system segfaults

[Szabolcs Nagy]

The 06/02/2020 14:58, Richard Henderson wrote:
> On 5/29/20 5:04 AM, Szabolcs Nagy wrote:
> > again i'm using the branch at
> > 
> > https://github.com/rth7680/qemu/tree/tgt-arm-mte
> > 
> > to test armv8.5-a mte, now qemu-system-aarch64 segfaults
> > and it's easy to reproduce: minimal source and static
> > linked binary is attached (should be executed on linux
> > with mte support, i used mte-v4 kernel with reverted
> > memory property check).
> 
> Now fixed on that branch.

thanks my tests now get further but later i run into
the previous assert failure:

target/arm/mte_helper.c:97:allocation_tag_mem: assertion failed: (tag_size <= in_page)

i might be able to reduce it to a small reproducer
this time. i assume that will help.

Re: another tst-arm-mte bug: qemu-system segfaults

[Richard Henderson]

On 6/3/20 6:50 AM, Szabolcs Nagy wrote:
> thanks my tests now get further but later i run into
> the previous assert failure:
> 
> target/arm/mte_helper.c:97:allocation_tag_mem: assertion failed: (tag_size <= in_page)
> 
> i might be able to reduce it to a small reproducer
> this time. i assume that will help.

Dang, I had hoped that the one fix would cover both -- it's definitely in the
same area.  Yes, a small reproducer will help, but I will also try again with
your larger reproducer.


r~

Re: another tst-arm-mte bug: qemu-system segfaults

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

The 06/03/2020 09:21, Richard Henderson wrote:
> On 6/3/20 6:50 AM, Szabolcs Nagy wrote:
> > thanks my tests now get further but later i run into
> > the previous assert failure:
> > 
> > target/arm/mte_helper.c:97:allocation_tag_mem: assertion failed: (tag_size <= in_page)
> > 
> > i might be able to reduce it to a small reproducer
> > this time. i assume that will help.
> 
> Dang, I had hoped that the one fix would cover both -- it's definitely in the
> same area.  Yes, a small reproducer will help, but I will also try again with
> your larger reproducer.

reproducer .c and static exe attached.

the referenced __memcmp_aarch64 is again
from the arm optimized-routines repo.

[-- Attachment #2: bug2.c --]
[-- Type: text/x-csrc, Size: 1686 bytes --]

#include <stddef.h>
#include <stdint.h>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <arm_acle.h>

int __memcmp_aarch64 (const void *, const void *, size_t);

#define PR_SET_TAGGED_ADDR_CTRL 55
#define PR_TAGGED_ADDR_ENABLE (1UL << 0)
#define PR_MTE_TCF_SHIFT 1
#define PR_MTE_TCF_SYNC (1UL << PR_MTE_TCF_SHIFT)
#define PR_MTE_TAG_SHIFT 3
#define PROT_MTE 0x20
#define MTE_GRANULE_SIZE 16

void *
alignup_mte (void *p)
{
  return (void *) (((uintptr_t) p + MTE_GRANULE_SIZE - 1)
		   & ~(MTE_GRANULE_SIZE - 1));
}

void *
aligndown_mte (void *p)
{
  return (void *) ((uintptr_t) p & ~(MTE_GRANULE_SIZE - 1));
}

void
tag_buffer_helper (void *p, int len)
{
  char *ptr = p;
  char *end = alignup_mte (ptr + len);
  ptr = aligndown_mte (p);
  for (; ptr < end; ptr += MTE_GRANULE_SIZE)
    {
      __arm_mte_set_tag (ptr);
    }
}

void *
tag_buffer (void *p, int len)
{
  p = __arm_mte_increment_tag (p, 1);
  tag_buffer_helper (p, len);
  return p;
}

int main (void)
{
  int r = prctl (PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | (0xfffe << PR_MTE_TAG_SHIFT), 0, 0, 0);
  if (r < 0) return -1;
  char *src1 = mmap (NULL, 4096, PROT_READ | PROT_WRITE | PROT_MTE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  char *src2 = mmap (NULL, 4096, PROT_READ | PROT_WRITE | PROT_MTE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  if (src1 == MAP_FAILED) return -1;
  if (src2 == MAP_FAILED) return -1;
  char *s1 = src1;
  char *s2 = src2 + 15;
  for (int i = 0; i < 250; i++)
    src1[i] = src2[i] = '?';
  for (int i = 0; i < 200; i++)
    s1[i] = s2[i] = 'a' + i % 23;
  s1 = tag_buffer (s1, 200);
  s2 = tag_buffer (s2, 200);
  __memcmp_aarch64(s1, s2, 200);
  return 0;
}

[-- Attachment #3: bug2 --]
[-- Type: application/octet-stream, Size: 22240 bytes --]

Re: another tst-arm-mte bug: qemu-system segfaults

[Richard Henderson]

On 6/3/20 10:17 AM, Szabolcs Nagy wrote:
> The 06/03/2020 09:21, Richard Henderson wrote:
>> On 6/3/20 6:50 AM, Szabolcs Nagy wrote:
>>> thanks my tests now get further but later i run into
>>> the previous assert failure:
>>>
>>> target/arm/mte_helper.c:97:allocation_tag_mem: assertion failed: (tag_size <= in_page)
>>>
>>> i might be able to reduce it to a small reproducer
>>> this time. i assume that will help.
>>
>> Dang, I had hoped that the one fix would cover both -- it's definitely in the
>> same area.  Yes, a small reproducer will help, but I will also try again with
>> your larger reproducer.
> 
> reproducer .c and static exe attached.
> 
> the referenced __memcmp_aarch64 is again
> from the arm optimized-routines repo.

That assert is just wrong -- it's attempting to sanity check a virtual address
against a property associated with the physical address, and even doing that
incorrectly.

I've pushed a fixup to the branch to remove it, and I'll look into adding a
correct assertion later.


r~

Re: another tst-arm-mte bug: qemu-system segfaults

[Szabolcs Nagy]

The 06/05/2020 13:26, Richard Henderson wrote:
> That assert is just wrong -- it's attempting to sanity check a virtual address
> against a property associated with the physical address, and even doing that
> incorrectly.
> 
> I've pushed a fixup to the branch to remove it, and I'll look into adding a
> correct assertion later.

thanks, my tests pass with the change.