All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -rc 0/4] Protect from GCC garbage input in GCOV
@ 2020-09-02  8:55 Leon Romanovsky
  2020-09-02  8:55 ` [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers Leon Romanovsky
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02  8:55 UTC (permalink / raw)
  To: Linus Torvalds, Peter Oberparleiter
  Cc: Leon Romanovsky, linux-kernel, Colin Ian King, Andrew Morton

From: Leon Romanovsky <leonro@nvidia.com>

Hi Linus,

Both Colin in Ubuntu [1] and I in FC 32 are having same kernel crashes
while GCOV is enabled. The reason to it that n_fuction variable that
should be provided by GCC is not initialized (or wrongly set).

This patch is based on the RFC [2] which I sent to gather feedback, but
didn't get any response, so sending it to you in proper -rc format.

Bottom line, GCOV is broken on GCC 10.2.

Thanks

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1891288
[2] https://lore.kernel.org/lkml/20200827133932.3338519-1-leon@kernel.org

Leon Romanovsky (4):
  gcov: Open-code kmemdup() to work correctly with kernel and user space
    pointers
  gcov: Use proper duplication routine for const pointer
  gcov: Protect from uninitialized number of functions provided by GCC
    10.2
  gcov: Don't print out-of-memory print for all failed files

 kernel/gcov/fs.c      |  5 +++--
 kernel/gcov/gcc_4_7.c | 17 +++++++++--------
 2 files changed, 12 insertions(+), 10 deletions(-)

--
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers
  2020-09-02  8:55 [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Leon Romanovsky
@ 2020-09-02  8:55 ` Leon Romanovsky
  2020-09-02 17:38   ` Linus Torvalds
  2020-09-02  8:55 ` [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer Leon Romanovsky
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02  8:55 UTC (permalink / raw)
  To: Linus Torvalds, Peter Oberparleiter
  Cc: Leon Romanovsky, linux-kernel, Colin Ian King, Andrew Morton

From: Leon Romanovsky <leonro@nvidia.com>

The kernel with KASAN and GCOV enabled generates the following splat
due to the situation that gcov_info can be both user and kernel pointer.

It is triggered by the memcpy() inside kmemdup(), so as a possible solution
let's copy fields manually.

 ==================================================================
 BUG: KASAN: global-out-of-bounds in kmemdup+0x43/0x70
 Read of size 120 at addr ffffffffa0d2c780 by task modprobe/296

 CPU: 0 PID: 296 Comm: modprobe Not tainted 5.9.0-rc1+ #1860
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04 /01/2014
 Call Trace:
   dump_stack+0x128/0x1af
   print_address_description.constprop.0+0x2c/0x3f0
   _raw_spin_lock_irqsave+0x34/0xa0
   __kasan_check_read+0x1d/0x30
   kmemdup+0x43/0x70
   kmemdup+0x43/0x70
   gcov_info_dup+0x2d/0x730
   __kasan_check_write+0x20/0x30
   __mutex_unlock_slowpath+0x10d/0x740
   gcov_event+0x88d/0xd30
   gcov_module_notifier+0xe9/0x100
   notifier_call_chain+0xeb/0x170
   blocking_notifier_call_chain+0x75/0xc0
   __x64_sys_delete_module+0x326/0x5a0
   do_init_module+0x810/0x810
   syscall_enter_from_user_mode+0x40/0x420
   trace_hardirqs_on+0x45/0xb0
   syscall_enter_from_user_mode+0x40/0x420
   do_syscall_64+0x45/0x70
   entry_SYSCALL_64_after_hwframe+0x44/0xa9

 The buggy address belongs to the variable:
  __gcov_.uverbs_attr_get_obj+0x60/0xfffffffffff778e0 [mlx5_ib]

 Memory state around the buggy address:
  ffffffffa0d2c680: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 f9 f9 f9
  ffffffffa0d2c700: f9 f9 f9 f9 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9
 >ffffffffa0d2c780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9
                                                              ^
  ffffffffa0d2c800: f9 f9 f9 f9 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9
  ffffffffa0d2c880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ==================================================================
 Disabling lock debugging due to kernel taint
 ---[ end trace 065ea9cc2ba144a6 ]---

Cc: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 kernel/gcov/gcc_4_7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
index 908fdf5098c3..6d706c5eed5c 100644
--- a/kernel/gcov/gcc_4_7.c
+++ b/kernel/gcov/gcc_4_7.c
@@ -275,13 +275,13 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)
 	size_t fi_size; /* function info size */
 	size_t cv_size; /* counter values size */

