* [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1
@ 2021-03-30 19:34 Andrey Konovalov
2021-03-30 23:22 ` [Bug 1921948] " Richard Henderson
` (17 more replies)
0 siblings, 18 replies; 63+ messages in thread
From: Andrey Konovalov @ 2021-03-30 19:34 UTC (permalink / raw)
To: qemu-devel
Public bug reported:
For kernel memory accesses that span across two memory granules, QEMU's
MTE implementation only checks the tag of the first granule but not of
the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic -nographic
-kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
** Affects: qemu
Importance: Undecided
Status: New
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
New
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
@ 2021-03-30 23:22 ` Richard Henderson
2021-03-30 23:32 ` Peter Collingbourne
` (16 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-03-30 23:22 UTC (permalink / raw)
To: qemu-devel
I believe that you're correct, and that I mis-read the MTE
specification.
I believed that exactly one mte tag check was made for any single memory
access. But I missed that unaligned accesses are as-if a sequence of byte
accesses -- in the Arm ARM, see aarch64/functions/memory/Mem[].
I'm still trying to verify this via the Arm FVP, but so far I've not
found the right incantation of parameters to properly enable MTE.
(I can enable the instructions, but a simple stg/ldg test suggests
that there is no tag storage enabled -- all tags read as 0.)
** Changed in: qemu
Assignee: (unassigned) => Richard Henderson (rth)
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
New
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
2021-03-30 23:22 ` [Bug 1921948] " Richard Henderson
@ 2021-03-30 23:32 ` Peter Collingbourne
2021-03-31 6:44 ` Richard Henderson
` (15 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Peter Collingbourne @ 2021-03-30 23:32 UTC (permalink / raw)
To: qemu-devel
The flags that you need to pass to FVP to enable MTE are listed near the
end of the README here:
https://cs.android.com/android/platform/superproject/+/master:device/generic/goldfish/fvpbase/README.md
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
New
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
2021-03-30 23:22 ` [Bug 1921948] " Richard Henderson
2021-03-30 23:32 ` Peter Collingbourne
@ 2021-03-31 6:44 ` Richard Henderson
2021-04-02 15:41 ` Richard Henderson
` (14 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-03-31 6:44 UTC (permalink / raw)
To: qemu-devel
Ah, perfect, I was missing dram_metadata.is_enabled.
And my userland unaligned test case demonstrates that
the second granule is tested, as reported.
** Changed in: qemu
Status: New => Confirmed
** Changed in: qemu
Status: Confirmed => In Progress
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (2 preceding siblings ...)
2021-03-31 6:44 ` Richard Henderson
@ 2021-04-02 15:41 ` Richard Henderson
2021-04-02 16:17 ` Andrey Konovalov
` (13 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-02 15:41 UTC (permalink / raw)
To: qemu-devel
https://patchew.org/QEMU/20210402053728.265173-1-richard.henderson@linaro.org/
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (3 preceding siblings ...)
2021-04-02 15:41 ` Richard Henderson
@ 2021-04-02 16:17 ` Andrey Konovalov
2021-04-02 16:31 ` Richard Henderson
` (12 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Andrey Konovalov @ 2021-04-02 16:17 UTC (permalink / raw)
To: qemu-devel
Hi Richard,
I tried your patch, but QEMU crashes with:
ERROR:../target/arm/mte_helper.c:588:mte_check_fail: code should not be reached
Bail out! ERROR:../target/arm/mte_helper.c:588:mte_check_fail: code should not be reached
when running KASAN tests.
Thanks!
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (4 preceding siblings ...)
2021-04-02 16:17 ` Andrey Konovalov
@ 2021-04-02 16:31 ` Richard Henderson
2021-04-03 14:34 ` Andrey Konovalov
` (11 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-02 16:31 UTC (permalink / raw)
To: qemu-devel
Yeah, I saw an error right after posting. Please try v2:
https://patchew.org/QEMU/20210402161835.286665-1-richard.henderson@linaro.org/
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (5 preceding siblings ...)
2021-04-02 16:31 ` Richard Henderson
@ 2021-04-03 14:34 ` Andrey Konovalov
2021-04-07 20:17 ` Andrey Konovalov
` (10 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Andrey Konovalov @ 2021-04-03 14:34 UTC (permalink / raw)
To: qemu-devel
With v2, a lot of KASAN tests start failing. This likely means that MTE
tag faults stop being generated in certain cases.
With v3 [1], no MTE faults are generated at all.
[1]
https://patchew.org/QEMU/20210402214217.422585-1-richard.henderson@linaro.org/
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v4 00/12] target/arm mte fixes
@ 2021-04-06 17:40 Richard Henderson
2021-04-06 17:40 ` [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions Richard Henderson
` (13 more replies)
0 siblings, 14 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Changes for v4:
* Fix tag count computation error in mte_checkN, which when used
by mte_check1 in patch 5, caused all sorts of KASAN failures.
* Fix PAGE_ANON / PAGE_TARGET_1 overlap.
r~
Richard Henderson (12):
accel/tcg: Preserve PAGE_ANON when changing page permissions
target/arm: Check PAGE_WRITE_ORG for MTE writeability
target/arm: Fix mte_checkN
target/arm: Split out mte_probe_int
target/arm: Fix unaligned checks for mte_check1, mte_probe1
test/tcg/aarch64: Add mte-5
target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
target/arm: Merge mte_check1, mte_checkN
target/arm: Rename mte_probe1 to mte_probe
target/arm: Simplify sve mte checking
target/arm: Remove log2_esize parameter to gen_mte_checkN
exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
include/exec/cpu-all.h | 4 +-
target/arm/helper-a64.h | 3 +-
target/arm/internals.h | 11 +-
target/arm/translate-a64.h | 2 +-
tests/tcg/aarch64/mte.h | 3 +-
accel/tcg/translate-all.c | 9 +-
target/arm/mte_helper.c | 185 ++++++++++++------------------
target/arm/sve_helper.c | 100 ++++++----------
target/arm/translate-a64.c | 22 ++--
target/arm/translate-sve.c | 9 +-
tests/tcg/aarch64/mte-5.c | 44 +++++++
tests/tcg/aarch64/mte-6.c | 43 +++++++
tests/tcg/aarch64/Makefile.target | 2 +-
13 files changed, 227 insertions(+), 210 deletions(-)
create mode 100644 tests/tcg/aarch64/mte-5.c
create mode 100644 tests/tcg/aarch64/mte-6.c
--
2.25.1
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-07 13:55 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 02/12] target/arm: Check PAGE_WRITE_ORG for MTE writeability Richard Henderson
` (12 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Stephen Long
Using mprotect() to change PROT_* does not change the MAP_ANON
previously set with mmap(). Our linux-user version of MTE only
works with MAP_ANON pages, so losing PAGE_ANON caused MTE to
stop working.
Reported-by: Stephen Long <steplong@quicinc.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/tcg/aarch64/mte.h | 3 ++-
accel/tcg/translate-all.c | 9 +++++--
tests/tcg/aarch64/mte-6.c | 43 +++++++++++++++++++++++++++++++
tests/tcg/aarch64/Makefile.target | 2 +-
4 files changed, 53 insertions(+), 4 deletions(-)
create mode 100644 tests/tcg/aarch64/mte-6.c
diff --git a/tests/tcg/aarch64/mte.h b/tests/tcg/aarch64/mte.h
index 141cef522c..0805676b11 100644
--- a/tests/tcg/aarch64/mte.h
+++ b/tests/tcg/aarch64/mte.h
@@ -48,7 +48,8 @@ static void enable_mte(int tcf)
}
}
-static void *alloc_mte_mem(size_t size)
+static void * alloc_mte_mem(size_t size) __attribute__((unused));
+static void * alloc_mte_mem(size_t size)
{
void *p = mmap(NULL, size, PROT_READ | PROT_WRITE | PROT_MTE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index f32df8b240..ba6ab09790 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2714,6 +2714,8 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
a missing call to h2g_valid. */
assert(end - 1 <= GUEST_ADDR_MAX);
assert(start < end);
+ /* Only set PAGE_ANON with new mappings. */
+ assert(!(flags & PAGE_ANON) || (flags & PAGE_RESET));
assert_memory_lock();
start = start & TARGET_PAGE_MASK;
@@ -2737,11 +2739,14 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
p->first_tb) {
tb_invalidate_phys_page(addr, 0);
}
- if (reset_target_data && p->target_data) {
+ if (reset_target_data) {
g_free(p->target_data);
p->target_data = NULL;
+ p->flags = flags;
+ } else {
+ /* Using mprotect on a page does not change MAP_ANON. */
+ p->flags = (p->flags & PAGE_ANON) | flags;
}
- p->flags = flags;
}
}
diff --git a/tests/tcg/aarch64/mte-6.c b/tests/tcg/aarch64/mte-6.c
new file mode 100644
index 0000000000..60d51d18be
--- /dev/null
+++ b/tests/tcg/aarch64/mte-6.c
@@ -0,0 +1,43 @@
+#include "mte.h"
+
+void pass(int sig, siginfo_t *info, void *uc)
+{
+ assert(info->si_code == SEGV_MTESERR);
+ exit(0);
+}
+
+int main(void)
+{
+ enable_mte(PR_MTE_TCF_SYNC);
+
+ void *brk = sbrk(16);
+ if (brk == (void *)-1) {
+ perror("sbrk");
+ return 2;
+ }
+
+ if (mprotect(brk, 16, PROT_READ | PROT_WRITE | PROT_MTE)) {
+ perror("mprotect");
+ return 2;
+ }
+
+ int *p1, *p2;
+ long excl = 1;
+
+ asm("irg %0,%1,%2" : "=r"(p1) : "r"(brk), "r"(excl));
+ asm("gmi %0,%1,%0" : "+r"(excl) : "r"(p1));
+ asm("irg %0,%1,%2" : "=r"(p2) : "r"(brk), "r"(excl));
+ asm("stg %0,[%0]" : : "r"(p1));
+
+ *p1 = 0;
+
+ struct sigaction sa;
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_sigaction = pass;
+ sa.sa_flags = SA_SIGINFO;
+ sigaction(SIGSEGV, &sa, NULL);
+
+ *p2 = 0;
+
+ abort();
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 56e48f4b34..05b2622bfc 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -37,7 +37,7 @@ AARCH64_TESTS += bti-2
# MTE Tests
ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_MTE),)
-AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4
+AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-6
mte-%: CFLAGS += -march=armv8.5-a+memtag
endif
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 02/12] target/arm: Check PAGE_WRITE_ORG for MTE writeability
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
2021-04-06 17:40 ` [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-07 15:34 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 03/12] target/arm: Fix mte_checkN Richard Henderson
` (11 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We can remove PAGE_WRITE when (internally) marking a page
read-only because it contains translated code.
This can be triggered by tests/tcg/aarch64/bti-2, after
having serviced SIGILL trampolines on the stack.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/mte_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 0bbb9ec346..8be17e1b70 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -83,7 +83,7 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
uint8_t *tags;
uintptr_t index;
- if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE : PAGE_READ))) {
+ if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE_ORG : PAGE_READ))) {
/* SIGSEGV */
arm_cpu_tlb_fill(env_cpu(env), ptr, ptr_size, ptr_access,
ptr_mmu_idx, false, ra);
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
2021-04-06 17:40 ` [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions Richard Henderson
2021-04-06 17:40 ` [PATCH v4 02/12] target/arm: Check PAGE_WRITE_ORG for MTE writeability Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-07 18:39 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 04/12] target/arm: Split out mte_probe_int Richard Henderson
` (10 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We were incorrectly assuming that only the first byte of an MTE access
is checked against the tags. But per the ARM, unaligned accesses are
pre-decomposed into single-byte accesses. So by the time we reach the
actual MTE check in the ARM pseudocode, all accesses are aligned.
Therefore, the first failure is always either the first byte of the
access, or the first byte of the granule.
In addition, some of the arithmetic is off for last-first -> count.
This does not become directly visible until a later patch that passes
single bytes into this function, so ptr == ptr_last.
Buglink: https://bugs.launchpad.net/bugs/1921948
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/mte_helper.c | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 8be17e1b70..c87717127c 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -757,10 +757,10 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
uint64_t ptr, uintptr_t ra)
{
int mmu_idx, ptr_tag, bit55;
- uint64_t ptr_last, ptr_end, prev_page, next_page;
- uint64_t tag_first, tag_end;
- uint64_t tag_byte_first, tag_byte_end;
- uint32_t esize, total, tag_count, tag_size, n, c;
+ uint64_t ptr_last, prev_page, next_page;
+ uint64_t tag_first, tag_last;
+ uint64_t tag_byte_first, tag_byte_last;
+ uint32_t total, tag_count, tag_size, n, c;
uint8_t *mem1, *mem2;
MMUAccessType type;
@@ -779,29 +779,27 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
- esize = FIELD_EX32(desc, MTEDESC, ESIZE);
total = FIELD_EX32(desc, MTEDESC, TSIZE);
/* Find the addr of the end of the access, and of the last element. */
- ptr_end = ptr + total;
- ptr_last = ptr_end - esize;
+ ptr_last = ptr + total - 1;
/* Round the bounds to the tag granule, and compute the number of tags. */
tag_first = QEMU_ALIGN_DOWN(ptr, TAG_GRANULE);
- tag_end = QEMU_ALIGN_UP(ptr_last, TAG_GRANULE);
- tag_count = (tag_end - tag_first) / TAG_GRANULE;
+ tag_last = QEMU_ALIGN_DOWN(ptr_last, TAG_GRANULE);
+ tag_count = ((tag_last - tag_first) / TAG_GRANULE) + 1;
/* Round the bounds to twice the tag granule, and compute the bytes. */
tag_byte_first = QEMU_ALIGN_DOWN(ptr, 2 * TAG_GRANULE);
- tag_byte_end = QEMU_ALIGN_UP(ptr_last, 2 * TAG_GRANULE);
+ tag_byte_last = QEMU_ALIGN_DOWN(ptr_last, 2 * TAG_GRANULE);
/* Locate the page boundaries. */
prev_page = ptr & TARGET_PAGE_MASK;
next_page = prev_page + TARGET_PAGE_SIZE;
- if (likely(tag_end - prev_page <= TARGET_PAGE_SIZE)) {
+ if (likely(tag_last - prev_page <= TARGET_PAGE_SIZE)) {
/* Memory access stays on one page. */
- tag_size = (tag_byte_end - tag_byte_first) / (2 * TAG_GRANULE);
+ tag_size = ((tag_byte_last - tag_byte_first) / (2 * TAG_GRANULE)) + 1;
mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, total,
MMU_DATA_LOAD, tag_size, ra);
if (!mem1) {
@@ -815,9 +813,9 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, next_page - ptr,
MMU_DATA_LOAD, tag_size, ra);
- tag_size = (tag_byte_end - next_page) / (2 * TAG_GRANULE);
+ tag_size = ((tag_byte_last - next_page) / (2 * TAG_GRANULE)) + 1;
mem2 = allocation_tag_mem(env, mmu_idx, next_page, type,
- ptr_end - next_page,
+ ptr_last - next_page + 1,
MMU_DATA_LOAD, tag_size, ra);
/*
@@ -838,15 +836,13 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
}
/*
- * If we failed, we know which granule. Compute the element that
- * is first in that granule, and signal failure on that element.
+ * If we failed, we know which granule. For the first granule, the
+ * failure address is @ptr, the first byte accessed. Otherwise the
+ * failure address is the first byte of the nth granule.
*/
if (unlikely(n < tag_count)) {
- uint64_t fail_ofs;
-
- fail_ofs = tag_first + n * TAG_GRANULE - ptr;
- fail_ofs = ROUND_UP(fail_ofs, esize);
- mte_check_fail(env, desc, ptr + fail_ofs, ra);
+ uint64_t fault = (n == 0 ? ptr : tag_first + n * TAG_GRANULE);
+ mte_check_fail(env, desc, fault, ra);
}
done:
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 04/12] target/arm: Split out mte_probe_int
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (2 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 03/12] target/arm: Fix mte_checkN Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 9:01 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1 Richard Henderson
` (9 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Split out a helper function from mte_checkN to perform
all of the checking and address manpulation. So far,
just use this in mte_checkN itself.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/mte_helper.c | 52 +++++++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 12 deletions(-)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index c87717127c..144bfa4a51 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -753,33 +753,45 @@ static int checkN(uint8_t *mem, int odd, int cmp, int count)
return n;
}
-uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
- uint64_t ptr, uintptr_t ra)
+/*
+ * mte_probe_int:
+ * @env: CPU environment
+ * @desc: MTEDESC descriptor
+ * @ptr: virtual address of the base of the access
+ * @fault: return virtual address of the first check failure
+ *
+ * Internal routine for both mte_probe and mte_check.
+ * Return zero on failure, filling in *fault.
+ * Return negative on trivial success for tbi disabled.
+ * Return positive on success with tbi enabled.
+ */
+static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
+ uintptr_t ra, uint32_t total, uint64_t *fault)
{
int mmu_idx, ptr_tag, bit55;
uint64_t ptr_last, prev_page, next_page;
uint64_t tag_first, tag_last;
uint64_t tag_byte_first, tag_byte_last;
- uint32_t total, tag_count, tag_size, n, c;
+ uint32_t tag_count, tag_size, n, c;
uint8_t *mem1, *mem2;
MMUAccessType type;
bit55 = extract64(ptr, 55, 1);
+ *fault = ptr;
/* If TBI is disabled, the access is unchecked, and ptr is not dirty. */
if (unlikely(!tbi_check(desc, bit55))) {
- return ptr;
+ return -1;
}
ptr_tag = allocation_tag_from_addr(ptr);
if (tcma_check(desc, bit55, ptr_tag)) {
- goto done;
+ return 1;
}
mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
- total = FIELD_EX32(desc, MTEDESC, TSIZE);
/* Find the addr of the end of the access, and of the last element. */
ptr_last = ptr + total - 1;
@@ -803,7 +815,7 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, total,
MMU_DATA_LOAD, tag_size, ra);
if (!mem1) {
- goto done;
+ return 1;
}
/* Perform all of the comparisons. */
n = checkN(mem1, ptr & TAG_GRANULE, ptr_tag, tag_count);
@@ -829,23 +841,39 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
}
if (n == c) {
if (!mem2) {
- goto done;
+ return 1;
}
n += checkN(mem2, 0, ptr_tag, tag_count - c);
}
}
+ if (likely(n == tag_count)) {
+ return 1;
+ }
+
/*
* If we failed, we know which granule. For the first granule, the
* failure address is @ptr, the first byte accessed. Otherwise the
* failure address is the first byte of the nth granule.
*/
- if (unlikely(n < tag_count)) {
- uint64_t fault = (n == 0 ? ptr : tag_first + n * TAG_GRANULE);
- mte_check_fail(env, desc, fault, ra);
+ if (n > 0) {
+ *fault = tag_first + n * TAG_GRANULE;
}
+ return 0;
+}
- done:
+uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
+ uint64_t ptr, uintptr_t ra)
+{
+ uint64_t fault;
+ uint32_t total = FIELD_EX32(desc, MTEDESC, TSIZE);
+ int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
+
+ if (unlikely(ret == 0)) {
+ mte_check_fail(env, desc, fault, ra);
+ } else if (ret < 0) {
+ return ptr;
+ }
return useronly_clean_ptr(ptr);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (3 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 04/12] target/arm: Split out mte_probe_int Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 9:05 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 06/12] test/tcg/aarch64: Add mte-5 Richard Henderson
` (8 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We were incorrectly assuming that only the first byte of an MTE access
is checked against the tags. But per the ARM, unaligned accesses are
pre-decomposed into single-byte accesses. So by the time we reach the
actual MTE check in the ARM pseudocode, all accesses are aligned.
We cannot tell a priori whether or not a given scalar access is aligned,
therefore we must at least check. Use mte_probe_int, which is already
set up for checking multiple granules.
Buglink: https://bugs.launchpad.net/bugs/1921948
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/mte_helper.c | 109 +++++++++++++---------------------------
1 file changed, 35 insertions(+), 74 deletions(-)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 144bfa4a51..619c4b9351 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -617,80 +617,6 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
}
}
-/*
- * Perform an MTE checked access for a single logical or atomic access.
- */
-static bool mte_probe1_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
- uintptr_t ra, int bit55)
-{
- int mem_tag, mmu_idx, ptr_tag, size;
- MMUAccessType type;
- uint8_t *mem;
-
- ptr_tag = allocation_tag_from_addr(ptr);
-
- if (tcma_check(desc, bit55, ptr_tag)) {
- return true;
- }
-
- mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
- type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
- size = FIELD_EX32(desc, MTEDESC, ESIZE);
-
- mem = allocation_tag_mem(env, mmu_idx, ptr, type, size,
- MMU_DATA_LOAD, 1, ra);
- if (!mem) {
- return true;
- }
-
- mem_tag = load_tag1(ptr, mem);
- return ptr_tag == mem_tag;
-}
-
-/*
- * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
- * Returns false if the access is Checked and the check failed. This
- * is only intended to probe the tag -- the validity of the page must
- * be checked beforehand.
- */
-bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
-{
- int bit55 = extract64(ptr, 55, 1);
-
- /* If TBI is disabled, the access is unchecked. */
- if (unlikely(!tbi_check(desc, bit55))) {
- return true;
- }
-
- return mte_probe1_int(env, desc, ptr, 0, bit55);
-}
-
-uint64_t mte_check1(CPUARMState *env, uint32_t desc,
- uint64_t ptr, uintptr_t ra)
-{
- int bit55 = extract64(ptr, 55, 1);
-
- /* If TBI is disabled, the access is unchecked, and ptr is not dirty. */
- if (unlikely(!tbi_check(desc, bit55))) {
- return ptr;
- }
-
- if (unlikely(!mte_probe1_int(env, desc, ptr, ra, bit55))) {
- mte_check_fail(env, desc, ptr, ra);
- }
-
- return useronly_clean_ptr(ptr);
-}
-
-uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
-{
- return mte_check1(env, desc, ptr, GETPC());
-}
-
-/*
- * Perform an MTE checked access for multiple logical accesses.
- */
-
/**
* checkN:
* @tag: tag memory to test
@@ -882,6 +808,41 @@ uint64_t HELPER(mte_checkN)(CPUARMState *env, uint32_t desc, uint64_t ptr)
return mte_checkN(env, desc, ptr, GETPC());
}
+uint64_t mte_check1(CPUARMState *env, uint32_t desc,
+ uint64_t ptr, uintptr_t ra)
+{
+ uint64_t fault;
+ uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
+ int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
+
+ if (unlikely(ret == 0)) {
+ mte_check_fail(env, desc, fault, ra);
+ } else if (ret < 0) {
+ return ptr;
+ }
+ return useronly_clean_ptr(ptr);
+}
+
+uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
+{
+ return mte_check1(env, desc, ptr, GETPC());
+}
+
+/*
+ * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
+ * Returns false if the access is Checked and the check failed. This
+ * is only intended to probe the tag -- the validity of the page must
+ * be checked beforehand.
+ */
+bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
+{
+ uint64_t fault;
+ uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
+ int ret = mte_probe_int(env, desc, ptr, 0, total, &fault);
+
+ return ret != 0;
+}
+
/*
* Perform an MTE checked access for DC_ZVA.
*/
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 06/12] test/tcg/aarch64: Add mte-5
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (4 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1 Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 9:07 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 07/12] target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1 Richard Henderson
` (7 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Buglink: https://bugs.launchpad.net/bugs/1921948
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/tcg/aarch64/mte-5.c | 44 +++++++++++++++++++++++++++++++
tests/tcg/aarch64/Makefile.target | 2 +-
2 files changed, 45 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/aarch64/mte-5.c
diff --git a/tests/tcg/aarch64/mte-5.c b/tests/tcg/aarch64/mte-5.c
new file mode 100644
index 0000000000..6dbd6ab3ea
--- /dev/null
+++ b/tests/tcg/aarch64/mte-5.c
@@ -0,0 +1,44 @@
+/*
+ * Memory tagging, faulting unaligned access.
+ *
+ * Copyright (c) 2021 Linaro Ltd
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "mte.h"
+
+void pass(int sig, siginfo_t *info, void *uc)
+{
+ assert(info->si_code == SEGV_MTESERR);
+ exit(0);
+}
+
+int main(int ac, char **av)
+{
+ struct sigaction sa;
+ void *p0, *p1, *p2;
+ long excl = 1;
+
+ enable_mte(PR_MTE_TCF_SYNC);
+ p0 = alloc_mte_mem(sizeof(*p0));
+
+ /* Create two differently tagged pointers. */
+ asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(excl));
+ asm("gmi %0,%1,%0" : "+r"(excl) : "r" (p1));
+ assert(excl != 1);
+ asm("irg %0,%1,%2" : "=r"(p2) : "r"(p0), "r"(excl));
+ assert(p1 != p2);
+
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_sigaction = pass;
+ sa.sa_flags = SA_SIGINFO;
+ sigaction(SIGSEGV, &sa, NULL);
+
+ /* Store store two different tags in sequential granules. */
+ asm("stg %0, [%0]" : : "r"(p1));
+ asm("stg %0, [%0]" : : "r"(p2 + 16));
+
+ /* Perform an unaligned load crossing the granules. */
+ asm volatile("ldr %0, [%1]" : "=r"(p0) : "r"(p1 + 12));
+ abort();
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 05b2622bfc..928357b10a 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -37,7 +37,7 @@ AARCH64_TESTS += bti-2
# MTE Tests
ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_MTE),)
-AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-6
+AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-5 mte-6
mte-%: CFLAGS += -march=armv8.5-a+memtag
endif
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 07/12] target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (5 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 06/12] test/tcg/aarch64: Add mte-5 Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 11:08 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 08/12] target/arm: Merge mte_check1, mte_checkN Richard Henderson
` (6 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
After recent changes, mte_checkN does not use ESIZE,
and mte_check1 never used TSIZE. We can combine the
two into a single field: SIZEM1.
Choose to pass size - 1 because size == 0 is never used,
our immediate need in mte_probe_int is for the address
of the last byte (ptr + size - 1), and since almost all
operations are powers of 2, this makes the immediate
constant one bit smaller.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 4 ++--
target/arm/mte_helper.c | 18 ++++++++----------
target/arm/translate-a64.c | 5 ++---
target/arm/translate-sve.c | 5 ++---
4 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index f11bd32696..2c77f2d50f 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -26,6 +26,7 @@
#define TARGET_ARM_INTERNALS_H
#include "hw/registerfields.h"
+#include "tcg/tcg-gvec-desc.h"
#include "syndrome.h"
/* register banks for CPU modes */
@@ -1142,8 +1143,7 @@ FIELD(MTEDESC, MIDX, 0, 4)
FIELD(MTEDESC, TBI, 4, 2)
FIELD(MTEDESC, TCMA, 6, 2)
FIELD(MTEDESC, WRITE, 8, 1)
-FIELD(MTEDESC, ESIZE, 9, 5)
-FIELD(MTEDESC, TSIZE, 14, 10) /* mte_checkN only */
+FIELD(MTEDESC, SIZEM1, 9, SIMD_DATA_BITS - 9) /* size - 1 */
bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr);
uint64_t mte_check1(CPUARMState *env, uint32_t desc,
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 619c4b9351..9dfbb14358 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -692,13 +692,13 @@ static int checkN(uint8_t *mem, int odd, int cmp, int count)
* Return positive on success with tbi enabled.
*/
static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
- uintptr_t ra, uint32_t total, uint64_t *fault)
+ uintptr_t ra, uint64_t *fault)
{
int mmu_idx, ptr_tag, bit55;
uint64_t ptr_last, prev_page, next_page;
uint64_t tag_first, tag_last;
uint64_t tag_byte_first, tag_byte_last;
- uint32_t tag_count, tag_size, n, c;
+ uint32_t sizem1, tag_count, tag_size, n, c;
uint8_t *mem1, *mem2;
MMUAccessType type;
@@ -718,9 +718,10 @@ static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
+ sizem1 = FIELD_EX32(desc, MTEDESC, SIZEM1);
/* Find the addr of the end of the access, and of the last element. */
- ptr_last = ptr + total - 1;
+ ptr_last = ptr + sizem1;
/* Round the bounds to the tag granule, and compute the number of tags. */
tag_first = QEMU_ALIGN_DOWN(ptr, TAG_GRANULE);
@@ -738,7 +739,7 @@ static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
if (likely(tag_last - prev_page <= TARGET_PAGE_SIZE)) {
/* Memory access stays on one page. */
tag_size = ((tag_byte_last - tag_byte_first) / (2 * TAG_GRANULE)) + 1;
- mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, total,
+ mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, sizem1 + 1,
MMU_DATA_LOAD, tag_size, ra);
if (!mem1) {
return 1;
@@ -792,8 +793,7 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
uint64_t ptr, uintptr_t ra)
{
uint64_t fault;
- uint32_t total = FIELD_EX32(desc, MTEDESC, TSIZE);
- int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
+ int ret = mte_probe_int(env, desc, ptr, ra, &fault);
if (unlikely(ret == 0)) {
mte_check_fail(env, desc, fault, ra);
@@ -812,8 +812,7 @@ uint64_t mte_check1(CPUARMState *env, uint32_t desc,
uint64_t ptr, uintptr_t ra)
{
uint64_t fault;
- uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
- int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
+ int ret = mte_probe_int(env, desc, ptr, ra, &fault);
if (unlikely(ret == 0)) {
mte_check_fail(env, desc, fault, ra);
@@ -837,8 +836,7 @@ uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
{
uint64_t fault;
- uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
- int ret = mte_probe_int(env, desc, ptr, 0, total, &fault);
+ int ret = mte_probe_int(env, desc, ptr, 0, &fault);
return ret != 0;
}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 0b42e53500..3af00ae90e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -272,7 +272,7 @@ static TCGv_i64 gen_mte_check1_mmuidx(DisasContext *s, TCGv_i64 addr,
desc = FIELD_DP32(desc, MTEDESC, TBI, s->tbid);
desc = FIELD_DP32(desc, MTEDESC, TCMA, s->tcma);
desc = FIELD_DP32(desc, MTEDESC, WRITE, is_write);
- desc = FIELD_DP32(desc, MTEDESC, ESIZE, 1 << log2_size);
+ desc = FIELD_DP32(desc, MTEDESC, SIZEM1, (1 << log2_size) - 1);
tcg_desc = tcg_const_i32(desc);
ret = new_tmp_a64(s);
@@ -306,8 +306,7 @@ TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
desc = FIELD_DP32(desc, MTEDESC, TBI, s->tbid);
desc = FIELD_DP32(desc, MTEDESC, TCMA, s->tcma);
desc = FIELD_DP32(desc, MTEDESC, WRITE, is_write);
- desc = FIELD_DP32(desc, MTEDESC, ESIZE, 1 << log2_esize);
- desc = FIELD_DP32(desc, MTEDESC, TSIZE, total_size);
+ desc = FIELD_DP32(desc, MTEDESC, SIZEM1, total_size - 1);
tcg_desc = tcg_const_i32(desc);
ret = new_tmp_a64(s);
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 0eefb61214..5179c1f836 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -4509,8 +4509,7 @@ static void do_mem_zpa(DisasContext *s, int zt, int pg, TCGv_i64 addr,
desc = FIELD_DP32(desc, MTEDESC, TBI, s->tbid);
desc = FIELD_DP32(desc, MTEDESC, TCMA, s->tcma);
desc = FIELD_DP32(desc, MTEDESC, WRITE, is_write);
- desc = FIELD_DP32(desc, MTEDESC, ESIZE, 1 << msz);
- desc = FIELD_DP32(desc, MTEDESC, TSIZE, mte_n << msz);
+ desc = FIELD_DP32(desc, MTEDESC, SIZEM1, (mte_n << msz) - 1);
desc <<= SVE_MTEDESC_SHIFT;
} else {
addr = clean_data_tbi(s, addr);
@@ -5189,7 +5188,7 @@ static void do_mem_zpz(DisasContext *s, int zt, int pg, int zm,
desc = FIELD_DP32(desc, MTEDESC, TBI, s->tbid);
desc = FIELD_DP32(desc, MTEDESC, TCMA, s->tcma);
desc = FIELD_DP32(desc, MTEDESC, WRITE, is_write);
- desc = FIELD_DP32(desc, MTEDESC, ESIZE, 1 << msz);
+ desc = FIELD_DP32(desc, MTEDESC, SIZEM1, (1 << msz) - 1);
desc <<= SVE_MTEDESC_SHIFT;
}
desc = simd_desc(vsz, vsz, desc | scale);
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 08/12] target/arm: Merge mte_check1, mte_checkN
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (6 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 07/12] target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1 Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 11:10 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 09/12] target/arm: Rename mte_probe1 to mte_probe Richard Henderson
` (5 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
The mte_check1 and mte_checkN functions are now identical.
Drop mte_check1 and rename mte_checkN to mte_check.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper-a64.h | 3 +--
target/arm/internals.h | 5 +----
target/arm/mte_helper.c | 26 +++-----------------------
target/arm/sve_helper.c | 14 +++++++-------
target/arm/translate-a64.c | 4 ++--
5 files changed, 14 insertions(+), 38 deletions(-)
diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index c139fa81f9..7b706571bb 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -104,8 +104,7 @@ DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
-DEF_HELPER_FLAGS_3(mte_check1, TCG_CALL_NO_WG, i64, env, i32, i64)
-DEF_HELPER_FLAGS_3(mte_checkN, TCG_CALL_NO_WG, i64, env, i32, i64)
+DEF_HELPER_FLAGS_3(mte_check, TCG_CALL_NO_WG, i64, env, i32, i64)
DEF_HELPER_FLAGS_3(mte_check_zva, TCG_CALL_NO_WG, i64, env, i32, i64)
DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
DEF_HELPER_FLAGS_4(addsubg, TCG_CALL_NO_RWG_SE, i64, env, i64, s32, i32)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 2c77f2d50f..af1db2cd9c 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1146,10 +1146,7 @@ FIELD(MTEDESC, WRITE, 8, 1)
FIELD(MTEDESC, SIZEM1, 9, SIMD_DATA_BITS - 9) /* size - 1 */
bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr);
-uint64_t mte_check1(CPUARMState *env, uint32_t desc,
- uint64_t ptr, uintptr_t ra);
-uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
- uint64_t ptr, uintptr_t ra);
+uint64_t mte_check(CPUARMState *env, uint32_t desc, uint64_t ptr, uintptr_t ra);
static inline int allocation_tag_from_addr(uint64_t ptr)
{
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 9dfbb14358..04479f33a1 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -789,8 +789,7 @@ static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
return 0;
}
-uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
- uint64_t ptr, uintptr_t ra)
+uint64_t mte_check(CPUARMState *env, uint32_t desc, uint64_t ptr, uintptr_t ra)
{
uint64_t fault;
int ret = mte_probe_int(env, desc, ptr, ra, &fault);
@@ -803,28 +802,9 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
return useronly_clean_ptr(ptr);
}
-uint64_t HELPER(mte_checkN)(CPUARMState *env, uint32_t desc, uint64_t ptr)
+uint64_t HELPER(mte_check)(CPUARMState *env, uint32_t desc, uint64_t ptr)
{
- return mte_checkN(env, desc, ptr, GETPC());
-}
-
-uint64_t mte_check1(CPUARMState *env, uint32_t desc,
- uint64_t ptr, uintptr_t ra)
-{
- uint64_t fault;
- int ret = mte_probe_int(env, desc, ptr, ra, &fault);
-
- if (unlikely(ret == 0)) {
- mte_check_fail(env, desc, fault, ra);
- } else if (ret < 0) {
- return ptr;
- }
- return useronly_clean_ptr(ptr);
-}
-
-uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
-{
- return mte_check1(env, desc, ptr, GETPC());
+ return mte_check(env, desc, ptr, GETPC());
}
/*
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index fd6c58f96a..b63ddfc7f9 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4442,7 +4442,7 @@ static void sve_cont_ldst_mte_check1(SVEContLdSt *info, CPUARMState *env,
uintptr_t ra)
{
sve_cont_ldst_mte_check_int(info, env, vg, addr, esize, msize,
- mtedesc, ra, mte_check1);
+ mtedesc, ra, mte_check);
}
static void sve_cont_ldst_mte_checkN(SVEContLdSt *info, CPUARMState *env,
@@ -4451,7 +4451,7 @@ static void sve_cont_ldst_mte_checkN(SVEContLdSt *info, CPUARMState *env,
uintptr_t ra)
{
sve_cont_ldst_mte_check_int(info, env, vg, addr, esize, msize,
- mtedesc, ra, mte_checkN);
+ mtedesc, ra, mte_check);
}
@@ -4826,7 +4826,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (fault == FAULT_FIRST) {
/* Trapping mte check for the first-fault element. */
if (mtedesc) {
- mte_check1(env, mtedesc, addr + mem_off, retaddr);
+ mte_check(env, mtedesc, addr + mem_off, retaddr);
}
/*
@@ -5373,7 +5373,7 @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
info.attrs, BP_MEM_READ, retaddr);
}
if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
- mte_check1(env, mtedesc, addr, retaddr);
+ mte_check(env, mtedesc, addr, retaddr);
}
host_fn(&scratch, reg_off, info.host);
} else {
@@ -5386,7 +5386,7 @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
BP_MEM_READ, retaddr);
}
if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
- mte_check1(env, mtedesc, addr, retaddr);
+ mte_check(env, mtedesc, addr, retaddr);
}
tlb_fn(env, &scratch, reg_off, addr, retaddr);
}
@@ -5552,7 +5552,7 @@ void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
*/
addr = base + (off_fn(vm, reg_off) << scale);
if (mtedesc) {
- mte_check1(env, mtedesc, addr, retaddr);
+ mte_check(env, mtedesc, addr, retaddr);
}
tlb_fn(env, vd, reg_off, addr, retaddr);
@@ -5773,7 +5773,7 @@ void sve_st1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
}
if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
- mte_check1(env, mtedesc, addr, retaddr);
+ mte_check(env, mtedesc, addr, retaddr);
}
}
i += 1;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 3af00ae90e..a68d5dd5d1 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -276,7 +276,7 @@ static TCGv_i64 gen_mte_check1_mmuidx(DisasContext *s, TCGv_i64 addr,
tcg_desc = tcg_const_i32(desc);
ret = new_tmp_a64(s);
- gen_helper_mte_check1(ret, cpu_env, tcg_desc, addr);
+ gen_helper_mte_check(ret, cpu_env, tcg_desc, addr);
tcg_temp_free_i32(tcg_desc);
return ret;
@@ -310,7 +310,7 @@ TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
tcg_desc = tcg_const_i32(desc);
ret = new_tmp_a64(s);
- gen_helper_mte_checkN(ret, cpu_env, tcg_desc, addr);
+ gen_helper_mte_check(ret, cpu_env, tcg_desc, addr);
tcg_temp_free_i32(tcg_desc);
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 09/12] target/arm: Rename mte_probe1 to mte_probe
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (7 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 08/12] target/arm: Merge mte_check1, mte_checkN Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 11:10 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 10/12] target/arm: Simplify sve mte checking Richard Henderson
` (4 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
For consistency with the mte_check1 + mte_checkN merge
to mte_check, rename the probe function as well.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 2 +-
target/arm/mte_helper.c | 6 +++---
target/arm/sve_helper.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index af1db2cd9c..886db56b58 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1145,7 +1145,7 @@ FIELD(MTEDESC, TCMA, 6, 2)
FIELD(MTEDESC, WRITE, 8, 1)
FIELD(MTEDESC, SIZEM1, 9, SIMD_DATA_BITS - 9) /* size - 1 */
-bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr);
+bool mte_probe(CPUARMState *env, uint32_t desc, uint64_t ptr);
uint64_t mte_check(CPUARMState *env, uint32_t desc, uint64_t ptr, uintptr_t ra);
static inline int allocation_tag_from_addr(uint64_t ptr)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 04479f33a1..6ac1d21318 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -121,7 +121,7 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
* exception for inaccessible pages, and resolves the virtual address
* into the softmmu tlb.
*
- * When RA == 0, this is for mte_probe1. The page is expected to be
+ * When RA == 0, this is for mte_probe. The page is expected to be
* valid. Indicate to probe_access_flags no-fault, then assert that
* we received a valid page.
*/
@@ -808,12 +808,12 @@ uint64_t HELPER(mte_check)(CPUARMState *env, uint32_t desc, uint64_t ptr)
}
/*
- * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
+ * No-fault version of mte_check, to be used by SVE for MemSingleNF.
* Returns false if the access is Checked and the check failed. This
* is only intended to probe the tag -- the validity of the page must
* be checked beforehand.
*/
-bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
+bool mte_probe(CPUARMState *env, uint32_t desc, uint64_t ptr)
{
uint64_t fault;
int ret = mte_probe_int(env, desc, ptr, 0, &fault);
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index b63ddfc7f9..982240d104 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4869,7 +4869,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
/* Watchpoint hit, see below. */
goto do_fault;
}
- if (mtedesc && !mte_probe1(env, mtedesc, addr + mem_off)) {
+ if (mtedesc && !mte_probe(env, mtedesc, addr + mem_off)) {
goto do_fault;
}
/*
@@ -4919,7 +4919,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
& BP_MEM_READ)) {
goto do_fault;
}
- if (mtedesc && !mte_probe1(env, mtedesc, addr + mem_off)) {
+ if (mtedesc && !mte_probe(env, mtedesc, addr + mem_off)) {
goto do_fault;
}
host_fn(vd, reg_off, host + mem_off);
@@ -5588,7 +5588,7 @@ void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
}
if (mtedesc &&
arm_tlb_mte_tagged(&info.attrs) &&
- !mte_probe1(env, mtedesc, addr)) {
+ !mte_probe(env, mtedesc, addr)) {
goto fault;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 10/12] target/arm: Simplify sve mte checking
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (8 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 09/12] target/arm: Rename mte_probe1 to mte_probe Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 11:23 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 11/12] target/arm: Remove log2_esize parameter to gen_mte_checkN Richard Henderson
` (3 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Now that mte_check1 and mte_checkN have been merged, we can
merge sve_cont_ldst_mte_check1 and sve_cont_ldst_mte_checkN.
Which means that we can eliminate the function pointer into
sve_ldN_r and sve_stN_r, calling sve_cont_ldst_mte_check directly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 84 +++++++++++++----------------------------
1 file changed, 26 insertions(+), 58 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 982240d104..c068dfa0d5 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4382,13 +4382,9 @@ static void sve_cont_ldst_watchpoints(SVEContLdSt *info, CPUARMState *env,
#endif
}
-typedef uint64_t mte_check_fn(CPUARMState *, uint32_t, uint64_t, uintptr_t);
-
-static inline QEMU_ALWAYS_INLINE
-void sve_cont_ldst_mte_check_int(SVEContLdSt *info, CPUARMState *env,
- uint64_t *vg, target_ulong addr, int esize,
- int msize, uint32_t mtedesc, uintptr_t ra,
- mte_check_fn *check)
+static void sve_cont_ldst_mte_check(SVEContLdSt *info, CPUARMState *env,
+ uint64_t *vg, target_ulong addr, int esize,
+ int msize, uint32_t mtedesc, uintptr_t ra)
{
intptr_t mem_off, reg_off, reg_last;
@@ -4405,7 +4401,7 @@ void sve_cont_ldst_mte_check_int(SVEContLdSt *info, CPUARMState *env,
uint64_t pg = vg[reg_off >> 6];
do {
if ((pg >> (reg_off & 63)) & 1) {
- check(env, mtedesc, addr, ra);
+ mte_check(env, mtedesc, addr, ra);
}
reg_off += esize;
mem_off += msize;
@@ -4422,7 +4418,7 @@ void sve_cont_ldst_mte_check_int(SVEContLdSt *info, CPUARMState *env,
uint64_t pg = vg[reg_off >> 6];
do {
if ((pg >> (reg_off & 63)) & 1) {
- check(env, mtedesc, addr, ra);
+ mte_check(env, mtedesc, addr, ra);
}
reg_off += esize;
mem_off += msize;
@@ -4431,30 +4427,6 @@ void sve_cont_ldst_mte_check_int(SVEContLdSt *info, CPUARMState *env,
}
}
-typedef void sve_cont_ldst_mte_check_fn(SVEContLdSt *info, CPUARMState *env,
- uint64_t *vg, target_ulong addr,
- int esize, int msize, uint32_t mtedesc,
- uintptr_t ra);
-
-static void sve_cont_ldst_mte_check1(SVEContLdSt *info, CPUARMState *env,
- uint64_t *vg, target_ulong addr,
- int esize, int msize, uint32_t mtedesc,
- uintptr_t ra)
-{
- sve_cont_ldst_mte_check_int(info, env, vg, addr, esize, msize,
- mtedesc, ra, mte_check);
-}
-
-static void sve_cont_ldst_mte_checkN(SVEContLdSt *info, CPUARMState *env,
- uint64_t *vg, target_ulong addr,
- int esize, int msize, uint32_t mtedesc,
- uintptr_t ra)
-{
- sve_cont_ldst_mte_check_int(info, env, vg, addr, esize, msize,
- mtedesc, ra, mte_check);
-}
-
-
/*
* Common helper for all contiguous 1,2,3,4-register predicated stores.
*/
@@ -4463,8 +4435,7 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
const int esz, const int msz, const int N, uint32_t mtedesc,
sve_ldst1_host_fn *host_fn,
- sve_ldst1_tlb_fn *tlb_fn,
- sve_cont_ldst_mte_check_fn *mte_check_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
const unsigned rd = simd_data(desc);
const intptr_t reg_max = simd_oprsz(desc);
@@ -4493,9 +4464,9 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
* Handle mte checks for all active elements.
* Since TBI must be set for MTE, !mtedesc => !mte_active.
*/
- if (mte_check_fn && mtedesc) {
- mte_check_fn(&info, env, vg, addr, 1 << esz, N << msz,
- mtedesc, retaddr);
+ if (mtedesc) {
+ sve_cont_ldst_mte_check(&info, env, vg, addr, 1 << esz, N << msz,
+ mtedesc, retaddr);
}
flags = info.page[0].flags | info.page[1].flags;
@@ -4621,8 +4592,7 @@ void sve_ldN_r_mte(CPUARMState *env, uint64_t *vg, target_ulong addr,
mtedesc = 0;
}
- sve_ldN_r(env, vg, addr, desc, ra, esz, msz, N, mtedesc, host_fn, tlb_fn,
- N == 1 ? sve_cont_ldst_mte_check1 : sve_cont_ldst_mte_checkN);
+ sve_ldN_r(env, vg, addr, desc, ra, esz, msz, N, mtedesc, host_fn, tlb_fn);
}
#define DO_LD1_1(NAME, ESZ) \
@@ -4630,7 +4600,7 @@ void HELPER(sve_##NAME##_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, MO_8, 1, 0, \
- sve_##NAME##_host, sve_##NAME##_tlb, NULL); \
+ sve_##NAME##_host, sve_##NAME##_tlb); \
} \
void HELPER(sve_##NAME##_r_mte)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
@@ -4644,22 +4614,22 @@ void HELPER(sve_##NAME##_le_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, 1, 0, \
- sve_##NAME##_le_host, sve_##NAME##_le_tlb, NULL); \
+ sve_##NAME##_le_host, sve_##NAME##_le_tlb); \
} \
void HELPER(sve_##NAME##_be_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, 1, 0, \
- sve_##NAME##_be_host, sve_##NAME##_be_tlb, NULL); \
+ sve_##NAME##_be_host, sve_##NAME##_be_tlb); \
} \
void HELPER(sve_##NAME##_le_r_mte)(CPUARMState *env, void *vg, \
- target_ulong addr, uint32_t desc) \
+ target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r_mte(env, vg, addr, desc, GETPC(), ESZ, MSZ, 1, \
sve_##NAME##_le_host, sve_##NAME##_le_tlb); \
} \
void HELPER(sve_##NAME##_be_r_mte)(CPUARMState *env, void *vg, \
- target_ulong addr, uint32_t desc) \
+ target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r_mte(env, vg, addr, desc, GETPC(), ESZ, MSZ, 1, \
sve_##NAME##_be_host, sve_##NAME##_be_tlb); \
@@ -4693,7 +4663,7 @@ void HELPER(sve_ld##N##bb_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r(env, vg, addr, desc, GETPC(), MO_8, MO_8, N, 0, \
- sve_ld1bb_host, sve_ld1bb_tlb, NULL); \
+ sve_ld1bb_host, sve_ld1bb_tlb); \
} \
void HELPER(sve_ld##N##bb_r_mte)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
@@ -4707,13 +4677,13 @@ void HELPER(sve_ld##N##SUFF##_le_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, ESZ, N, 0, \
- sve_ld1##SUFF##_le_host, sve_ld1##SUFF##_le_tlb, NULL); \
+ sve_ld1##SUFF##_le_host, sve_ld1##SUFF##_le_tlb); \
} \
void HELPER(sve_ld##N##SUFF##_be_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, ESZ, N, 0, \
- sve_ld1##SUFF##_be_host, sve_ld1##SUFF##_be_tlb, NULL); \
+ sve_ld1##SUFF##_be_host, sve_ld1##SUFF##_be_tlb); \
} \
void HELPER(sve_ld##N##SUFF##_le_r_mte)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
@@ -5090,8 +5060,7 @@ void sve_stN_r(CPUARMState *env, uint64_t *vg, target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
const int esz, const int msz, const int N, uint32_t mtedesc,
sve_ldst1_host_fn *host_fn,
- sve_ldst1_tlb_fn *tlb_fn,
- sve_cont_ldst_mte_check_fn *mte_check_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
const unsigned rd = simd_data(desc);
const intptr_t reg_max = simd_oprsz(desc);
@@ -5117,9 +5086,9 @@ void sve_stN_r(CPUARMState *env, uint64_t *vg, target_ulong addr,
* Handle mte checks for all active elements.
* Since TBI must be set for MTE, !mtedesc => !mte_active.
*/
- if (mte_check_fn && mtedesc) {
- mte_check_fn(&info, env, vg, addr, 1 << esz, N << msz,
- mtedesc, retaddr);
+ if (mtedesc) {
+ sve_cont_ldst_mte_check(&info, env, vg, addr, 1 << esz, N << msz,
+ mtedesc, retaddr);
}
flags = info.page[0].flags | info.page[1].flags;
@@ -5233,8 +5202,7 @@ void sve_stN_r_mte(CPUARMState *env, uint64_t *vg, target_ulong addr,
mtedesc = 0;
}
- sve_stN_r(env, vg, addr, desc, ra, esz, msz, N, mtedesc, host_fn, tlb_fn,
- N == 1 ? sve_cont_ldst_mte_check1 : sve_cont_ldst_mte_checkN);
+ sve_stN_r(env, vg, addr, desc, ra, esz, msz, N, mtedesc, host_fn, tlb_fn);
}
#define DO_STN_1(N, NAME, ESZ) \
@@ -5242,7 +5210,7 @@ void HELPER(sve_st##N##NAME##_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_stN_r(env, vg, addr, desc, GETPC(), ESZ, MO_8, N, 0, \
- sve_st1##NAME##_host, sve_st1##NAME##_tlb, NULL); \
+ sve_st1##NAME##_host, sve_st1##NAME##_tlb); \
} \
void HELPER(sve_st##N##NAME##_r_mte)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
@@ -5256,13 +5224,13 @@ void HELPER(sve_st##N##NAME##_le_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_stN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, N, 0, \
- sve_st1##NAME##_le_host, sve_st1##NAME##_le_tlb, NULL); \
+ sve_st1##NAME##_le_host, sve_st1##NAME##_le_tlb); \
} \
void HELPER(sve_st##N##NAME##_be_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
sve_stN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, N, 0, \
- sve_st1##NAME##_be_host, sve_st1##NAME##_be_tlb, NULL); \
+ sve_st1##NAME##_be_host, sve_st1##NAME##_be_tlb); \
} \
void HELPER(sve_st##N##NAME##_le_r_mte)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 11/12] target/arm: Remove log2_esize parameter to gen_mte_checkN
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (9 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 10/12] target/arm: Simplify sve mte checking Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-08 11:35 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
` (2 subsequent siblings)
13 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
The log2_esize parameter is not used except trivially.
Drop the parameter and the deferral to gen_mte_check1.
This fixes a bug in that the parameters as documented
in the header file were the reverse from those in the
implementation. Which meant that translate-sve.c was
passing the parameters in the wrong order.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate-a64.h | 2 +-
target/arm/translate-a64.c | 15 +++++++--------
target/arm/translate-sve.c | 4 ++--
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
index 3668b671dd..868d355048 100644
--- a/target/arm/translate-a64.h
+++ b/target/arm/translate-a64.h
@@ -44,7 +44,7 @@ TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr);
TCGv_i64 gen_mte_check1(DisasContext *s, TCGv_i64 addr, bool is_write,
bool tag_checked, int log2_size);
TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
- bool tag_checked, int count, int log2_esize);
+ bool tag_checked, int size);
/* We should have at some point before trying to access an FP register
* done the necessary access check, so assert that
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index a68d5dd5d1..f35a5e8174 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -295,9 +295,9 @@ TCGv_i64 gen_mte_check1(DisasContext *s, TCGv_i64 addr, bool is_write,
* For MTE, check multiple logical sequential accesses.
*/
TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
- bool tag_checked, int log2_esize, int total_size)
+ bool tag_checked, int size)
{
- if (tag_checked && s->mte_active[0] && total_size != (1 << log2_esize)) {
+ if (tag_checked && s->mte_active[0]) {
TCGv_i32 tcg_desc;
TCGv_i64 ret;
int desc = 0;
@@ -306,7 +306,7 @@ TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
desc = FIELD_DP32(desc, MTEDESC, TBI, s->tbid);
desc = FIELD_DP32(desc, MTEDESC, TCMA, s->tcma);
desc = FIELD_DP32(desc, MTEDESC, WRITE, is_write);
- desc = FIELD_DP32(desc, MTEDESC, SIZEM1, total_size - 1);
+ desc = FIELD_DP32(desc, MTEDESC, SIZEM1, size - 1);
tcg_desc = tcg_const_i32(desc);
ret = new_tmp_a64(s);
@@ -315,7 +315,7 @@ TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
return ret;
}
- return gen_mte_check1(s, addr, is_write, tag_checked, log2_esize);
+ return clean_data_tbi(s, addr);
}
typedef struct DisasCompare64 {
@@ -2965,8 +2965,7 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn)
}
clean_addr = gen_mte_checkN(s, dirty_addr, !is_load,
- (wback || rn != 31) && !set_tag,
- size, 2 << size);
+ (wback || rn != 31) && !set_tag, 2 << size);
if (is_vector) {
if (is_load) {
@@ -3713,7 +3712,7 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
* promote consecutive little-endian elements below.
*/
clean_addr = gen_mte_checkN(s, tcg_rn, is_store, is_postidx || rn != 31,
- size, total);
+ total);
/*
* Consecutive little-endian elements from a single register
@@ -3866,7 +3865,7 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
tcg_rn = cpu_reg_sp(s, rn);
clean_addr = gen_mte_checkN(s, tcg_rn, !is_load, is_postidx || rn != 31,
- scale, total);
+ total);
tcg_ebytes = tcg_const_i64(1 << scale);
for (xs = 0; xs < selem; xs++) {
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 5179c1f836..584c4d047c 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -4264,7 +4264,7 @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm)
dirty_addr = tcg_temp_new_i64();
tcg_gen_addi_i64(dirty_addr, cpu_reg_sp(s, rn), imm);
- clean_addr = gen_mte_checkN(s, dirty_addr, false, rn != 31, len, MO_8);
+ clean_addr = gen_mte_checkN(s, dirty_addr, false, rn != 31, len);
tcg_temp_free_i64(dirty_addr);
/*
@@ -4352,7 +4352,7 @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm)
dirty_addr = tcg_temp_new_i64();
tcg_gen_addi_i64(dirty_addr, cpu_reg_sp(s, rn), imm);
- clean_addr = gen_mte_checkN(s, dirty_addr, false, rn != 31, len, MO_8);
+ clean_addr = gen_mte_checkN(s, dirty_addr, false, rn != 31, len);
tcg_temp_free_i64(dirty_addr);
/* Note that unpredicated load/store of vector/predicate registers
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (10 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 11/12] target/arm: Remove log2_esize parameter to gen_mte_checkN Richard Henderson
@ 2021-04-06 17:40 ` Richard Henderson
2021-04-06 18:21 ` Laurent Vivier
` (3 more replies)
2021-04-06 17:57 ` [PATCH v4 00/12] target/arm mte fixes no-reply
2021-04-08 12:47 ` Peter Maydell
13 siblings, 4 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-06 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Laurent Vivier
Unfortuately, the elements of PAGE_* were not in numerical
order and so PAGE_ANON was added to an "unused" bit.
As an arbitrary choice, move PAGE_TARGET_{1,2} together.
Cc: Laurent Vivier <laurent@vivier.eu>
Fixes: 26bab757d41b ("linux-user: Introduce PAGE_ANON")
Buglink: https://bugs.launchpad.net/bugs/1922617
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index d76b0b9e02..32cfb634c6 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -268,8 +268,8 @@ extern intptr_t qemu_host_page_mask;
#define PAGE_RESERVED 0x0100
#endif
/* Target-specific bits that will be used via page_get_flags(). */
-#define PAGE_TARGET_1 0x0080
-#define PAGE_TARGET_2 0x0200
+#define PAGE_TARGET_1 0x0200
+#define PAGE_TARGET_2 0x0400
#if defined(CONFIG_USER_ONLY)
void page_dump(FILE *f);
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* Re: [PATCH v4 00/12] target/arm mte fixes
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (11 preceding siblings ...)
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
@ 2021-04-06 17:57 ` no-reply
2021-04-08 12:47 ` Peter Maydell
13 siblings, 0 replies; 63+ messages in thread
From: no-reply @ 2021-04-06 17:57 UTC (permalink / raw)
To: richard.henderson; +Cc: qemu-arm, qemu-devel
Patchew URL: https://patchew.org/QEMU/20210406174031.64299-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210406174031.64299-1-richard.henderson@linaro.org
Subject: [PATCH v4 00/12] target/arm mte fixes
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
4216ba1..d0d3dd4 master -> master
* [new tag] patchew/20210406174031.64299-1-richard.henderson@linaro.org -> patchew/20210406174031.64299-1-richard.henderson@linaro.org
Switched to a new branch 'test'
141183d exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
ed40d9f target/arm: Remove log2_esize parameter to gen_mte_checkN
ee9a6b6 target/arm: Simplify sve mte checking
dafd1fd target/arm: Rename mte_probe1 to mte_probe
b8221af target/arm: Merge mte_check1, mte_checkN
016a7b8 target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
94b32fd test/tcg/aarch64: Add mte-5
9a2c49d target/arm: Fix unaligned checks for mte_check1, mte_probe1
273f7ff target/arm: Split out mte_probe_int
36a80bd target/arm: Fix mte_checkN
b27cf9c target/arm: Check PAGE_WRITE_ORG for MTE writeability
02265f3 accel/tcg: Preserve PAGE_ANON when changing page permissions
=== OUTPUT BEGIN ===
1/12 Checking commit 02265f3d4c09 (accel/tcg: Preserve PAGE_ANON when changing page permissions)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#66:
new file mode 100644
ERROR: "foo * bar" should be "foo *bar"
#123: FILE: tests/tcg/aarch64/mte.h:51:
+static void * alloc_mte_mem(size_t size) __attribute__((unused));
ERROR: "foo * bar" should be "foo *bar"
#124: FILE: tests/tcg/aarch64/mte.h:52:
+static void * alloc_mte_mem(size_t size)
total: 2 errors, 1 warnings, 84 lines checked
Patch 1/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/12 Checking commit b27cf9cb34e9 (target/arm: Check PAGE_WRITE_ORG for MTE writeability)
WARNING: line over 80 characters
#28: FILE: target/arm/mte_helper.c:86:
+ if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE_ORG : PAGE_READ))) {
total: 0 errors, 1 warnings, 8 lines checked
Patch 2/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/12 Checking commit 36a80bd11e76 (target/arm: Fix mte_checkN)
4/12 Checking commit 273f7ff6a310 (target/arm: Split out mte_probe_int)
5/12 Checking commit 9a2c49d51da9 (target/arm: Fix unaligned checks for mte_check1, mte_probe1)
6/12 Checking commit 94b32fd20e68 (test/tcg/aarch64: Add mte-5)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29:
new file mode 100644
total: 0 errors, 1 warnings, 52 lines checked
Patch 6/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/12 Checking commit 016a7b87a158 (target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1)
8/12 Checking commit b8221af4c2a2 (target/arm: Merge mte_check1, mte_checkN)
9/12 Checking commit dafd1fdfbae1 (target/arm: Rename mte_probe1 to mte_probe)
10/12 Checking commit ee9a6b6f448c (target/arm: Simplify sve mte checking)
ERROR: spaces required around that '*' (ctx:WxV)
#95: FILE: target/arm/sve_helper.c:4438:
+ sve_ldst1_tlb_fn *tlb_fn)
^
ERROR: spaces required around that '*' (ctx:WxV)
#189: FILE: target/arm/sve_helper.c:5063:
+ sve_ldst1_tlb_fn *tlb_fn)
^
total: 2 errors, 0 warnings, 202 lines checked
Patch 10/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/12 Checking commit ed40d9f2b918 (target/arm: Remove log2_esize parameter to gen_mte_checkN)
12/12 Checking commit 141183d47a07 (exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210406174031.64299-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
@ 2021-04-06 18:21 ` Laurent Vivier
2021-04-06 19:36 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 0 replies; 63+ messages in thread
From: Laurent Vivier @ 2021-04-06 18:21 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
Le 06/04/2021 à 19:40, Richard Henderson a écrit :
> Unfortuately, the elements of PAGE_* were not in numerical
> order and so PAGE_ANON was added to an "unused" bit.
> As an arbitrary choice, move PAGE_TARGET_{1,2} together.
>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Fixes: 26bab757d41b ("linux-user: Introduce PAGE_ANON")
> Buglink: https://bugs.launchpad.net/bugs/1922617
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/cpu-all.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index d76b0b9e02..32cfb634c6 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -268,8 +268,8 @@ extern intptr_t qemu_host_page_mask;
> #define PAGE_RESERVED 0x0100
> #endif
> /* Target-specific bits that will be used via page_get_flags(). */
> -#define PAGE_TARGET_1 0x0080
> -#define PAGE_TARGET_2 0x0200
> +#define PAGE_TARGET_1 0x0200
> +#define PAGE_TARGET_2 0x0400
>
> #if defined(CONFIG_USER_ONLY)
> void page_dump(FILE *f);
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
2021-04-06 18:21 ` Laurent Vivier
@ 2021-04-06 19:36 ` Laurent Vivier
2021-04-07 17:16 ` Alex Bennée
2021-04-07 21:33 ` Nathan Chancellor
3 siblings, 0 replies; 63+ messages in thread
From: Laurent Vivier @ 2021-04-06 19:36 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
Le 06/04/2021 à 19:40, Richard Henderson a écrit :
> Unfortuately, the elements of PAGE_* were not in numerical
> order and so PAGE_ANON was added to an "unused" bit.
> As an arbitrary choice, move PAGE_TARGET_{1,2} together.
>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Fixes: 26bab757d41b ("linux-user: Introduce PAGE_ANON")
> Buglink: https://bugs.launchpad.net/bugs/1922617
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/cpu-all.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index d76b0b9e02..32cfb634c6 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -268,8 +268,8 @@ extern intptr_t qemu_host_page_mask;
> #define PAGE_RESERVED 0x0100
> #endif
> /* Target-specific bits that will be used via page_get_flags(). */
> -#define PAGE_TARGET_1 0x0080
> -#define PAGE_TARGET_2 0x0200
> +#define PAGE_TARGET_1 0x0200
> +#define PAGE_TARGET_2 0x0400
>
> #if defined(CONFIG_USER_ONLY)
> void page_dump(FILE *f);
>
Tested-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions
2021-04-06 17:40 ` [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions Richard Henderson
@ 2021-04-07 13:55 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 13:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel, Stephen Long
Richard Henderson <richard.henderson@linaro.org> writes:
> Using mprotect() to change PROT_* does not change the MAP_ANON
> previously set with mmap(). Our linux-user version of MTE only
> works with MAP_ANON pages, so losing PAGE_ANON caused MTE to
> stop working.
>
> Reported-by: Stephen Long <steplong@quicinc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Yay test case ;-) It certainly fails before and works after.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 02/12] target/arm: Check PAGE_WRITE_ORG for MTE writeability
2021-04-06 17:40 ` [PATCH v4 02/12] target/arm: Check PAGE_WRITE_ORG for MTE writeability Richard Henderson
@ 2021-04-07 15:34 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 15:34 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> We can remove PAGE_WRITE when (internally) marking a page
> read-only because it contains translated code.
>
> This can be triggered by tests/tcg/aarch64/bti-2, after
> having serviced SIGILL trampolines on the stack.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
2021-04-06 18:21 ` Laurent Vivier
2021-04-06 19:36 ` Laurent Vivier
@ 2021-04-07 17:16 ` Alex Bennée
2021-04-07 21:33 ` Nathan Chancellor
3 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 17:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel, Laurent Vivier
Richard Henderson <richard.henderson@linaro.org> writes:
> Unfortuately, the elements of PAGE_* were not in numerical
> order and so PAGE_ANON was added to an "unused" bit.
> As an arbitrary choice, move PAGE_TARGET_{1,2} together.
>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Fixes: 26bab757d41b ("linux-user: Introduce PAGE_ANON")
> Buglink: https://bugs.launchpad.net/bugs/1922617
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-06 17:40 ` [PATCH v4 03/12] target/arm: Fix mte_checkN Richard Henderson
@ 2021-04-07 18:39 ` Alex Bennée
2021-04-07 18:39 ` [Bug 1921948] " Alex Bennée
` (2 more replies)
0 siblings, 3 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 18:39 UTC (permalink / raw)
To: Richard Henderson; +Cc: 1921948, qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> We were incorrectly assuming that only the first byte of an MTE access
> is checked against the tags. But per the ARM, unaligned accesses are
> pre-decomposed into single-byte accesses. So by the time we reach the
> actual MTE check in the ARM pseudocode, all accesses are aligned.
>
> Therefore, the first failure is always either the first byte of the
> access, or the first byte of the granule.
>
> In addition, some of the arithmetic is off for last-first -> count.
> This does not become directly visible until a later patch that passes
> single bytes into this function, so ptr == ptr_last.
>
> Buglink: https://bugs.launchpad.net/bugs/1921948
Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
automatically copy patches to the appropriate bugs which is useful if
you don't have the Cc for the reporter.
Anyway I'm trying to get the kasas unit tests running as a way of
testing this (and maybe expanding with a version of Andrey's test). I
suspect this may be a PEBCAC issue but I built an MTE enabled kernel
with:
CONFIG_HAVE_ARCH_KASAN=y
CONFIG_HAVE_ARCH_KASAN_SW_TAGS=y
CONFIG_HAVE_ARCH_KASAN_HW_TAGS=y
CONFIG_CC_HAS_KASAN_GENERIC=y
CONFIG_KASAN=y
# CONFIG_KASAN_GENERIC is not set
CONFIG_KASAN_HW_TAGS=y
CONFIG_KASAN_STACK=1
CONFIG_KASAN_KUNIT_TEST=m
CONFIG_TEST_KASAN_MODULE=m
and was able to boot it. But when I insmod the kasan tests:
insmod test_kasan.ko
it looks like it just keeps looping failing on the same test:
Ignoring spurious kernel translation fault at virtual address dead00000000010a
WARNING: CPU: 0 PID: 1444 at arch/arm64/mm/fault.c:364 __do_kernel_fault+0xc4/0x1bc
Modules linked in: test_kasan(+)
CPU: 0 PID: 1444 Comm: kunit_try_catch Tainted: G B W 5.11.0-ajb-kasan #3
Hardware name: linux,dummy-virt (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO BTYPE=--)
pc : __do_kernel_fault+0xc4/0x1bc
lr : __do_kernel_fault+0xc4/0x1bc
sp : ffffffc01191b900
x29: ffffffc01191b900 x28: fcffff8001f7a880
x27: fcffff8001c01e00 x26: 0000000000000000
x25: 0000000000000001 x24: 00000000000000f4
x23: 0000000020400009 x22: dead00000000010a
x21: 0000000000000025 x20: ffffffc01191b9d0
x19: 0000000097c08004 x18: 0000000000000000
x17: 000000000000000a x16: 000017a83fb75794
x15: 0000000000000030 x14: 6c656e72656b2073
x13: ffffffc010e21be0 x12: 00000000000001aa
x11: 000000000000008e x10: ffffffc010e2d930
x9 : 000000000003a6d0 x8 : ffffffc010e21be0
x7 : ffffffc010e2cbe0 x6 : 0000000000000d50
x5 : ffffff8007f9c850 x4 : ffffffc01191b700
x3 : 0000000000000001 x2 : 0000000000000000
x1 : 0000000000000000 x0 : fcffff8001f7a880
Call trace:
__do_kernel_fault+0xc4/0x1bc
do_translation_fault+0x98/0xb0
do_mem_abort+0x44/0xb0
el1_abort+0x40/0x6c
el1_sync_handler+0x6c/0xb0
el1_sync+0x70/0x100
kasan_update_kunit_status+0x6c/0x1ac
kasan_report_invalid_free+0x34/0xa0
____kasan_slab_free.constprop.0+0xf8/0x1a0
__kasan_slab_free+0x10/0x20
slab_free_freelist_hook+0xf8/0x1a0
kfree+0x148/0x25c
kunit_destroy_resource+0x15c/0x1bc
string_stream_destroy+0x20/0x80
kunit_do_assertion+0x190/0x1e4
kmalloc_double_kzfree+0x158/0x190 [test_kasan]
kunit_try_run_case+0x78/0xa4
kunit_generic_run_threadfn_adapter+0x20/0x2c
kthread+0x134/0x144
ret_from_fork+0x10/0x38
---[ end trace 5acd02cdb9b3d3f0 ]---
but maybe I'm using the kunit tests wrong. It's my first time playing
with them.
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/mte_helper.c | 38 +++++++++++++++++---------------------
> 1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 8be17e1b70..c87717127c 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -757,10 +757,10 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> uint64_t ptr, uintptr_t ra)
> {
> int mmu_idx, ptr_tag, bit55;
> - uint64_t ptr_last, ptr_end, prev_page, next_page;
> - uint64_t tag_first, tag_end;
> - uint64_t tag_byte_first, tag_byte_end;
> - uint32_t esize, total, tag_count, tag_size, n, c;
> + uint64_t ptr_last, prev_page, next_page;
> + uint64_t tag_first, tag_last;
> + uint64_t tag_byte_first, tag_byte_last;
> + uint32_t total, tag_count, tag_size, n, c;
> uint8_t *mem1, *mem2;
> MMUAccessType type;
>
> @@ -779,29 +779,27 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
>
> mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
> type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
> - esize = FIELD_EX32(desc, MTEDESC, ESIZE);
> total = FIELD_EX32(desc, MTEDESC, TSIZE);
>
> /* Find the addr of the end of the access, and of the last element. */
> - ptr_end = ptr + total;
> - ptr_last = ptr_end - esize;
> + ptr_last = ptr + total - 1;
>
> /* Round the bounds to the tag granule, and compute the number of tags. */
> tag_first = QEMU_ALIGN_DOWN(ptr, TAG_GRANULE);
> - tag_end = QEMU_ALIGN_UP(ptr_last, TAG_GRANULE);
> - tag_count = (tag_end - tag_first) / TAG_GRANULE;
> + tag_last = QEMU_ALIGN_DOWN(ptr_last, TAG_GRANULE);
> + tag_count = ((tag_last - tag_first) / TAG_GRANULE) + 1;
>
> /* Round the bounds to twice the tag granule, and compute the bytes. */
> tag_byte_first = QEMU_ALIGN_DOWN(ptr, 2 * TAG_GRANULE);
> - tag_byte_end = QEMU_ALIGN_UP(ptr_last, 2 * TAG_GRANULE);
> + tag_byte_last = QEMU_ALIGN_DOWN(ptr_last, 2 * TAG_GRANULE);
>
> /* Locate the page boundaries. */
> prev_page = ptr & TARGET_PAGE_MASK;
> next_page = prev_page + TARGET_PAGE_SIZE;
>
> - if (likely(tag_end - prev_page <= TARGET_PAGE_SIZE)) {
> + if (likely(tag_last - prev_page <= TARGET_PAGE_SIZE)) {
> /* Memory access stays on one page. */
> - tag_size = (tag_byte_end - tag_byte_first) / (2 * TAG_GRANULE);
> + tag_size = ((tag_byte_last - tag_byte_first) / (2 * TAG_GRANULE)) + 1;
> mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, total,
> MMU_DATA_LOAD, tag_size, ra);
> if (!mem1) {
> @@ -815,9 +813,9 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, next_page - ptr,
> MMU_DATA_LOAD, tag_size, ra);
>
> - tag_size = (tag_byte_end - next_page) / (2 * TAG_GRANULE);
> + tag_size = ((tag_byte_last - next_page) / (2 * TAG_GRANULE)) + 1;
> mem2 = allocation_tag_mem(env, mmu_idx, next_page, type,
> - ptr_end - next_page,
> + ptr_last - next_page + 1,
> MMU_DATA_LOAD, tag_size, ra);
>
> /*
> @@ -838,15 +836,13 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> }
>
> /*
> - * If we failed, we know which granule. Compute the element that
> - * is first in that granule, and signal failure on that element.
> + * If we failed, we know which granule. For the first granule, the
> + * failure address is @ptr, the first byte accessed. Otherwise the
> + * failure address is the first byte of the nth granule.
> */
> if (unlikely(n < tag_count)) {
> - uint64_t fail_ofs;
> -
> - fail_ofs = tag_first + n * TAG_GRANULE - ptr;
> - fail_ofs = ROUND_UP(fail_ofs, esize);
> - mte_check_fail(env, desc, ptr + fail_ofs, ra);
> + uint64_t fault = (n == 0 ? ptr : tag_first + n * TAG_GRANULE);
> + mte_check_fail(env, desc, fault, ra);
> }
>
> done:
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-07 18:39 ` Alex Bennée
@ 2021-04-07 18:39 ` Alex Bennée
2021-04-07 19:56 ` Richard Henderson
2021-04-08 8:50 ` Peter Maydell
2 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 18:39 UTC (permalink / raw)
To: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> We were incorrectly assuming that only the first byte of an MTE access
> is checked against the tags. But per the ARM, unaligned accesses are
> pre-decomposed into single-byte accesses. So by the time we reach the
> actual MTE check in the ARM pseudocode, all accesses are aligned.
>
> Therefore, the first failure is always either the first byte of the
> access, or the first byte of the granule.
>
> In addition, some of the arithmetic is off for last-first -> count.
> This does not become directly visible until a later patch that passes
> single bytes into this function, so ptr == ptr_last.
>
> Buglink: https://bugs.launchpad.net/bugs/1921948
Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
automatically copy patches to the appropriate bugs which is useful if
you don't have the Cc for the reporter.
Anyway I'm trying to get the kasas unit tests running as a way of
testing this (and maybe expanding with a version of Andrey's test). I
suspect this may be a PEBCAC issue but I built an MTE enabled kernel
with:
CONFIG_HAVE_ARCH_KASAN=y
CONFIG_HAVE_ARCH_KASAN_SW_TAGS=y
CONFIG_HAVE_ARCH_KASAN_HW_TAGS=y
CONFIG_CC_HAS_KASAN_GENERIC=y
CONFIG_KASAN=y
# CONFIG_KASAN_GENERIC is not set
CONFIG_KASAN_HW_TAGS=y
CONFIG_KASAN_STACK=1
CONFIG_KASAN_KUNIT_TEST=m
CONFIG_TEST_KASAN_MODULE=m
and was able to boot it. But when I insmod the kasan tests:
insmod test_kasan.ko
it looks like it just keeps looping failing on the same test:
Ignoring spurious kernel translation fault at virtual address dead00000000010a
WARNING: CPU: 0 PID: 1444 at arch/arm64/mm/fault.c:364 __do_kernel_fault+0xc4/0x1bc
Modules linked in: test_kasan(+)
CPU: 0 PID: 1444 Comm: kunit_try_catch Tainted: G B W 5.11.0-ajb-kasan #3
Hardware name: linux,dummy-virt (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO BTYPE=--)
pc : __do_kernel_fault+0xc4/0x1bc
lr : __do_kernel_fault+0xc4/0x1bc
sp : ffffffc01191b900
x29: ffffffc01191b900 x28: fcffff8001f7a880
x27: fcffff8001c01e00 x26: 0000000000000000
x25: 0000000000000001 x24: 00000000000000f4
x23: 0000000020400009 x22: dead00000000010a
x21: 0000000000000025 x20: ffffffc01191b9d0
x19: 0000000097c08004 x18: 0000000000000000
x17: 000000000000000a x16: 000017a83fb75794
x15: 0000000000000030 x14: 6c656e72656b2073
x13: ffffffc010e21be0 x12: 00000000000001aa
x11: 000000000000008e x10: ffffffc010e2d930
x9 : 000000000003a6d0 x8 : ffffffc010e21be0
x7 : ffffffc010e2cbe0 x6 : 0000000000000d50
x5 : ffffff8007f9c850 x4 : ffffffc01191b700
x3 : 0000000000000001 x2 : 0000000000000000
x1 : 0000000000000000 x0 : fcffff8001f7a880
Call trace:
__do_kernel_fault+0xc4/0x1bc
do_translation_fault+0x98/0xb0
do_mem_abort+0x44/0xb0
el1_abort+0x40/0x6c
el1_sync_handler+0x6c/0xb0
el1_sync+0x70/0x100
kasan_update_kunit_status+0x6c/0x1ac
kasan_report_invalid_free+0x34/0xa0
____kasan_slab_free.constprop.0+0xf8/0x1a0
__kasan_slab_free+0x10/0x20
slab_free_freelist_hook+0xf8/0x1a0
kfree+0x148/0x25c
kunit_destroy_resource+0x15c/0x1bc
string_stream_destroy+0x20/0x80
kunit_do_assertion+0x190/0x1e4
kmalloc_double_kzfree+0x158/0x190 [test_kasan]
kunit_try_run_case+0x78/0xa4
kunit_generic_run_threadfn_adapter+0x20/0x2c
kthread+0x134/0x144
ret_from_fork+0x10/0x38
---[ end trace 5acd02cdb9b3d3f0 ]---
but maybe I'm using the kunit tests wrong. It's my first time playing
with them.
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/mte_helper.c | 38 +++++++++++++++++---------------------
> 1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 8be17e1b70..c87717127c 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -757,10 +757,10 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> uint64_t ptr, uintptr_t ra)
> {
> int mmu_idx, ptr_tag, bit55;
> - uint64_t ptr_last, ptr_end, prev_page, next_page;
> - uint64_t tag_first, tag_end;
> - uint64_t tag_byte_first, tag_byte_end;
> - uint32_t esize, total, tag_count, tag_size, n, c;
> + uint64_t ptr_last, prev_page, next_page;
> + uint64_t tag_first, tag_last;
> + uint64_t tag_byte_first, tag_byte_last;
> + uint32_t total, tag_count, tag_size, n, c;
> uint8_t *mem1, *mem2;
> MMUAccessType type;
>
> @@ -779,29 +779,27 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
>
> mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
> type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
> - esize = FIELD_EX32(desc, MTEDESC, ESIZE);
> total = FIELD_EX32(desc, MTEDESC, TSIZE);
>
> /* Find the addr of the end of the access, and of the last element. */
> - ptr_end = ptr + total;
> - ptr_last = ptr_end - esize;
> + ptr_last = ptr + total - 1;
>
> /* Round the bounds to the tag granule, and compute the number of tags. */
> tag_first = QEMU_ALIGN_DOWN(ptr, TAG_GRANULE);
> - tag_end = QEMU_ALIGN_UP(ptr_last, TAG_GRANULE);
> - tag_count = (tag_end - tag_first) / TAG_GRANULE;
> + tag_last = QEMU_ALIGN_DOWN(ptr_last, TAG_GRANULE);
> + tag_count = ((tag_last - tag_first) / TAG_GRANULE) + 1;
>
> /* Round the bounds to twice the tag granule, and compute the bytes. */
> tag_byte_first = QEMU_ALIGN_DOWN(ptr, 2 * TAG_GRANULE);
> - tag_byte_end = QEMU_ALIGN_UP(ptr_last, 2 * TAG_GRANULE);
> + tag_byte_last = QEMU_ALIGN_DOWN(ptr_last, 2 * TAG_GRANULE);
>
> /* Locate the page boundaries. */
> prev_page = ptr & TARGET_PAGE_MASK;
> next_page = prev_page + TARGET_PAGE_SIZE;
>
> - if (likely(tag_end - prev_page <= TARGET_PAGE_SIZE)) {
> + if (likely(tag_last - prev_page <= TARGET_PAGE_SIZE)) {
> /* Memory access stays on one page. */
> - tag_size = (tag_byte_end - tag_byte_first) / (2 * TAG_GRANULE);
> + tag_size = ((tag_byte_last - tag_byte_first) / (2 * TAG_GRANULE)) + 1;
> mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, total,
> MMU_DATA_LOAD, tag_size, ra);
> if (!mem1) {
> @@ -815,9 +813,9 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, next_page - ptr,
> MMU_DATA_LOAD, tag_size, ra);
>
> - tag_size = (tag_byte_end - next_page) / (2 * TAG_GRANULE);
> + tag_size = ((tag_byte_last - next_page) / (2 * TAG_GRANULE)) + 1;
> mem2 = allocation_tag_mem(env, mmu_idx, next_page, type,
> - ptr_end - next_page,
> + ptr_last - next_page + 1,
> MMU_DATA_LOAD, tag_size, ra);
>
> /*
> @@ -838,15 +836,13 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> }
>
> /*
> - * If we failed, we know which granule. Compute the element that
> - * is first in that granule, and signal failure on that element.
> + * If we failed, we know which granule. For the first granule, the
> + * failure address is @ptr, the first byte accessed. Otherwise the
> + * failure address is the first byte of the nth granule.
> */
> if (unlikely(n < tag_count)) {
> - uint64_t fail_ofs;
> -
> - fail_ofs = tag_first + n * TAG_GRANULE - ptr;
> - fail_ofs = ROUND_UP(fail_ofs, esize);
> - mte_check_fail(env, desc, ptr + fail_ofs, ra);
> + uint64_t fault = (n == 0 ? ptr : tag_first + n * TAG_GRANULE);
> + mte_check_fail(env, desc, fault, ra);
> }
>
> done:
--
Alex Bennée
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-07 18:39 ` Alex Bennée
2021-04-07 18:39 ` [Bug 1921948] " Alex Bennée
@ 2021-04-07 19:56 ` Richard Henderson
2021-04-08 8:36 ` Alex Bennée
2021-04-08 8:50 ` Peter Maydell
2 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-07 19:56 UTC (permalink / raw)
To: Alex Bennée; +Cc: 1921948, qemu-arm, qemu-devel
On 4/7/21 11:39 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> We were incorrectly assuming that only the first byte of an MTE access
>> is checked against the tags. But per the ARM, unaligned accesses are
>> pre-decomposed into single-byte accesses. So by the time we reach the
>> actual MTE check in the ARM pseudocode, all accesses are aligned.
>>
>> Therefore, the first failure is always either the first byte of the
>> access, or the first byte of the granule.
>>
>> In addition, some of the arithmetic is off for last-first -> count.
>> This does not become directly visible until a later patch that passes
>> single bytes into this function, so ptr == ptr_last.
>>
>> Buglink: https://bugs.launchpad.net/bugs/1921948
>
> Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
> automatically copy patches to the appropriate bugs which is useful if
> you don't have the Cc for the reporter.
>
> Anyway I'm trying to get the kasas unit tests running as a way of
> testing this (and maybe expanding with a version of Andrey's test). I
> suspect this may be a PEBCAC issue but I built an MTE enabled kernel
> with:
>
> CONFIG_HAVE_ARCH_KASAN=y
> CONFIG_HAVE_ARCH_KASAN_SW_TAGS=y
> CONFIG_HAVE_ARCH_KASAN_HW_TAGS=y
> CONFIG_CC_HAS_KASAN_GENERIC=y
> CONFIG_KASAN=y
> # CONFIG_KASAN_GENERIC is not set
> CONFIG_KASAN_HW_TAGS=y
> CONFIG_KASAN_STACK=1
> CONFIG_KASAN_KUNIT_TEST=m
> CONFIG_TEST_KASAN_MODULE=m
I built it all in:
CONFIG_HAVE_ARCH_KASAN=y
CONFIG_HAVE_ARCH_KASAN_SW_TAGS=y
CONFIG_HAVE_ARCH_KASAN_HW_TAGS=y
CONFIG_CC_HAS_KASAN_GENERIC=y
CONFIG_KASAN=y
# CONFIG_KASAN_GENERIC is not set
CONFIG_KASAN_HW_TAGS=y
CONFIG_KASAN_KUNIT_TEST=y
Then I just boot the raw kernel (no filesystem or anything):
./qemu-system-aarch64 -M virt,mte=on -cpu max -nographic \
-kernel ~/linux/bld-aa/arch/arm64/boot/Image
There's a ton of output, but at the end I see
[ 11.901185] ok 48 - match_all_mem_tag
[ 11.901422] ok 1 - kasan
just before the "VFS: Cannot open root device" panic.
Which has done all we wanted, so, yay.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (6 preceding siblings ...)
2021-04-03 14:34 ` Andrey Konovalov
@ 2021-04-07 20:17 ` Andrey Konovalov
2021-04-07 20:46 ` Alex Bennée
` (9 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Andrey Konovalov @ 2021-04-07 20:17 UTC (permalink / raw)
To: qemu-devel
This warning is caused by "virtualization=on" QEMU option. This is
another QEMU bug AFAIU, see [1] and [2].
[1] https://lore.kernel.org/lkml/CAAeHK+wDz8aSLyjq1b=q3+HG9aJXxwYR6+gN_fTttMN5osM5gg@mail.gmail.com/
[2] https://lore.kernel.org/lkml/20210311123315.GF37303@C02TD0UTHF1T.local/T/
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (7 preceding siblings ...)
2021-04-07 20:17 ` Andrey Konovalov
@ 2021-04-07 20:46 ` Alex Bennée
2021-04-07 20:58 ` Andrey Konovalov
` (8 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 20:46 UTC (permalink / raw)
To: qemu-devel
It gets further without but still spams a lot of failure messages:
The buggy address belongs to the object at ffffff80036a2200
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 11 bytes to the right of
128-byte region [ffffff80036a2200, ffffff80036a2280)
The buggy address belongs to the page:
page:0000000046e01872 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x436a2
flags: 0x3fc0000000000200(slab)
raw: 3fc0000000000200 dead000000000100 dead000000000122 f9ffff8001c01e00
raw: 0000000000000000 0000000080100010 00000001ffffffff f3ffff80036a2401
page dumped because: kasan: bad access detected
pages's memcg:f3ffff80036a2401
Memory state around the buggy address:
ffffff80036a2000: f6 f6 f6 f6 f6 f6 f6 f6 fe fe fe fe fe fe fe fe
ffffff80036a2100: fa fa fa fa fe fe fe fe fe fe fe fe fe fe fe fe
>ffffff80036a2200: f9 f9 f9 f9 f9 f9 f9 f9 fe fe fe fe fe fe fe fe
^
ffffff80036a2300: fc fc fc fc fe fe fe fe fe fe fe fe fe fe fe fe
ffffff80036a2400: f3 f3 f3 f3 f3 f3 f3 f3 fe fe fe fe fe fe fe fe
==================================================================
Disabling lock debugging due to kernel taint
# kmalloc_oob_right: EXPECTATION FAILED at lib/test_kasan.c:86
Expected fail_data.report_expected == fail_data.report_found, but
fail_data.report_expected == 1
fail_data.report_found == 0
not ok 1 - kmalloc_oob_right
# kmalloc_oob_left: EXPECTATION FAILED at lib/test_kasan.c:98
Expected fail_data.report_expected == fail_data.report_found, but
fail_data.report_expected == 1
fail_data.report_found == 0
not ok 2 - kmalloc_oob_left
# kmalloc_node_oob_right: EXPECTATION FAILED at lib/test_kasan.c:110
Expected fail_data.report_expected == fail_data.report_found, but
fail_data.report_expected == 1
fail_data.report_found == 0
not ok 3 - kmalloc_node_oob_right
# kmalloc_pagealloc_oob_right: EXPECTATION FAILED at lib/test_kasan.c:130
Expected fail_data.report_expected == fail_data.report_found, but
fail_data.report_expected == 1
fail_data.report_found == 0
not ok 4 - kmalloc_pagealloc_oob_right
# kmalloc_pagealloc_uaf: EXPECTATION FAILED at lib/test_kasan.c:148
Expected fail_data.report_expected == fail_data.report_found, but
fail_data.report_expected == 1
fail_data.report_found == 0
not ok 5 - kmalloc_pagealloc_uaf
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (8 preceding siblings ...)
2021-04-07 20:46 ` Alex Bennée
@ 2021-04-07 20:58 ` Andrey Konovalov
2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:19 ` Richard Henderson
` (7 subsequent siblings)
17 siblings, 1 reply; 63+ messages in thread
From: Andrey Konovalov @ 2021-04-07 20:58 UTC (permalink / raw)
To: qemu-devel
Is this with QEMU master without the patches mentioned in this bug?
Which kernel version do you use?
Could you share your kernel config?
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (9 preceding siblings ...)
2021-04-07 20:58 ` Andrey Konovalov
@ 2021-04-07 21:19 ` Richard Henderson
2021-04-07 22:02 ` Andrey Konovalov
` (6 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-07 21:19 UTC (permalink / raw)
To: qemu-devel
Re comments #8 and #10, I don't replicate that.
I get full pass on KASAN_UNIT_TEST with
and without virtualization enabled.
Re comment #9, if there are bugs suspected in qemu, they
need to be reported, or we'll never hear about them.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-04-07 20:58 ` Andrey Konovalov
@ 2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:45 ` Alex Bennée
0 siblings, 2 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 21:29 UTC (permalink / raw)
To: Bug 1921948; +Cc: qemu-devel
Andrey Konovalov <1921948@bugs.launchpad.net> writes:
> Is this with QEMU master without the patches mentioned in this bug?
This is with Richard's latest series.
>
> Which kernel version do you use?
v5.11
> Could you share your kernel config?
We are just testing with Richard's config and eliminating compiler
shenanigans now.
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-04-07 21:29 ` Alex Bennée
@ 2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:45 ` Alex Bennée
1 sibling, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 21:29 UTC (permalink / raw)
To: qemu-devel
Andrey Konovalov <1921948@bugs.launchpad.net> writes:
> Is this with QEMU master without the patches mentioned in this bug?
This is with Richard's latest series.
>
> Which kernel version do you use?
v5.11
> Could you share your kernel config?
We are just testing with Richard's config and eliminating compiler
shenanigans now.
--
Alex Bennée
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
` (2 preceding siblings ...)
2021-04-07 17:16 ` Alex Bennée
@ 2021-04-07 21:33 ` Nathan Chancellor
3 siblings, 0 replies; 63+ messages in thread
From: Nathan Chancellor @ 2021-04-07 21:33 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel, Laurent Vivier
On Tue, Apr 06, 2021 at 10:40:31AM -0700, Richard Henderson wrote:
> Unfortuately, the elements of PAGE_* were not in numerical
> order and so PAGE_ANON was added to an "unused" bit.
> As an arbitrary choice, move PAGE_TARGET_{1,2} together.
>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Fixes: 26bab757d41b ("linux-user: Introduce PAGE_ANON")
> Buglink: https://bugs.launchpad.net/bugs/1922617
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
> ---
> include/exec/cpu-all.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index d76b0b9e02..32cfb634c6 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -268,8 +268,8 @@ extern intptr_t qemu_host_page_mask;
> #define PAGE_RESERVED 0x0100
> #endif
> /* Target-specific bits that will be used via page_get_flags(). */
> -#define PAGE_TARGET_1 0x0080
> -#define PAGE_TARGET_2 0x0200
> +#define PAGE_TARGET_1 0x0200
> +#define PAGE_TARGET_2 0x0400
>
> #if defined(CONFIG_USER_ONLY)
> void page_dump(FILE *f);
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:29 ` Alex Bennée
@ 2021-04-07 21:45 ` Alex Bennée
2021-04-07 21:45 ` Alex Bennée
1 sibling, 1 reply; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 21:45 UTC (permalink / raw)
To: Bug 1921948; +Cc: qemu-devel
Alex Bennée <alex.bennee@linaro.org> writes:
> Andrey Konovalov <1921948@bugs.launchpad.net> writes:
>
>> Is this with QEMU master without the patches mentioned in this bug?
>
> This is with Richard's latest series.
>
>>
>> Which kernel version do you use?
>
> v5.11
>
>> Could you share your kernel config?
>
> We are just testing with Richard's config and eliminating compiler
> shenanigans now.
OK with v5.12-rc5 and Richard's config I get a clean pass.
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-04-07 21:45 ` Alex Bennée
@ 2021-04-07 21:45 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-07 21:45 UTC (permalink / raw)
To: qemu-devel
Alex Bennée <alex.bennee@linaro.org> writes:
> Andrey Konovalov <1921948@bugs.launchpad.net> writes:
>
>> Is this with QEMU master without the patches mentioned in this bug?
>
> This is with Richard's latest series.
>
>>
>> Which kernel version do you use?
>
> v5.11
>
>> Could you share your kernel config?
>
> We are just testing with Richard's config and eliminating compiler
> shenanigans now.
OK with v5.12-rc5 and Richard's config I get a clean pass.
--
Alex Bennée
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (10 preceding siblings ...)
2021-04-07 21:19 ` Richard Henderson
@ 2021-04-07 22:02 ` Andrey Konovalov
2021-05-06 18:39 ` Richard Henderson
` (5 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Andrey Konovalov @ 2021-04-07 22:02 UTC (permalink / raw)
To: qemu-devel
Ah, there's v4 now.
Tested with KASAN tests + a custom test to check unaligned accesses that
span across two granules, everything works.
Thank you!
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-07 19:56 ` Richard Henderson
@ 2021-04-08 8:36 ` Alex Bennée
2021-04-08 8:36 ` [Bug 1921948] " Alex Bennée
0 siblings, 1 reply; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 8:36 UTC (permalink / raw)
To: Richard Henderson; +Cc: 1921948, qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 4/7/21 11:39 AM, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> We were incorrectly assuming that only the first byte of an MTE access
>>> is checked against the tags. But per the ARM, unaligned accesses are
>>> pre-decomposed into single-byte accesses. So by the time we reach the
>>> actual MTE check in the ARM pseudocode, all accesses are aligned.
>>>
>>> Therefore, the first failure is always either the first byte of the
>>> access, or the first byte of the granule.
>>>
<snip>
I replicated the original test case as a kunit test:
static void kmalloc_node_oob_span_right(struct kunit *test)
{
char *ptr;
size_t size = 128;
ptr = kmalloc_node(size, GFP_KERNEL, 0);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
KUNIT_EXPECT_KASAN_FAIL(test, *(volatile unsigned long *)(ptr + 124) = 0);
kfree(ptr);
}
which before this fix triggered:
[ 6.753429] # kmalloc_node_oob_span_right: EXPECTATION FAILED at lib/test_kasan.c:163
[ 6.753429] Expected ({ do { extern void __compiletime_assert_337(void) __attribute__((__error__("Unsupported access size for {READ,WRITE}_ONCE()."))); if (!((sizeof(
fail_data.report_expected) == sizeof(char) || sizeof(fail_data.report_expected) == sizeof(short) || sizeof(fail_data.report_expected) == sizeof(int) || sizeof(fail_data.repo
rt_expected) == sizeof(long)) || sizeof(fail_data.report_expected) == sizeof(long long))) __compiletime_assert_337(); } while (0); (*(const volatile typeof( _Generic((fail_d
ata.report_expected), char: (char)0, unsigned char: (unsigned char)0, signed char: (signed char)0, unsigned short: (unsigned short)0, signed short: (signed short)0, unsigned
int: (unsigned int)0, signed int: (signed int)0, unsigned long: (unsigned long)0, signed long: (signed long)0, unsigned long long: (unsigned long long)0, signed long long:
(signed long long)0, default: (fail_data.report_expected))) *
[ 6.759388] not ok 4 - kmalloc_node_oob_span_right
And is OK by the end of the series:
[ 6.587381] The buggy address belongs to the object at ffff000003325800
[ 6.587381] which belongs to the cache kmalloc-128 of size 128
[ 6.588372] The buggy address is located 0 bytes to the right of
[ 6.588372] 128-byte region [ffff000003325800, ffff000003325880)
[ 6.589354] The buggy address belongs to the page:
[ 6.589948] page:(____ptrval____) refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x43325
[ 6.590911] flags: 0x3fffc0000000200(slab)
[ 6.591648] raw: 03fffc0000000200 dead000000000100 dead000000000122 fdff000002401200
[ 6.592346] raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000
[ 6.593007] page dumped because: kasan: bad access detected
[ 6.593532]
[ 6.593775] Memory state around the buggy address:
[ 6.594360] ffff000003325600: f3 f3 f3 f3 f3 f3 f3 f3 fe fe fe fe fe fe fe fe
[ 6.594991] ffff000003325700: fd fd fd fd fd fd fd fd fe fe fe fe fe fe fe fe
[ 6.595594] >ffff000003325800: f8 f8 f8 f8 f8 f8 f8 f8 fe fe fe fe fe fe fe fe
[ 6.596180] ^
[ 6.596714] ffff000003325900: f7 f7 f7 f7 fe fe fe fe fe fe fe fe fe fe fe fe
[ 6.597309] ffff000003325a00: f4 f4 fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 6.597925] ==================================================================
[ 6.598513] Disabling lock debugging due to kernel taint
[ 6.600353] ok 1 - kmalloc_node_oob_span_right
[ 6.600554] ok 1 - kasan
But it still fails this patch until:
target/arm: Fix unaligned checks for mte_check1, mte_probe1
So I guess that is the one that should have the buglink.
Anyway code wise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-08 8:36 ` Alex Bennée
@ 2021-04-08 8:36 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 8:36 UTC (permalink / raw)
To: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 4/7/21 11:39 AM, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> We were incorrectly assuming that only the first byte of an MTE access
>>> is checked against the tags. But per the ARM, unaligned accesses are
>>> pre-decomposed into single-byte accesses. So by the time we reach the
>>> actual MTE check in the ARM pseudocode, all accesses are aligned.
>>>
>>> Therefore, the first failure is always either the first byte of the
>>> access, or the first byte of the granule.
>>>
<snip>
I replicated the original test case as a kunit test:
static void kmalloc_node_oob_span_right(struct kunit *test)
{
char *ptr;
size_t size = 128;
ptr = kmalloc_node(size, GFP_KERNEL, 0);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
KUNIT_EXPECT_KASAN_FAIL(test, *(volatile unsigned long *)(ptr + 124) = 0);
kfree(ptr);
}
which before this fix triggered:
[ 6.753429] # kmalloc_node_oob_span_right: EXPECTATION FAILED at lib/test_kasan.c:163
[ 6.753429] Expected ({ do { extern void __compiletime_assert_337(void) __attribute__((__error__("Unsupported access size for {READ,WRITE}_ONCE()."))); if (!((sizeof(
fail_data.report_expected) == sizeof(char) || sizeof(fail_data.report_expected) == sizeof(short) || sizeof(fail_data.report_expected) == sizeof(int) || sizeof(fail_data.repo
rt_expected) == sizeof(long)) || sizeof(fail_data.report_expected) == sizeof(long long))) __compiletime_assert_337(); } while (0); (*(const volatile typeof( _Generic((fail_d
ata.report_expected), char: (char)0, unsigned char: (unsigned char)0, signed char: (signed char)0, unsigned short: (unsigned short)0, signed short: (signed short)0, unsigned
int: (unsigned int)0, signed int: (signed int)0, unsigned long: (unsigned long)0, signed long: (signed long)0, unsigned long long: (unsigned long long)0, signed long long:
(signed long long)0, default: (fail_data.report_expected))) *
[ 6.759388] not ok 4 - kmalloc_node_oob_span_right
And is OK by the end of the series:
[ 6.587381] The buggy address belongs to the object at ffff000003325800
[ 6.587381] which belongs to the cache kmalloc-128 of size 128
[ 6.588372] The buggy address is located 0 bytes to the right of
[ 6.588372] 128-byte region [ffff000003325800, ffff000003325880)
[ 6.589354] The buggy address belongs to the page:
[ 6.589948] page:(____ptrval____) refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x43325
[ 6.590911] flags: 0x3fffc0000000200(slab)
[ 6.591648] raw: 03fffc0000000200 dead000000000100 dead000000000122 fdff000002401200
[ 6.592346] raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000
[ 6.593007] page dumped because: kasan: bad access detected
[ 6.593532]
[ 6.593775] Memory state around the buggy address:
[ 6.594360] ffff000003325600: f3 f3 f3 f3 f3 f3 f3 f3 fe fe fe fe fe fe fe fe
[ 6.594991] ffff000003325700: fd fd fd fd fd fd fd fd fe fe fe fe fe fe fe fe
[ 6.595594] >ffff000003325800: f8 f8 f8 f8 f8 f8 f8 f8 fe fe fe fe fe fe fe fe
[ 6.596180] ^
[ 6.596714] ffff000003325900: f7 f7 f7 f7 fe fe fe fe fe fe fe fe fe fe fe fe
[ 6.597309] ffff000003325a00: f4 f4 fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 6.597925] ==================================================================
[ 6.598513] Disabling lock debugging due to kernel taint
[ 6.600353] ok 1 - kmalloc_node_oob_span_right
[ 6.600554] ok 1 - kasan
But it still fails this patch until:
target/arm: Fix unaligned checks for mte_check1, mte_probe1
So I guess that is the one that should have the buglink.
Anyway code wise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-07 18:39 ` Alex Bennée
2021-04-07 18:39 ` [Bug 1921948] " Alex Bennée
2021-04-07 19:56 ` Richard Henderson
@ 2021-04-08 8:50 ` Peter Maydell
2021-04-08 8:50 ` [Bug 1921948] " Peter Maydell
2021-04-08 10:02 ` Alex Bennée
2 siblings, 2 replies; 63+ messages in thread
From: Peter Maydell @ 2021-04-08 8:50 UTC (permalink / raw)
To: Alex Bennée; +Cc: 1921948, qemu-arm, Richard Henderson, QEMU Developers
On Wed, 7 Apr 2021 at 19:54, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
> > We were incorrectly assuming that only the first byte of an MTE access
> > is checked against the tags. But per the ARM, unaligned accesses are
> > pre-decomposed into single-byte accesses. So by the time we reach the
> > actual MTE check in the ARM pseudocode, all accesses are aligned.
> >
> > Therefore, the first failure is always either the first byte of the
> > access, or the first byte of the granule.
> >
> > In addition, some of the arithmetic is off for last-first -> count.
> > This does not become directly visible until a later patch that passes
> > single bytes into this function, so ptr == ptr_last.
> >
> > Buglink: https://bugs.launchpad.net/bugs/1921948
>
> Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
> automatically copy patches to the appropriate bugs which is useful if
> you don't have the Cc for the reporter.
I'm not sure cc'ing bugs on patches is very useful, though (and especially
not for big series). I usually prefer to just add a note to the bug with
the URL of the series in patchew afterwards.
-- PMM
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-08 8:50 ` Peter Maydell
@ 2021-04-08 8:50 ` Peter Maydell
2021-04-08 10:02 ` Alex Bennée
1 sibling, 0 replies; 63+ messages in thread
From: Peter Maydell @ 2021-04-08 8:50 UTC (permalink / raw)
To: qemu-devel
On Wed, 7 Apr 2021 at 19:54, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
> > We were incorrectly assuming that only the first byte of an MTE access
> > is checked against the tags. But per the ARM, unaligned accesses are
> > pre-decomposed into single-byte accesses. So by the time we reach the
> > actual MTE check in the ARM pseudocode, all accesses are aligned.
> >
> > Therefore, the first failure is always either the first byte of the
> > access, or the first byte of the granule.
> >
> > In addition, some of the arithmetic is off for last-first -> count.
> > This does not become directly visible until a later patch that passes
> > single bytes into this function, so ptr == ptr_last.
> >
> > Buglink: https://bugs.launchpad.net/bugs/1921948
>
> Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
> automatically copy patches to the appropriate bugs which is useful if
> you don't have the Cc for the reporter.
I'm not sure cc'ing bugs on patches is very useful, though (and especially
not for big series). I usually prefer to just add a note to the bug with
the URL of the series in patchew afterwards.
-- PMM
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 04/12] target/arm: Split out mte_probe_int
2021-04-06 17:40 ` [PATCH v4 04/12] target/arm: Split out mte_probe_int Richard Henderson
@ 2021-04-08 9:01 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 9:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Split out a helper function from mte_checkN to perform
> all of the checking and address manpulation. So far,
> just use this in mte_checkN itself.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/mte_helper.c | 52 +++++++++++++++++++++++++++++++----------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index c87717127c..144bfa4a51 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -753,33 +753,45 @@ static int checkN(uint8_t *mem, int odd, int cmp, int count)
> return n;
> }
>
> -uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> - uint64_t ptr, uintptr_t ra)
> +/*
> + * mte_probe_int:
nit: you might as well go full kernel-doc here as you are documenting
the helper:
/**
* mte_probe_int() - helper for mte_probe and mte_check
> + * @env: CPU environment
> + * @desc: MTEDESC descriptor
> + * @ptr: virtual address of the base of the access
> + * @fault: return virtual address of the first check failure
> + *
> + * Internal routine for both mte_probe and mte_check.
> + * Return zero on failure, filling in *fault.
> + * Return negative on trivial success for tbi disabled.
> + * Return positive on success with tbi enabled.
> + */
> +static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
> + uintptr_t ra, uint32_t total, uint64_t *fault)
> {
> int mmu_idx, ptr_tag, bit55;
> uint64_t ptr_last, prev_page, next_page;
> uint64_t tag_first, tag_last;
> uint64_t tag_byte_first, tag_byte_last;
> - uint32_t total, tag_count, tag_size, n, c;
> + uint32_t tag_count, tag_size, n, c;
> uint8_t *mem1, *mem2;
> MMUAccessType type;
>
> bit55 = extract64(ptr, 55, 1);
> + *fault = ptr;
>
> /* If TBI is disabled, the access is unchecked, and ptr is not dirty. */
> if (unlikely(!tbi_check(desc, bit55))) {
> - return ptr;
> + return -1;
> }
>
> ptr_tag = allocation_tag_from_addr(ptr);
>
> if (tcma_check(desc, bit55, ptr_tag)) {
> - goto done;
> + return 1;
> }
>
> mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
> type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
> - total = FIELD_EX32(desc, MTEDESC, TSIZE);
>
> /* Find the addr of the end of the access, and of the last element. */
> ptr_last = ptr + total - 1;
> @@ -803,7 +815,7 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> mem1 = allocation_tag_mem(env, mmu_idx, ptr, type, total,
> MMU_DATA_LOAD, tag_size, ra);
> if (!mem1) {
> - goto done;
> + return 1;
> }
> /* Perform all of the comparisons. */
> n = checkN(mem1, ptr & TAG_GRANULE, ptr_tag, tag_count);
> @@ -829,23 +841,39 @@ uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> }
> if (n == c) {
> if (!mem2) {
> - goto done;
> + return 1;
> }
> n += checkN(mem2, 0, ptr_tag, tag_count - c);
> }
> }
>
> + if (likely(n == tag_count)) {
> + return 1;
> + }
> +
> /*
> * If we failed, we know which granule. For the first granule, the
> * failure address is @ptr, the first byte accessed. Otherwise the
> * failure address is the first byte of the nth granule.
> */
> - if (unlikely(n < tag_count)) {
> - uint64_t fault = (n == 0 ? ptr : tag_first + n * TAG_GRANULE);
> - mte_check_fail(env, desc, fault, ra);
> + if (n > 0) {
> + *fault = tag_first + n * TAG_GRANULE;
> }
> + return 0;
> +}
>
> - done:
> +uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
> + uint64_t ptr, uintptr_t ra)
> +{
> + uint64_t fault;
> + uint32_t total = FIELD_EX32(desc, MTEDESC, TSIZE);
> + int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
> +
> + if (unlikely(ret == 0)) {
> + mte_check_fail(env, desc, fault, ra);
> + } else if (ret < 0) {
> + return ptr;
> + }
> return useronly_clean_ptr(ptr);
> }
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1
2021-04-06 17:40 ` [PATCH v4 05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1 Richard Henderson
@ 2021-04-08 9:05 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 9:05 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> We were incorrectly assuming that only the first byte of an MTE access
> is checked against the tags. But per the ARM, unaligned accesses are
> pre-decomposed into single-byte accesses. So by the time we reach the
> actual MTE check in the ARM pseudocode, all accesses are aligned.
>
> We cannot tell a priori whether or not a given scalar access is aligned,
> therefore we must at least check. Use mte_probe_int, which is already
> set up for checking multiple granules.
>
> Buglink: https://bugs.launchpad.net/bugs/1921948
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
(tested with hand crafted kunit test)
> ---
> target/arm/mte_helper.c | 109 +++++++++++++---------------------------
> 1 file changed, 35 insertions(+), 74 deletions(-)
>
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 144bfa4a51..619c4b9351 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -617,80 +617,6 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
> }
> }
>
> -/*
> - * Perform an MTE checked access for a single logical or atomic access.
> - */
> -static bool mte_probe1_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
> - uintptr_t ra, int bit55)
> -{
> - int mem_tag, mmu_idx, ptr_tag, size;
> - MMUAccessType type;
> - uint8_t *mem;
> -
> - ptr_tag = allocation_tag_from_addr(ptr);
> -
> - if (tcma_check(desc, bit55, ptr_tag)) {
> - return true;
> - }
> -
> - mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
> - type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
> - size = FIELD_EX32(desc, MTEDESC, ESIZE);
> -
> - mem = allocation_tag_mem(env, mmu_idx, ptr, type, size,
> - MMU_DATA_LOAD, 1, ra);
> - if (!mem) {
> - return true;
> - }
> -
> - mem_tag = load_tag1(ptr, mem);
> - return ptr_tag == mem_tag;
> -}
> -
> -/*
> - * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
> - * Returns false if the access is Checked and the check failed. This
> - * is only intended to probe the tag -- the validity of the page must
> - * be checked beforehand.
> - */
> -bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
> -{
> - int bit55 = extract64(ptr, 55, 1);
> -
> - /* If TBI is disabled, the access is unchecked. */
> - if (unlikely(!tbi_check(desc, bit55))) {
> - return true;
> - }
> -
> - return mte_probe1_int(env, desc, ptr, 0, bit55);
> -}
> -
> -uint64_t mte_check1(CPUARMState *env, uint32_t desc,
> - uint64_t ptr, uintptr_t ra)
> -{
> - int bit55 = extract64(ptr, 55, 1);
> -
> - /* If TBI is disabled, the access is unchecked, and ptr is not dirty. */
> - if (unlikely(!tbi_check(desc, bit55))) {
> - return ptr;
> - }
> -
> - if (unlikely(!mte_probe1_int(env, desc, ptr, ra, bit55))) {
> - mte_check_fail(env, desc, ptr, ra);
> - }
> -
> - return useronly_clean_ptr(ptr);
> -}
> -
> -uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
> -{
> - return mte_check1(env, desc, ptr, GETPC());
> -}
> -
> -/*
> - * Perform an MTE checked access for multiple logical accesses.
> - */
> -
> /**
> * checkN:
> * @tag: tag memory to test
> @@ -882,6 +808,41 @@ uint64_t HELPER(mte_checkN)(CPUARMState *env, uint32_t desc, uint64_t ptr)
> return mte_checkN(env, desc, ptr, GETPC());
> }
>
> +uint64_t mte_check1(CPUARMState *env, uint32_t desc,
> + uint64_t ptr, uintptr_t ra)
> +{
> + uint64_t fault;
> + uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
> + int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
> +
> + if (unlikely(ret == 0)) {
> + mte_check_fail(env, desc, fault, ra);
> + } else if (ret < 0) {
> + return ptr;
> + }
> + return useronly_clean_ptr(ptr);
> +}
> +
> +uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
> +{
> + return mte_check1(env, desc, ptr, GETPC());
> +}
> +
> +/*
> + * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
> + * Returns false if the access is Checked and the check failed. This
> + * is only intended to probe the tag -- the validity of the page must
> + * be checked beforehand.
> + */
> +bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
> +{
> + uint64_t fault;
> + uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
> + int ret = mte_probe_int(env, desc, ptr, 0, total, &fault);
> +
> + return ret != 0;
> +}
> +
> /*
> * Perform an MTE checked access for DC_ZVA.
> */
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 06/12] test/tcg/aarch64: Add mte-5
2021-04-06 17:40 ` [PATCH v4 06/12] test/tcg/aarch64: Add mte-5 Richard Henderson
@ 2021-04-08 9:07 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 9:07 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Buglink: https://bugs.launchpad.net/bugs/1921948
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-08 8:50 ` Peter Maydell
2021-04-08 8:50 ` [Bug 1921948] " Peter Maydell
@ 2021-04-08 10:02 ` Alex Bennée
2021-04-08 10:02 ` [Bug 1921948] " Alex Bennée
1 sibling, 1 reply; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 10:02 UTC (permalink / raw)
To: Peter Maydell; +Cc: 1921948, qemu-arm, Richard Henderson, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On Wed, 7 Apr 2021 at 19:54, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>> > We were incorrectly assuming that only the first byte of an MTE access
>> > is checked against the tags. But per the ARM, unaligned accesses are
>> > pre-decomposed into single-byte accesses. So by the time we reach the
>> > actual MTE check in the ARM pseudocode, all accesses are aligned.
>> >
>> > Therefore, the first failure is always either the first byte of the
>> > access, or the first byte of the granule.
>> >
>> > In addition, some of the arithmetic is off for last-first -> count.
>> > This does not become directly visible until a later patch that passes
>> > single bytes into this function, so ptr == ptr_last.
>> >
>> > Buglink: https://bugs.launchpad.net/bugs/1921948
>>
>> Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
>> automatically copy patches to the appropriate bugs which is useful if
>> you don't have the Cc for the reporter.
>
> I'm not sure cc'ing bugs on patches is very useful, though (and especially
> not for big series). I usually prefer to just add a note to the bug with
> the URL of the series in patchew afterwards.
Ideally the tooling would pick up bug links in Patchew and automatically
update the relevant bugs when new series are posted. I only mentioned it
because the original bug reporters weren't Cc'ed on the patches and lo
now they know about the iteration they have tested it ;-)
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: [PATCH v4 03/12] target/arm: Fix mte_checkN
2021-04-08 10:02 ` Alex Bennée
@ 2021-04-08 10:02 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 10:02 UTC (permalink / raw)
To: qemu-devel
Peter Maydell <peter.maydell@linaro.org> writes:
> On Wed, 7 Apr 2021 at 19:54, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>> > We were incorrectly assuming that only the first byte of an MTE access
>> > is checked against the tags. But per the ARM, unaligned accesses are
>> > pre-decomposed into single-byte accesses. So by the time we reach the
>> > actual MTE check in the ARM pseudocode, all accesses are aligned.
>> >
>> > Therefore, the first failure is always either the first byte of the
>> > access, or the first byte of the granule.
>> >
>> > In addition, some of the arithmetic is off for last-first -> count.
>> > This does not become directly visible until a later patch that passes
>> > single bytes into this function, so ptr == ptr_last.
>> >
>> > Buglink: https://bugs.launchpad.net/bugs/1921948
>>
>> Minor note: you can Cc: Bug 1921948 <1921948@bugs.launchpad.net> to
>> automatically copy patches to the appropriate bugs which is useful if
>> you don't have the Cc for the reporter.
>
> I'm not sure cc'ing bugs on patches is very useful, though (and especially
> not for big series). I usually prefer to just add a note to the bug with
> the URL of the series in patchew afterwards.
Ideally the tooling would pick up bug links in Patchew and automatically
update the relevant bugs when new series are posted. I only mentioned it
because the original bug reporters weren't Cc'ed on the patches and lo
now they know about the iteration they have tested it ;-)
--
Alex Bennée
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
In Progress
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 07/12] target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
2021-04-06 17:40 ` [PATCH v4 07/12] target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1 Richard Henderson
@ 2021-04-08 11:08 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 11:08 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> After recent changes, mte_checkN does not use ESIZE,
> and mte_check1 never used TSIZE. We can combine the
> two into a single field: SIZEM1.
>
> Choose to pass size - 1 because size == 0 is never used,
> our immediate need in mte_probe_int is for the address
> of the last byte (ptr + size - 1), and since almost all
> operations are powers of 2, this makes the immediate
> constant one bit smaller.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 08/12] target/arm: Merge mte_check1, mte_checkN
2021-04-06 17:40 ` [PATCH v4 08/12] target/arm: Merge mte_check1, mte_checkN Richard Henderson
@ 2021-04-08 11:10 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 11:10 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> The mte_check1 and mte_checkN functions are now identical.
> Drop mte_check1 and rename mte_checkN to mte_check.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 09/12] target/arm: Rename mte_probe1 to mte_probe
2021-04-06 17:40 ` [PATCH v4 09/12] target/arm: Rename mte_probe1 to mte_probe Richard Henderson
@ 2021-04-08 11:10 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 11:10 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> For consistency with the mte_check1 + mte_checkN merge
> to mte_check, rename the probe function as well.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 10/12] target/arm: Simplify sve mte checking
2021-04-06 17:40 ` [PATCH v4 10/12] target/arm: Simplify sve mte checking Richard Henderson
@ 2021-04-08 11:23 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 11:23 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Now that mte_check1 and mte_checkN have been merged, we can
> merge sve_cont_ldst_mte_check1 and sve_cont_ldst_mte_checkN.
>
> Which means that we can eliminate the function pointer into
> sve_ldN_r and sve_stN_r, calling sve_cont_ldst_mte_check directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 11/12] target/arm: Remove log2_esize parameter to gen_mte_checkN
2021-04-06 17:40 ` [PATCH v4 11/12] target/arm: Remove log2_esize parameter to gen_mte_checkN Richard Henderson
@ 2021-04-08 11:35 ` Alex Bennée
0 siblings, 0 replies; 63+ messages in thread
From: Alex Bennée @ 2021-04-08 11:35 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> The log2_esize parameter is not used except trivially.
> Drop the parameter and the deferral to gen_mte_check1.
>
> This fixes a bug in that the parameters as documented
> in the header file were the reverse from those in the
> implementation. Which meant that translate-sve.c was
> passing the parameters in the wrong order.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 00/12] target/arm mte fixes
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
` (12 preceding siblings ...)
2021-04-06 17:57 ` [PATCH v4 00/12] target/arm mte fixes no-reply
@ 2021-04-08 12:47 ` Peter Maydell
2021-04-08 14:25 ` Richard Henderson
13 siblings, 1 reply; 63+ messages in thread
From: Peter Maydell @ 2021-04-08 12:47 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Tue, 6 Apr 2021 at 18:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Changes for v4:
> * Fix tag count computation error in mte_checkN, which when used
> by mte_check1 in patch 5, caused all sorts of KASAN failures.
> * Fix PAGE_ANON / PAGE_TARGET_1 overlap.
>
>
> r~
>
>
> Richard Henderson (12):
> accel/tcg: Preserve PAGE_ANON when changing page permissions
> target/arm: Check PAGE_WRITE_ORG for MTE writeability
> target/arm: Fix mte_checkN
> target/arm: Split out mte_probe_int
> target/arm: Fix unaligned checks for mte_check1, mte_probe1
> test/tcg/aarch64: Add mte-5
> target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
> target/arm: Merge mte_check1, mte_checkN
> target/arm: Rename mte_probe1 to mte_probe
> target/arm: Simplify sve mte checking
> target/arm: Remove log2_esize parameter to gen_mte_checkN
> exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
So, what should we do with this series for 6.0 ? We'll be at rc3
next week, and this big a change seems risky at this point
in the cycle. Is there anything in here that's worth extracting
as a for-6.0 change? (maybe patches 1, 2, 12?)
thanks
-- PMM
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 00/12] target/arm mte fixes
2021-04-08 12:47 ` Peter Maydell
@ 2021-04-08 14:25 ` Richard Henderson
2021-04-09 9:53 ` Peter Maydell
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-08 14:25 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/8/21 5:47 AM, Peter Maydell wrote:
>> Richard Henderson (12):
>> accel/tcg: Preserve PAGE_ANON when changing page permissions
>> target/arm: Check PAGE_WRITE_ORG for MTE writeability
>> target/arm: Fix mte_checkN
>> target/arm: Split out mte_probe_int
>> target/arm: Fix unaligned checks for mte_check1, mte_probe1
>> test/tcg/aarch64: Add mte-5
>> target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
>> target/arm: Merge mte_check1, mte_checkN
>> target/arm: Rename mte_probe1 to mte_probe
>> target/arm: Simplify sve mte checking
>> target/arm: Remove log2_esize parameter to gen_mte_checkN
>> exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
>
> So, what should we do with this series for 6.0 ? We'll be at rc3
> next week, and this big a change seems risky at this point
> in the cycle. Is there anything in here that's worth extracting
> as a for-6.0 change? (maybe patches 1, 2, 12?)
Definitely 12, since that broke BTI.
Patches 1 and 2 are certainly simple enough.
Otherwise... the rest would be nice to have. It's quite isolated to mte=on.
If you defer, I guess that's fine too -- the bug report did come in quite late.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v4 00/12] target/arm mte fixes
2021-04-08 14:25 ` Richard Henderson
@ 2021-04-09 9:53 ` Peter Maydell
0 siblings, 0 replies; 63+ messages in thread
From: Peter Maydell @ 2021-04-09 9:53 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Thu, 8 Apr 2021 at 15:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/8/21 5:47 AM, Peter Maydell wrote:
> >> Richard Henderson (12):
> >> accel/tcg: Preserve PAGE_ANON when changing page permissions
> >> target/arm: Check PAGE_WRITE_ORG for MTE writeability
> >> target/arm: Fix mte_checkN
> >> target/arm: Split out mte_probe_int
> >> target/arm: Fix unaligned checks for mte_check1, mte_probe1
> >> test/tcg/aarch64: Add mte-5
> >> target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1
> >> target/arm: Merge mte_check1, mte_checkN
> >> target/arm: Rename mte_probe1 to mte_probe
> >> target/arm: Simplify sve mte checking
> >> target/arm: Remove log2_esize parameter to gen_mte_checkN
> >> exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
> >
> > So, what should we do with this series for 6.0 ? We'll be at rc3
> > next week, and this big a change seems risky at this point
> > in the cycle. Is there anything in here that's worth extracting
> > as a for-6.0 change? (maybe patches 1, 2, 12?)
>
> Definitely 12, since that broke BTI.
>
> Patches 1 and 2 are certainly simple enough.
>
> Otherwise... the rest would be nice to have. It's quite isolated to mte=on.
> If you defer, I guess that's fine too -- the bug report did come in quite late.
OK, I'm going to take 1, 2 and 12 into target-arm.next for 6.0, and we'll
defer the rest. AIUI the bug fixed by the remaining patches is basically
"we didn't do some tag checks we should have done in a corner case of
misaligned accesses that span two differently tagged regions", and I think
that's not a terrible bug to leave in 6.0, compared to the risk of breaking
MTE more seriously while fixing it.
thanks
-- PMM
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (11 preceding siblings ...)
2021-04-07 22:02 ` Andrey Konovalov
@ 2021-05-06 18:39 ` Richard Henderson
2021-05-22 5:12 ` Peter Collingbourne
` (4 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-05-06 18:39 UTC (permalink / raw)
To: qemu-devel
** Changed in: qemu
Status: In Progress => Fix Committed
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
Fix Committed
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (12 preceding siblings ...)
2021-05-06 18:39 ` Richard Henderson
@ 2021-05-22 5:12 ` Peter Collingbourne
2021-05-22 5:17 ` Peter Collingbourne
` (3 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Peter Collingbourne @ 2021-05-22 5:12 UTC (permalink / raw)
To: qemu-devel
It looks like there's still a bug here: I'm seeing false positive MTE
faults for unaligned accesses that touch multiple pages. This userspace
assembly program demonstrates the problem, but for some reason it only
reproduces some of the time for me:
.arch_extension memtag
.globl _start
_start:
mov x0, #0x37 // PR_SET_TAGGED_ADDR_CTRL
mov x1, #0x3 // PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_ASYNC
mov x2, #0
mov x3, #0
mov x4, #0
mov x8, #0xa7 // prctl
svc #0
mov x0, xzr
mov w1, #0x2000
mov w2, #0x23 // PROT_READ|PROT_WRITE|PROT_MTE
mov w3, #0x22 // MAP_PRIVATE|MAP_ANONYMOUS
mov w4, #0xffffffff
mov x5, xzr
mov x8, #0xde // mmap
svc #0
mov x1, #(1 << 56)
add x0, x0, x1
add x0, x0, #0xff0
stg x0, [x0]
stg x0, [x0, #16]
str x1, [x0, #12]
mov x0, #0
mov x8, #0x5d // exit
svc #0
** Changed in: qemu
Status: Fix Committed => Confirmed
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
Confirmed
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (13 preceding siblings ...)
2021-05-22 5:12 ` Peter Collingbourne
@ 2021-05-22 5:17 ` Peter Collingbourne
2021-05-26 19:55 ` Vitaly Buka
` (2 subsequent siblings)
17 siblings, 0 replies; 63+ messages in thread
From: Peter Collingbourne @ 2021-05-22 5:17 UTC (permalink / raw)
To: qemu-devel
(s/PR_MTE_TCF_ASYNC/PR_MTE_TCF_SYNC/g in the above program -- but the
actual constant is correct)
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
Confirmed
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (14 preceding siblings ...)
2021-05-22 5:17 ` Peter Collingbourne
@ 2021-05-26 19:55 ` Vitaly Buka
2021-06-10 2:28 ` Peter Collingbourne
2021-06-10 6:06 ` Thomas Huth
17 siblings, 0 replies; 63+ messages in thread
From: Vitaly Buka @ 2021-05-26 19:55 UTC (permalink / raw)
To: qemu-devel
I see something similar in memset
It SEGV on
stur q0, [x4, #-16]
for x4 set to 0xd000055214fe008
and near tags are 0xd000055214fdff0 and 0xd000055214fe000
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
Confirmed
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (15 preceding siblings ...)
2021-05-26 19:55 ` Vitaly Buka
@ 2021-06-10 2:28 ` Peter Collingbourne
2021-06-10 6:06 ` Thomas Huth
17 siblings, 0 replies; 63+ messages in thread
From: Peter Collingbourne @ 2021-06-10 2:28 UTC (permalink / raw)
To: qemu-devel
I happened to notice that you're moving your bug tracker to gitlab so I
refiled this issue over there: https://gitlab.com/qemu-
project/qemu/-/issues/403
** Bug watch added: gitlab.com/qemu-project/qemu/-/issues #403
https://gitlab.com/qemu-project/qemu/-/issues/403
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
Confirmed
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Bug 1921948] Re: MTE tags not checked properly for unaligned accesses at EL1
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
` (16 preceding siblings ...)
2021-06-10 2:28 ` Peter Collingbourne
@ 2021-06-10 6:06 ` Thomas Huth
17 siblings, 0 replies; 63+ messages in thread
From: Thomas Huth @ 2021-06-10 6:06 UTC (permalink / raw)
To: qemu-devel
Thanks for opening the new ticket. I'm closing this one here on
Launchpad now so that we don't accidentally migrate it later
automatically.
** Changed in: qemu
Status: Confirmed => Fix Released
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1921948
Title:
MTE tags not checked properly for unaligned accesses at EL1
Status in QEMU:
Fix Released
Bug description:
For kernel memory accesses that span across two memory granules,
QEMU's MTE implementation only checks the tag of the first granule but
not of the second one.
To reproduce this, build the Linux kernel with CONFIG_KASAN_HW_TAGS
enabled, apply the patch below, and boot the kernel:
diff --git a/sound/last.c b/sound/last.c
index f0bb98780e70..04745cb30b74 100644
--- a/sound/last.c
+++ b/sound/last.c
@@ -5,12 +5,18 @@
*/
#include <linux/init.h>
+#include <linux/slab.h>
#include <sound/core.h>
static int __init alsa_sound_last_init(void)
{
struct snd_card *card;
int idx, ok = 0;
+
+ char *ptr = kmalloc(128, GFP_KERNEL);
+ pr_err("KASAN report should follow:\n");
+ *(volatile unsigned long *)(ptr + 124);
+ kfree(ptr);
printk(KERN_INFO "ALSA device list:\n");
for (idx = 0; idx < SNDRV_CARDS; idx++) {
KASAN tags the 128 allocated bytes with the same tag as the returned
pointer. The memory granule that follows the 128 allocated bytes has a
different tag (with 1/15 probability).
Expected result: a tag fault is detected and a KASAN report is printed when accessing bytes [124, 130).
Observed result: no tag fault is detected and no KASAN report is printed.
Here are the flags that I use to run QEMU if they matter:
qemu-system-aarch64 -s -machine virt,mte=on -cpu max -m 2G -smp 2 -net
user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 -net nic
-nographic -kernel ./Image -append "console=ttyAMA0 root=/dev/vda
earlyprintk=serial" -drive file=./fs.img,format=raw,if=virtio -no-
shutdown -no-reboot
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1921948/+subscriptions
^ permalink raw reply [flat|nested] 63+ messages in thread
end of thread, other threads:[~2021-06-10 6:22 UTC | newest]
Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06 17:40 [PATCH v4 00/12] target/arm mte fixes Richard Henderson
2021-04-06 17:40 ` [PATCH v4 01/12] accel/tcg: Preserve PAGE_ANON when changing page permissions Richard Henderson
2021-04-07 13:55 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 02/12] target/arm: Check PAGE_WRITE_ORG for MTE writeability Richard Henderson
2021-04-07 15:34 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 03/12] target/arm: Fix mte_checkN Richard Henderson
2021-04-07 18:39 ` Alex Bennée
2021-04-07 18:39 ` [Bug 1921948] " Alex Bennée
2021-04-07 19:56 ` Richard Henderson
2021-04-08 8:36 ` Alex Bennée
2021-04-08 8:36 ` [Bug 1921948] " Alex Bennée
2021-04-08 8:50 ` Peter Maydell
2021-04-08 8:50 ` [Bug 1921948] " Peter Maydell
2021-04-08 10:02 ` Alex Bennée
2021-04-08 10:02 ` [Bug 1921948] " Alex Bennée
2021-04-06 17:40 ` [PATCH v4 04/12] target/arm: Split out mte_probe_int Richard Henderson
2021-04-08 9:01 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1 Richard Henderson
2021-04-08 9:05 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 06/12] test/tcg/aarch64: Add mte-5 Richard Henderson
2021-04-08 9:07 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 07/12] target/arm: Replace MTEDESC ESIZE+TSIZE with SIZEM1 Richard Henderson
2021-04-08 11:08 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 08/12] target/arm: Merge mte_check1, mte_checkN Richard Henderson
2021-04-08 11:10 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 09/12] target/arm: Rename mte_probe1 to mte_probe Richard Henderson
2021-04-08 11:10 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 10/12] target/arm: Simplify sve mte checking Richard Henderson
2021-04-08 11:23 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 11/12] target/arm: Remove log2_esize parameter to gen_mte_checkN Richard Henderson
2021-04-08 11:35 ` Alex Bennée
2021-04-06 17:40 ` [PATCH v4 12/12] exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 Richard Henderson
2021-04-06 18:21 ` Laurent Vivier
2021-04-06 19:36 ` Laurent Vivier
2021-04-07 17:16 ` Alex Bennée
2021-04-07 21:33 ` Nathan Chancellor
2021-04-06 17:57 ` [PATCH v4 00/12] target/arm mte fixes no-reply
2021-04-08 12:47 ` Peter Maydell
2021-04-08 14:25 ` Richard Henderson
2021-04-09 9:53 ` Peter Maydell
-- strict thread matches above, loose matches on Subject: below --
2021-03-30 19:34 [Bug 1921948] [NEW] MTE tags not checked properly for unaligned accesses at EL1 Andrey Konovalov
2021-03-30 23:22 ` [Bug 1921948] " Richard Henderson
2021-03-30 23:32 ` Peter Collingbourne
2021-03-31 6:44 ` Richard Henderson
2021-04-02 15:41 ` Richard Henderson
2021-04-02 16:17 ` Andrey Konovalov
2021-04-02 16:31 ` Richard Henderson
2021-04-03 14:34 ` Andrey Konovalov
2021-04-07 20:17 ` Andrey Konovalov
2021-04-07 20:46 ` Alex Bennée
2021-04-07 20:58 ` Andrey Konovalov
2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:29 ` Alex Bennée
2021-04-07 21:45 ` Alex Bennée
2021-04-07 21:45 ` Alex Bennée
2021-04-07 21:19 ` Richard Henderson
2021-04-07 22:02 ` Andrey Konovalov
2021-05-06 18:39 ` Richard Henderson
2021-05-22 5:12 ` Peter Collingbourne
2021-05-22 5:17 ` Peter Collingbourne
2021-05-26 19:55 ` Vitaly Buka
2021-06-10 2:28 ` Peter Collingbourne
2021-06-10 6:06 ` Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).