| Message ID | 20240513122754.1282833-1-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arch/x86/um: Disable UBSAN sanitization | expand |
On Mon, 2024-05-13 at 14:27 +0200, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > Disable UBSAN sanitization on UML, since UML does not support it. > Luckily, that isn't actually true, nor does it actually do this at all. Please fix the commit message. johannes
On Mon, 2024-05-13 at 14:29 +0200, Johannes Berg wrote: > On Mon, 2024-05-13 at 14:27 +0200, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > Disable UBSAN sanitization on UML, since UML does not support it. > > > > Luckily, that isn't actually true, nor does it actually do this at all. > Please fix the commit message. Thanks, I was actually wondering. I based that statement based on ARCH_HAS_UBSAN=n. Any other solution would be ok. Thanks Roberto
On Mon, 2024-05-13 at 14:42 +0200, Roberto Sassu wrote: > On Mon, 2024-05-13 at 14:29 +0200, Johannes Berg wrote: > > On Mon, 2024-05-13 at 14:27 +0200, Roberto Sassu wrote: > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > Disable UBSAN sanitization on UML, since UML does not support it. > > > > > > > Luckily, that isn't actually true, nor does it actually do this at all. > > Please fix the commit message. > > Thanks, I was actually wondering. I based that statement based on > ARCH_HAS_UBSAN=n. > > Any other solution would be ok. Not sure I get it. What you're doing in the patch is perfectly fine and almost certainly required, but you're definitely not disabling UBSAN on ARCH=um as you described in the commit message? johannes
On Mon, 2024-05-13 at 14:52 +0200, Johannes Berg wrote: > On Mon, 2024-05-13 at 14:42 +0200, Roberto Sassu wrote: > > On Mon, 2024-05-13 at 14:29 +0200, Johannes Berg wrote: > > > On Mon, 2024-05-13 at 14:27 +0200, Roberto Sassu wrote: > > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > > Disable UBSAN sanitization on UML, since UML does not support it. > > > > > > > > > > Luckily, that isn't actually true, nor does it actually do this at all. > > > Please fix the commit message. > > > > Thanks, I was actually wondering. I based that statement based on > > ARCH_HAS_UBSAN=n. > > > > Any other solution would be ok. > > Not sure I get it. What you're doing in the patch is perfectly fine and > almost certainly required, but you're definitely not disabling UBSAN on > ARCH=um as you described in the commit message? Ok, I guess the right word is instrumentation (got it from commit d4be85d068b44). And the reason is that the vDSO is executing in user space. Will fix it. Thanks Roberto
On Mon, 2024-05-13 at 14:58 +0200, Roberto Sassu wrote: > On Mon, 2024-05-13 at 14:52 +0200, Johannes Berg wrote: > > On Mon, 2024-05-13 at 14:42 +0200, Roberto Sassu wrote: > > > On Mon, 2024-05-13 at 14:29 +0200, Johannes Berg wrote: > > > > On Mon, 2024-05-13 at 14:27 +0200, Roberto Sassu wrote: > > > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > > > > Disable UBSAN sanitization on UML, since UML does not support it. > > > > > > > > > > > > > Luckily, that isn't actually true, nor does it actually do this at all. > > > > Please fix the commit message. > > > > > > Thanks, I was actually wondering. I based that statement based on > > > ARCH_HAS_UBSAN=n. > > > > > > Any other solution would be ok. > > > > Not sure I get it. What you're doing in the patch is perfectly fine and > > almost certainly required, but you're definitely not disabling UBSAN on > > ARCH=um as you described in the commit message? > > Ok, I guess the right word is instrumentation (got it from commit > d4be85d068b44). And the reason is that the vDSO is executing in user > space. Will fix it. No, UBSAN is fine, but you're only disabling it for the vDSO :) The commit message doesn't even mention the vDSO though. johannes
On Mon, 2024-05-13 at 15:08 +0200, Johannes Berg wrote: > On Mon, 2024-05-13 at 14:58 +0200, Roberto Sassu wrote: > > On Mon, 2024-05-13 at 14:52 +0200, Johannes Berg wrote: > > > On Mon, 2024-05-13 at 14:42 +0200, Roberto Sassu wrote: > > > > On Mon, 2024-05-13 at 14:29 +0200, Johannes Berg wrote: > > > > > On Mon, 2024-05-13 at 14:27 +0200, Roberto Sassu wrote: > > > > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > > > > > > Disable UBSAN sanitization on UML, since UML does not support it. > > > > > > > > > > > > > > > > Luckily, that isn't actually true, nor does it actually do this at all. > > > > > Please fix the commit message. > > > > > > > > Thanks, I was actually wondering. I based that statement based on > > > > ARCH_HAS_UBSAN=n. > > > > > > > > Any other solution would be ok. > > > > > > Not sure I get it. What you're doing in the patch is perfectly fine and > > > almost certainly required, but you're definitely not disabling UBSAN on > > > ARCH=um as you described in the commit message? > > > > Ok, I guess the right word is instrumentation (got it from commit > > d4be85d068b44). And the reason is that the vDSO is executing in user > > space. Will fix it. > > No, UBSAN is fine, but you're only disabling it for the vDSO :) The > commit message doesn't even mention the vDSO though. You are right, the commit message was misleading without vDSO. Thanks Roberto
diff --git a/arch/x86/um/vdso/Makefile b/arch/x86/um/vdso/Makefile index b86d634730b2..ca79c0de582e 100644 --- a/arch/x86/um/vdso/Makefile +++ b/arch/x86/um/vdso/Makefile @@ -3,8 +3,10 @@ # Building vDSO images for x86. # -# do not instrument on vdso because KASAN is not compatible with user mode +# do not instrument on vdso because KASAN/UBSAN are not compatible with user +# mode KASAN_SANITIZE := n +UBSAN_SANITIZE := n # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in. KCOV_INSTRUMENT := n