-	dup = kmemdup(info, sizeof(*dup), GFP_KERNEL);
+	dup = kzalloc(sizeof(*dup), GFP_KERNEL);
 	if (!dup)
 		return NULL;

-	dup->next = NULL;
-	dup->filename = NULL;
-	dup->functions = NULL;
+	for (fi_idx = 0; fi_idx < GCOV_COUNTERS; fi_idx++)
+		dup->merge[fi_idx] = info->merge[fi_idx];
+	dup->n_functions = info->n_functions;

 	dup->filename = kstrdup(info->filename, GFP_KERNEL);
 	if (!dup->filename)
--
2.26.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer
  2020-09-02  8:55 [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Leon Romanovsky
  2020-09-02  8:55 ` [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers Leon Romanovsky
@ 2020-09-02  8:55 ` Leon Romanovsky
  2020-09-03  8:56   ` Rasmus Villemoes
  2020-09-02  8:55 ` [PATCH rdma-next 3/4] gcov: Protect from uninitialized number of functions provided by GCC 10.2 Leon Romanovsky
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02  8:55 UTC (permalink / raw)
  To: Linus Torvalds, Peter Oberparleiter
  Cc: Leon Romanovsky, linux-kernel, Colin Ian King, Andrew Morton

From: Leon Romanovsky <leonro@nvidia.com>

The filename is a const pointer, so use the proper string duplication
routine that takes into account const identifier.

Cc: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 kernel/gcov/gcc_4_7.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
index 6d706c5eed5c..318211deb903 100644
--- a/kernel/gcov/gcc_4_7.c
+++ b/kernel/gcov/gcc_4_7.c
@@ -283,7 +283,7 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)
 		dup->merge[fi_idx] = info->merge[fi_idx];
 	dup->n_functions = info->n_functions;

-	dup->filename = kstrdup(info->filename, GFP_KERNEL);
+	dup->filename = kstrdup_const(info->filename, GFP_KERNEL);
 	if (!dup->filename)
 		goto err_free;

@@ -359,7 +359,7 @@ void gcov_info_free(struct gcov_info *info)

 free_info:
 	kfree(info->functions);
-	kfree(info->filename);
+	kfree_const(info->filename);
 	kfree(info);
 }

