| Message ID | 9cdc75c4-326f-5adb-b32c-c95e1f3211ce@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | ubsan: don't handle misaligned address when support unaligned access | expand |
From: Ding Tianhong > Sent: 01 December 2017 11:32 > To: akpm@linux-foundation.org; aryabinin@virtuozzo.co; linux-kernel@vger.kernel.org; LinuxArm > Subject: [PATCH] ubsan: don't handle misaligned address when support unaligned access > > The ubsan always report Warning just like: > > UBSAN: Undefined behaviour in ../include/linux/etherdevice.h:386:9 > load of misaligned address ffffffc069ba0482 for type 'long unsigned int' > which requires 8 byte alignment > CPU: 0 PID: 901 Comm: sshd Not tainted 4.xx+ #1 > Hardware name: linux,dummy-virt (DT) > Call trace: > [<ffffffc000093600>] dump_backtrace+0x0/0x348 > [<ffffffc000093968>] show_stack+0x20/0x30 > [<ffffffc001651664>] dump_stack+0x144/0x1b4 > [<ffffffc0016519b0>] ubsan_epilogue+0x18/0x74 > [<ffffffc001651bac>] __ubsan_handle_type_mismatch+0x1a0/0x25c > [<ffffffc00125d8a0>] dev_gro_receive+0x17d8/0x1830 > [<ffffffc00125d928>] napi_gro_receive+0x30/0x158 > [<ffffffc000f4f93c>] virtnet_receive+0xad4/0x1fa8 > > The reason is that when enable the CONFIG_UBSAN_ALIGNMENT, the ubsan > will report the unaligned access even if the system support it > (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y), it will produce a lot > of noise in the log and cause confusion. > > This patch will close the detection of unaligned access when > the system support unaligned access. > > Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> > --- > lib/ubsan.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/ubsan.c b/lib/ubsan.c > index fb0409d..278b4c3 100644 > --- a/lib/ubsan.c > +++ b/lib/ubsan.c > @@ -321,7 +321,8 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, > > if (!ptr) > handle_null_ptr_deref(data); > - else if (data->alignment && !IS_ALIGNED(ptr, data->alignment)) > + else if (data->alignment && !IS_ALIGNED(ptr, data->alignment) && > + !IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) > handle_missaligned_access(data, ptr); > else > handle_object_size_mismatch(data, ptr); Won't that report an object size error instead of actually doing the required access? Surely it shouldn't get into this function at all? I guess 'alignment' is set to 4 or 8. If it were set to 3 or 7 (or 0) then the tests on the pointer would be much simpler - maybe at a slight extra cost in setup. David
On 2017/12/1 19:47, David Laight wrote: >> of noise in the log and cause confusion. >> >> This patch will close the detection of unaligned access when >> the system support unaligned access. >> >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >> --- >> lib/ubsan.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/lib/ubsan.c b/lib/ubsan.c >> index fb0409d..278b4c3 100644 >> --- a/lib/ubsan.c >> +++ b/lib/ubsan.c >> @@ -321,7 +321,8 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, >> >> if (!ptr) >> handle_null_ptr_deref(data); >> - else if (data->alignment && !IS_ALIGNED(ptr, data->alignment)) >> + else if (data->alignment && !IS_ALIGNED(ptr, data->alignment) && >> + !IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) >> handle_missaligned_access(data, ptr); >> else >> handle_object_size_mismatch(data, ptr); > > Won't that report an object size error instead of actually > doing the required access? > Yes，I miss it. > Surely it shouldn't get into this function at all? > > I guess 'alignment' is set to 4 or 8. > If it were set to 3 or 7 (or 0) then the tests on the pointer > would be much simpler - maybe at a slight extra cost in setup. > Looks like we need to fix it in the handle_missaligned_access: diff --git a/lib/ubsan.c b/lib/ubsan.c index 278b4c3..040f8b2 100644 --- a/lib/ubsan.c +++ b/lib/ubsan.c @@ -289,6 +289,9 @@ static void handle_missaligned_access(struct type_mismatch_data *data, if (suppress_report(&data->location)) return; + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) + return; + ubsan_prologue(&data->location, &flags); pr_err("%s misaligned address %p for type %s\n", Thanks Ding > David >
diff --git a/lib/ubsan.c b/lib/ubsan.c index fb0409d..278b4c3 100644 --- a/lib/ubsan.c +++ b/lib/ubsan.c @@ -321,7 +321,8 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, if (!ptr) handle_null_ptr_deref(data); - else if (data->alignment && !IS_ALIGNED(ptr, data->alignment)) + else if (data->alignment && !IS_ALIGNED(ptr, data->alignment) && + !IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) handle_missaligned_access(data, ptr); else handle_object_size_mismatch(data, ptr);
The ubsan always report Warning just like: UBSAN: Undefined behaviour in ../include/linux/etherdevice.h:386:9 load of misaligned address ffffffc069ba0482 for type 'long unsigned int' which requires 8 byte alignment CPU: 0 PID: 901 Comm: sshd Not tainted 4.xx+ #1 Hardware name: linux,dummy-virt (DT) Call trace: [<ffffffc000093600>] dump_backtrace+0x0/0x348 [<ffffffc000093968>] show_stack+0x20/0x30 [<ffffffc001651664>] dump_stack+0x144/0x1b4 [<ffffffc0016519b0>] ubsan_epilogue+0x18/0x74 [<ffffffc001651bac>] __ubsan_handle_type_mismatch+0x1a0/0x25c [<ffffffc00125d8a0>] dev_gro_receive+0x17d8/0x1830 [<ffffffc00125d928>] napi_gro_receive+0x30/0x158 [<ffffffc000f4f93c>] virtnet_receive+0xad4/0x1fa8 The reason is that when enable the CONFIG_UBSAN_ALIGNMENT, the ubsan will report the unaligned access even if the system support it (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y), it will produce a lot of noise in the log and cause confusion. This patch will close the detection of unaligned access when the system support unaligned access. Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> --- lib/ubsan.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 1.8.3.1