--
2.26.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH rdma-next 3/4] gcov: Protect from uninitialized number of functions provided by GCC 10.2
  2020-09-02  8:55 [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Leon Romanovsky
  2020-09-02  8:55 ` [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers Leon Romanovsky
  2020-09-02  8:55 ` [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer Leon Romanovsky
@ 2020-09-02  8:55 ` Leon Romanovsky
  2020-09-02 17:41   ` Linus Torvalds
  2020-09-02  8:55 ` [PATCH rdma-next 4/4] gcov: Don't print out-of-memory print for all failed files Leon Romanovsky
  2020-09-02 17:42 ` [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Linus Torvalds
  4 siblings, 1 reply; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02  8:55 UTC (permalink / raw)
  To: Linus Torvalds, Peter Oberparleiter
  Cc: Leon Romanovsky, linux-kernel, Colin Ian King, Andrew Morton

From: Leon Romanovsky <leonro@nvidia.com>

The kernel compiled with GCC 10.2.1 and KASAN together with GCOV enabled
produces the following splat while reloading modules. The very similar
trace was reported by Colin [1].

 ------------[ cut here ]------------
 WARNING: CPU: 0 PID: 296 at mm/page_alloc.c:4859 __alloc_pages_nodemask+0x670/0x3190
 Modules linked in: mlx5_ib(-) mlx5_core mlxfw ptp ib_ipoib pps_core rdma_ucm rdma_cm iw_cm ib_cm ib_umad  ib_uverbs ib_core
 CPU: 0 PID: 296 Comm: modprobe Tainted: G    B             5.9.0-rc1+ #1860
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04 /01/2014
 RIP: 0010:__alloc_pages_nodemask+0x670/0x3190
 Code: e9 af fc ff ff 48 83 05 fd 28 90 05 01 81 e7 00 20 00 00 48 c7 44 24 28 00 00 00 00 0f 85 fb fd ff  ff 48 83 05 f0 28 90 05 01 <0f> 0b 48 83 05 ee 28 90 05 01 48 83 05 ee 28 90 05 01 e9 dc fd ff
 RSP: 0018:ffff88805f7ffa28 EFLAGS: 00010202
 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 1ffff1100befff5e
 RDX: 0000000000000000 RSI: 0000000000000017 RDI: 0000000000000000
 RBP: 000000050695a900 R08: ffff888060fc7900 R09: ffff888060fc793b
 R10: ffffed100c1f8f27 R11: ffffed100c1f8f28 R12: 0000000000040dc0
 R13: 000000050695a900 R14: 0000000000000017 R15: 0000000000000001
 FS:  00007f521f695740(0000) GS:ffff88806ce00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007f31b013f000 CR3: 000000006637e001 CR4: 0000000000370eb0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 Call Trace:
  ? __kmalloc_track_caller+0x17a/0x570
  ? gcov_info_dup+0xfe/0x730
  ? gcov_event+0x88d/0xd30
  ? gcov_module_notifier+0xe9/0x100
  ? blocking_notifier_call_chain+0x75/0xc0
  ? __x64_sys_delete_module+0x326/0x5a0
  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
  ? mark_lock+0xba0/0xba0
  ? mark_lock+0xba0/0xba0
  ? notifier_call_chain+0xeb/0x170
  ? blocking_notifier_call_chain+0x75/0xc0
  ? __x64_sys_delete_module+0x326/0x5a0
  ? do_syscall_64+0x45/0x70
  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
  ? warn_alloc+0x130/0x130
  ? lock_acquire+0x1f2/0xa30
  ? fs_reclaim_acquire+0x1f/0x70
  ? fs_reclaim_release+0x1f/0x50
  ? __kasan_check_read+0x1d/0x30
  ? reacquire_held_locks+0x420/0x420
  ? reacquire_held_locks+0x420/0x420
  kmalloc_order+0x3f/0xc0
  kmalloc_order_trace+0x24/0x220
  __kmalloc+0x41b/0x5a0
  ? gcov_info_dup+0xfe/0x730
  ? memcpy+0x73/0xa0
  gcov_info_dup+0x176/0x730
  gcov_event+0x88d/0xd30
  gcov_module_notifier+0xe9/0x100
  notifier_call_chain+0xeb/0x170
  blocking_notifier_call_chain+0x75/0xc0
  __x64_sys_delete_module+0x326/0x5a0
  ? do_init_module+0x810/0x810
  ? syscall_enter_from_user_mode+0x40/0x420
  ? trace_hardirqs_on+0x45/0xb0
  ? syscall_enter_from_user_mode+0x40/0x420
  do_syscall_64+0x45/0x70
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
 RIP: 0033:0x7f521f7c531b
 Code: 73 01 c3 48 8b 0d 7d 0b 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 4d 0b 0c 00 f7 d8 64 89 01 48
 RSP: 002b:00007ffe1bd4af48 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
 RAX: ffffffffffffffda RBX: 0000561a3eae0910 RCX: 00007f521f7c531b
 RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000561a3eae0978
 RBP: 0000561a3eae0910 R08: 1999999999999999 R09: 0000000000000000
 R10: 00007f521f839ac0 R11: 0000000000000206 R12: 0000000000000000
 R13: 0000561a3eae0978 R14: 0000000000000000 R15: 0000561a3eae84d0
 irq event stamp: 326464
 hardirqs last  enabled at (326463): [<ffffffff832ecdde>] _raw_spin_unlock_irqrestore+0x8e/0xb0
 hardirqs last disabled at (326464): [<ffffffff832ec994>] _raw_spin_lock_irqsave+0x34/0xa0
 hardirqs last disabled at (326464): [<ffffffff832ec994>] _raw_spin_lock_irqsave+0x34/0xa0
 softirqs last  enabled at (320794): [<ffffffff83600931>] __do_softirq+0x931/0xbc4
 softirqs last disabled at (320789): [<ffffffff83400f2f>] asm_call_on_stack+0xf/0x20
 ---[ end trace 065ea9cc2ba144a6 ]---

This trace is seen because n_function value provided by GCC through
__gcov_init() is ridiculously high, in my case it was 2698213824,
which probably means that the field is not initialized.

In order to overcome this, fail allocation silently and rely on GCOV print later:
 gcov: could not save data for '/home/leonro/src/kernel/drivers/infiniband/hw/mlx5/std_types.gcda' (out of memory)

[1] https://bugzilla.kernel.org/show_bug.cgi?id=208885#c1
Cc: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 kernel/gcov/gcc_4_7.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
index 318211deb903..f2856896d12f 100644
--- a/kernel/gcov/gcc_4_7.c
+++ b/kernel/gcov/gcc_4_7.c
@@ -287,8 +287,9 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)
 	if (!dup->filename)
 		goto err_free;

-	dup->functions = kcalloc(info->n_functions,
-				 sizeof(struct gcov_fn_info *), GFP_KERNEL);
+	dup->functions =
+		kcalloc(info->n_functions, sizeof(struct gcov_fn_info *),
+			GFP_KERNEL | __GFP_NOWARN);
 	if (!dup->functions)
 		goto err_free;

--
2.26.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH rdma-next 4/4] gcov: Don't print out-of-memory print for all failed files
  2020-09-02  8:55 [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Leon Romanovsky
                   ` (2 preceding siblings ...)
  2020-09-02  8:55 ` [PATCH rdma-next 3/4] gcov: Protect from uninitialized number of functions provided by GCC 10.2 Leon Romanovsky
@ 2020-09-02  8:55 ` Leon Romanovsky
  2020-09-02 17:42 ` [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Linus Torvalds
  4 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02  8:55 UTC (permalink / raw)
  To: Linus Torvalds, Peter Oberparleiter
  Cc: Leon Romanovsky, linux-kernel, Colin Ian King, Andrew Morton

From: Leon Romanovsky <leonro@nvidia.com>

Once GCOV fails to duplicate information, the following error is
printed:
 gcov: could not save data for '/home/leonro/src/kernel/drivers/infiniband/hw/mlx5/std_types.gcda' (out of memory)

In the event of out-of-memory such prints are seen for almost every kernel
file, so instead of spamming dmesg, we print the first failure and inform
the user that future prints are suppressed.

Cc: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 kernel/gcov/fs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/gcov/fs.c b/kernel/gcov/fs.c
index 82babf5aa077..b74d426ca99e 100644
--- a/kernel/gcov/fs.c
+++ b/kernel/gcov/fs.c
@@ -685,8 +685,9 @@ static void save_info(struct gcov_node *node, struct gcov_info *info)
 	else {
 		node->unloaded_info = gcov_info_dup(info);
 		if (!node->unloaded_info) {
-			pr_warn("could not save data for '%s' "
-				"(out of memory)\n",
+			pr_warn_once(
+				"could not save data for first file '%s' "
+				"(out of memory), other files are suppressed\n",
 				gcov_info_filename(info));
 		}
 	}
--
2.26.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers
  2020-09-02  8:55 ` [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers Leon Romanovsky
@ 2020-09-02 17:38   ` Linus Torvalds
  2020-09-02 17:46     ` Leon Romanovsky
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Torvalds @ 2020-09-02 17:38 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Peter Oberparleiter, Leon Romanovsky, Linux Kernel Mailing List,
	Colin Ian King, Andrew Morton

On Wed, Sep 2, 2020 at 1:55 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> The kernel with KASAN and GCOV enabled generates the following splat
> due to the situation that gcov_info can be both user and kernel pointer.

I can't parse the above explanation..

> It is triggered by the memcpy() inside kmemdup(), so as a possible solution
> let's copy fields manually.

.. and I don't see why copying the fields manually makes a difference.

Can you explain more?

             Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 3/4] gcov: Protect from uninitialized number of functions provided by GCC 10.2
  2020-09-02  8:55 ` [PATCH rdma-next 3/4] gcov: Protect from uninitialized number of functions provided by GCC 10.2 Leon Romanovsky
@ 2020-09-02 17:41   ` Linus Torvalds
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Torvalds @ 2020-09-02 17:41 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Peter Oberparleiter, Leon Romanovsky, Linux Kernel Mailing List,
	Colin Ian King, Andrew Morton

On Wed, Sep 2, 2020 at 1:55 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> This trace is seen because n_function value provided by GCC through
> __gcov_init() is ridiculously high, in my case it was 2698213824,
> which probably means that the field is not initialized.

This seems to be wrong - since a different (smaller) uninitialized
value will succeed in the kcalloc, but will then walk some random
array size when copying and fail later instead.

So this doesn't actually seem to _fix_ anything, it just hides one
special case of bogus values.

              Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH -rc 0/4] Protect from GCC garbage input in GCOV
  2020-09-02  8:55 [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Leon Romanovsky
                   ` (3 preceding siblings ...)
  2020-09-02  8:55 ` [PATCH rdma-next 4/4] gcov: Don't print out-of-memory print for all failed files Leon Romanovsky
@ 2020-09-02 17:42 ` Linus Torvalds
  2020-09-02 17:52   ` Leon Romanovsky
  4 siblings, 1 reply; 17+ messages in thread
From: Linus Torvalds @ 2020-09-02 17:42 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Peter Oberparleiter, Leon Romanovsky, Linux Kernel Mailing List,
	Colin Ian King, Andrew Morton

On Wed, Sep 2, 2020 at 1:55 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> Bottom line, GCOV is broken on GCC 10.2.

The patches don't really make sense to me.

How about we just disable GCOV with the known-broken compiler version
instead? As mentioned in the replies to individual patches, it looks
like the "fixes" are random bandaids that don't _really_ fix anything.

                Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers
  2020-09-02 17:38   ` Linus Torvalds
@ 2020-09-02 17:46     ` Leon Romanovsky
  2020-09-02 18:27       ` Linus Torvalds
  0 siblings, 1 reply; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02 17:46 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 02, 2020 at 10:38:20AM -0700, Linus Torvalds wrote:
> On Wed, Sep 2, 2020 at 1:55 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > The kernel with KASAN and GCOV enabled generates the following splat
> > due to the situation that gcov_info can be both user and kernel pointer.
>
> I can't parse the above explanation..
>
> > It is triggered by the memcpy() inside kmemdup(), so as a possible solution
> > let's copy fields manually.
>
> .. and I don't see why copying the fields manually makes a difference.
>
> Can you explain more?

Definitely my explanation is wrong, but it was my interpretation of
"BUG: KASAN: global-out-of-bounds in kmemdup+0x43/0x70" line. I saw
that the failure was in memcpy() inside of kmemdup(), so I changed
from memcpy to be copy_from_user() and it solved the KASAN warning.

This is why I wrote "both user and kernel pointer".

Thanks

>
>              Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH -rc 0/4] Protect from GCC garbage input in GCOV
  2020-09-02 17:42 ` [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Linus Torvalds
@ 2020-09-02 17:52   ` Leon Romanovsky
  2020-09-02 18:24     ` Linus Torvalds
  0 siblings, 1 reply; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02 17:52 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 02, 2020 at 10:42:55AM -0700, Linus Torvalds wrote:
> On Wed, Sep 2, 2020 at 1:55 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > Bottom line, GCOV is broken on GCC 10.2.
>
> The patches don't really make sense to me.
>
> How about we just disable GCOV with the known-broken compiler version
> instead? As mentioned in the replies to individual patches, it looks
> like the "fixes" are random bandaids that don't _really_ fix anything.

Right, as I wrote in RFC "solution is wrong", I knew it, just didn't
get any feedback on how to do it correctly.

Are you suggesting something like this?

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 3110c77230c7..bc0e355f64aa 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -3,7 +3,7 @@ menu "GCOV-based kernel profiling"

 config GCOV_KERNEL
        bool "Enable gcov-based kernel profiling"
-       depends on DEBUG_FS
+       depends on DEBUG_FS && (GCC_VERSION >= XXX && GCC_VERSION < YYY)
        select CONSTRUCTORS if !UML
        default n
        help
~


Thanks

>
>                 Linus

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH -rc 0/4] Protect from GCC garbage input in GCOV
  2020-09-02 17:52   ` Leon Romanovsky
@ 2020-09-02 18:24     ` Linus Torvalds
  2020-09-02 18:28       ` Leon Romanovsky
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Torvalds @ 2020-09-02 18:24 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 2, 2020 at 10:52 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> Are you suggesting something like this?
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 3110c77230c7..bc0e355f64aa 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -3,7 +3,7 @@ menu "GCOV-based kernel profiling"
>
>  config GCOV_KERNEL
>         bool "Enable gcov-based kernel profiling"
> -       depends on DEBUG_FS
> +       depends on DEBUG_FS && (GCC_VERSION >= XXX && GCC_VERSION < YYY)
>         select CONSTRUCTORS if !UML
>         default n
>         help

Yes. Except please don't mix it up with DEBUG_FS. Just add a new
"depends on" line. They are separate issues.

              Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers
  2020-09-02 17:46     ` Leon Romanovsky
@ 2020-09-02 18:27       ` Linus Torvalds
  2020-09-02 18:44         ` Leon Romanovsky
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Torvalds @ 2020-09-02 18:27 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 2, 2020 at 10:46 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> Definitely my explanation is wrong, but it was my interpretation of
> "BUG: KASAN: global-out-of-bounds in kmemdup+0x43/0x70" line. I saw
> that the failure was in memcpy() inside of kmemdup(), so I changed
> from memcpy to be copy_from_user() and it solved the KASAN warning.

But the actual patch attached to that explanation *doesn't* use
copy_from_user().

So your "changed from memcpy to be copy_from_user() solved the KASAN
warning" explanation makes even less sense. Because that's not at all
what the patch does.

             Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH -rc 0/4] Protect from GCC garbage input in GCOV
  2020-09-02 18:24     ` Linus Torvalds
@ 2020-09-02 18:28       ` Leon Romanovsky
  0 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02 18:28 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 02, 2020 at 11:24:50AM -0700, Linus Torvalds wrote:
> On Wed, Sep 2, 2020 at 10:52 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > Are you suggesting something like this?
> >
> > diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> > index 3110c77230c7..bc0e355f64aa 100644
> > --- a/kernel/gcov/Kconfig
> > +++ b/kernel/gcov/Kconfig
> > @@ -3,7 +3,7 @@ menu "GCOV-based kernel profiling"
> >
> >  config GCOV_KERNEL
> >         bool "Enable gcov-based kernel profiling"
> > -       depends on DEBUG_FS
> > +       depends on DEBUG_FS && (GCC_VERSION >= XXX && GCC_VERSION < YYY)
> >         select CONSTRUCTORS if !UML
> >         default n
> >         help
>
> Yes. Except please don't mix it up with DEBUG_FS. Just add a new
> "depends on" line. They are separate issues.

Thanks, I'll do and resend.

>
>               Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers
  2020-09-02 18:27       ` Linus Torvalds
@ 2020-09-02 18:44         ` Leon Romanovsky
  2020-09-02 19:04           ` Linus Torvalds
  0 siblings, 1 reply; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-02 18:44 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 02, 2020 at 11:27:14AM -0700, Linus Torvalds wrote:
> On Wed, Sep 2, 2020 at 10:46 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > Definitely my explanation is wrong, but it was my interpretation of
> > "BUG: KASAN: global-out-of-bounds in kmemdup+0x43/0x70" line. I saw
> > that the failure was in memcpy() inside of kmemdup(), so I changed
> > from memcpy to be copy_from_user() and it solved the KASAN warning.
>
> But the actual patch attached to that explanation *doesn't* use
> copy_from_user().
>
> So your "changed from memcpy to be copy_from_user() solved the KASAN
> warning" explanation makes even less sense. Because that's not at all
> what the patch does.

I already don't remember why, but copy_from_user() caused to second flow
of gcov_info_dup() through gcov_event() to generate another set of warnings.

As a summary, I have a workaround, but I don't know why it works and not
proud about it.

Thanks

>
>              Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers
  2020-09-02 18:44         ` Leon Romanovsky
@ 2020-09-02 19:04           ` Linus Torvalds
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Torvalds @ 2020-09-02 19:04 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Peter Oberparleiter, Linux Kernel Mailing List, Colin Ian King,
	Andrew Morton

On Wed, Sep 2, 2020 at 11:44 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> I already don't remember why, but copy_from_user() caused to second flow
> of gcov_info_dup() through gcov_event() to generate another set of warnings.
>
> As a summary, I have a workaround, but I don't know why it works and not
> proud about it.

Ok, it does sound like "let's just disable gcov for the affected
compilers" is the way to go.

If/when somebody figures out exactly what's up and how to work around
it properly, maybe we can then revisit the issue.

Thanks,

                Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer
  2020-09-02  8:55 ` [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer Leon Romanovsky
@ 2020-09-03  8:56   ` Rasmus Villemoes
  2020-09-03 10:38     ` Leon Romanovsky
  0 siblings, 1 reply; 17+ messages in thread
From: Rasmus Villemoes @ 2020-09-03  8:56 UTC (permalink / raw)
  To: Leon Romanovsky, Linus Torvalds, Peter Oberparleiter
  Cc: Leon Romanovsky, linux-kernel, Colin Ian King, Andrew Morton

On 02/09/2020 10.55, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> The filename is a const pointer, so use the proper string duplication
> routine that takes into account const identifier.

This commit log makes no sense at all.

kstrdup_const is merely an optimization that can be used when there's a
good chance that the passed string lives in vmlinux' .rodata, in which
case it is known to be immortal, and we can avoid allocating heap memory
to contain a duplicate. [It also requires that the caller has no
intention of modifying the returned string.]

In the case of something called ->filename, I assume it's initialized
with __FILE__ somewhere, making the above true for built-in stuff but
not for modules. So if the gcov_info can live longer than the module,
it's of course necessary to duplicate the string, but OTOH making an
optimization for the built-in stuff makes sense. So this is certainly
one of the places where kstrdup_const() seems applicable. But it has
nothing whatsoever to do with the C-level qualifiers the argument may have.

Rasmus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer
  2020-09-03  8:56   ` Rasmus Villemoes
@ 2020-09-03 10:38     ` Leon Romanovsky
  0 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2020-09-03 10:38 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Linus Torvalds, Peter Oberparleiter, linux-kernel,
	Colin Ian King, Andrew Morton

On Thu, Sep 03, 2020 at 10:56:38AM +0200, Rasmus Villemoes wrote:
> On 02/09/2020 10.55, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> >
> > The filename is a const pointer, so use the proper string duplication
> > routine that takes into account const identifier.
>
> This commit log makes no sense at all.
>
> kstrdup_const is merely an optimization that can be used when there's a
> good chance that the passed string lives in vmlinux' .rodata, in which
> case it is known to be immortal, and we can avoid allocating heap memory
> to contain a duplicate. [It also requires that the caller has no
> intention of modifying the returned string.]
>
> In the case of something called ->filename, I assume it's initialized
> with __FILE__ somewhere, making the above true for built-in stuff but
> not for modules. So if the gcov_info can live longer than the module,
> it's of course necessary to duplicate the string, but OTOH making an
> optimization for the built-in stuff makes sense. So this is certainly
> one of the places where kstrdup_const() seems applicable. But it has
> nothing whatsoever to do with the C-level qualifiers the argument may have.

Thanks, GCOV can't be built as module.

>
> Rasmus

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-09-03 10:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02  8:55 [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Leon Romanovsky
2020-09-02  8:55 ` [PATCH rdma-next 1/4] gcov: Open-code kmemdup() to work correctly with kernel and user space pointers Leon Romanovsky
2020-09-02 17:38   ` Linus Torvalds
2020-09-02 17:46     ` Leon Romanovsky
2020-09-02 18:27       ` Linus Torvalds
2020-09-02 18:44         ` Leon Romanovsky
2020-09-02 19:04           ` Linus Torvalds
2020-09-02  8:55 ` [PATCH rdma-next 2/4] gcov: Use proper duplication routine for const pointer Leon Romanovsky
2020-09-03  8:56   ` Rasmus Villemoes
2020-09-03 10:38     ` Leon Romanovsky
2020-09-02  8:55 ` [PATCH rdma-next 3/4] gcov: Protect from uninitialized number of functions provided by GCC 10.2 Leon Romanovsky
2020-09-02 17:41   ` Linus Torvalds
2020-09-02  8:55 ` [PATCH rdma-next 4/4] gcov: Don't print out-of-memory print for all failed files Leon Romanovsky
2020-09-02 17:42 ` [PATCH -rc 0/4] Protect from GCC garbage input in GCOV Linus Torvalds
2020-09-02 17:52   ` Leon Romanovsky
2020-09-02 18:24     ` Linus Torvalds
2020-09-02 18:28       ` Leon Romanovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.