gcov: NULL pointer dereference with gcc 9.3.1

gcov: NULL pointer dereference with gcc 9.3.1

[Luis Henriques]

Hi!

Maybe this is a known (gcc?) issue, but I'm seeing a NULL pointer splat if
I instrument my kernel (or a module, more specifically) using gcc 9.3.1.

It looks like, during initialization in __gcov_init(), gcov_info struct is
invalid: the filename seems to be correct but ->function is NULL and
->n_functions contains garbage.  This will result in the NULL pointer if,
for example, I run:

  # echo 1 > /sys/kernel/debug/gcov/reset

Other installed gcc versions (7.5.0 and 10.3.0) work fine.

Is this a know issue with this specific gcc versions?  Anyway, it may
be worth having something like (not tested!):

diff --git a/kernel/gcov/gcc_base.c b/kernel/gcov/gcc_base.c
index 3cf736b9f880..de3ea47f4f62 100644
--- a/kernel/gcov/gcc_base.c
+++ b/kernel/gcov/gcc_base.c
@@ -13,6 +13,11 @@ void __gcov_init(struct gcov_info *info)
 {
 	static unsigned int gcov_version;
 
+	if (!info || !info->functions) {
+		pr_err_once("Invalid gcov_info structure\n");
+		return;
+	}
+
 	mutex_lock(&gcov_lock);
 	if (gcov_version == 0) {
 		gcov_version = gcov_info_version(info);

And also something similar in the clang code, I guess.

I can send a real (tested) patch if you wish, but I just wanted to know if
I'm missing something first.

Cheers,
--
Luis

[RFC PATCH] gcov: add basic gcov_info validation to gcov initialization

[Luis Henriques]

Add a basic gcov_info struct validation helper to gcc to ensure we have
sane from the compiler.

Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
Hi!

I know this won't really validate the gcov_info struct, but it will at
least prevent kernel crashes in simple scenarios (such as the one I'm
seeing with gcc 9.3.1).

Cheers,
--
Luis

 kernel/gcov/gcc_4_7.c  | 8 ++++++++
 kernel/gcov/gcc_base.c | 5 +++++
 kernel/gcov/gcov.h     | 1 +
 3 files changed, 14 insertions(+)

diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
index 460c12b7dfea..028d9b94463d 100644
--- a/kernel/gcov/gcc_4_7.c
+++ b/kernel/gcov/gcc_4_7.c
@@ -126,6 +126,14 @@ struct gcov_info *gcov_info_next(struct gcov_info *info)
 	return info->next;
 }
 
+bool gcov_info_is_valid(struct gcov_info *info)
+{
+	if (!info || !info->functions)
+		return false;
+
+	return true;
+}
+
 /**
  * gcov_info_link - link/add profiling data set to the list
  * @info: profiling data set
diff --git a/kernel/gcov/gcc_base.c b/kernel/gcov/gcc_base.c
index 3cf736b9f880..3e02a89f69b6 100644
--- a/kernel/gcov/gcc_base.c
+++ b/kernel/gcov/gcc_base.c
@@ -13,6 +13,11 @@ void __gcov_init(struct gcov_info *info)
 {
 	static unsigned int gcov_version;
 
+	if (!gcov_info_is_valid(info)) {
+		pr_err_once("Invalid gcov_info structure\n");
+		return;
+	}
+
 	mutex_lock(&gcov_lock);
 	if (gcov_version == 0) {
 		gcov_version = gcov_info_version(info);
diff --git a/kernel/gcov/gcov.h b/kernel/gcov/gcov.h
index 912b8ea01d33..4b3d924c2031 100644
--- a/kernel/gcov/gcov.h
+++ b/kernel/gcov/gcov.h
@@ -45,6 +45,7 @@ struct gcov_info;
 const char *gcov_info_filename(struct gcov_info *info);
 unsigned int gcov_info_version(struct gcov_info *info);
 struct gcov_info *gcov_info_next(struct gcov_info *info);
+bool gcov_info_is_valid(struct gcov_info *info);
 void gcov_info_link(struct gcov_info *info);
 void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info);
 bool gcov_info_within_module(struct gcov_info *info, struct module *mod);

Re: [RFC PATCH] gcov: add basic gcov_info validation to gcov initialization

[Peter Oberparleiter]

On 02.06.2021 12:24, Luis Henriques wrote:
> Add a basic gcov_info struct validation helper to gcc to ensure we have
> sane from the compiler.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>
> ---
> Hi!
> 
> I know this won't really validate the gcov_info struct, but it will at
> least prevent kernel crashes in simple scenarios (such as the one I'm
> seeing with gcc 9.3.1).

Thanks for your suggestion of adding validity checking for the gcov_info
struct. The goal you aim at is definitely something that we want to have
to reduce the impact of fallout from changes to GCC's gcov_info struct.

In my opinion though the approach you described - looking at the
contents of specific fields in gcov_info - isn't the correct way to go
forward. Since you cannot know how gcov_info changed, accessing any data
in it is very dangerous. Even if there's no out-of-bounds access (if the
struct's size was reduced) the field you are checking could have moved
elsewhere so the meaningfulness of the check is very limited.

In a previous discussion on the same topic I proposed a different
approach for a build-time check that would fully check the compatibility
of kernel code and GCC-emitted gcov-related data structures. See:

https://lore.kernel.org/patchwork/patch/1393585/#1592411

Unfortunately I have not yet found the time to implement this approach
but it's still on my to-do-list.

Regarding the cause of the error you're seeing I'll have a look at the
corresponding GCC source to see if there's anything that could be
causing the issue.


Regards,
  Peter Oberparleiter

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany

Re: [RFC PATCH] gcov: add basic gcov_info validation to gcov initialization

[Luis Henriques]

On Mon, Jun 07, 2021 at 11:59:45AM +0200, Peter Oberparleiter wrote:
> On 02.06.2021 12:24, Luis Henriques wrote:
> > Add a basic gcov_info struct validation helper to gcc to ensure we have
> > sane from the compiler.
> > 
> > Signed-off-by: Luis Henriques <lhenriques@suse.de>
> > ---
> > Hi!
> > 
> > I know this won't really validate the gcov_info struct, but it will at
> > least prevent kernel crashes in simple scenarios (such as the one I'm
> > seeing with gcc 9.3.1).
> 
> Thanks for your suggestion of adding validity checking for the gcov_info
> struct. The goal you aim at is definitely something that we want to have
> to reduce the impact of fallout from changes to GCC's gcov_info struct.
> 
> In my opinion though the approach you described - looking at the
> contents of specific fields in gcov_info - isn't the correct way to go
> forward. Since you cannot know how gcov_info changed, accessing any data
> in it is very dangerous. Even if there's no out-of-bounds access (if the
> struct's size was reduced) the field you are checking could have moved
> elsewhere so the meaningfulness of the check is very limited.
> 
> In a previous discussion on the same topic I proposed a different
> approach for a build-time check that would fully check the compatibility
> of kernel code and GCC-emitted gcov-related data structures. See:
> 
> https://lore.kernel.org/patchwork/patch/1393585/#1592411
> 

Thanks, I see the problem is way more complex and I understand that my
patch is just wrong.  Thanks for pointing me at this thread.

> Unfortunately I have not yet found the time to implement this approach
> but it's still on my to-do-list.
> 
> Regarding the cause of the error you're seeing I'll have a look at the
> corresponding GCC source to see if there's anything that could be
> causing the issue.

Great, thanks.  Let me know if you need me to provide more info or
testing.  I'll be glad to help.

Cheers,
--
Luís

Re: gcov: NULL pointer dereference with gcc 9.3.1

[Peter Oberparleiter]

On 01.06.2021 17:56, Luis Henriques wrote:
> Hi!
> 
> Maybe this is a known (gcc?) issue, but I'm seeing a NULL pointer splat if
> I instrument my kernel (or a module, more specifically) using gcc 9.3.1.
> 
> It looks like, during initialization in __gcov_init(), gcov_info struct is
> invalid: the filename seems to be correct but ->function is NULL and
> ->n_functions contains garbage.

Thanks for reporting this issue. The symptoms you're seeing look similar
to those that occur when the struct gcov_info layout emitted by GCC does
not match the one used by the kernel. In particular a change in the
GCOV_COUNTER value can cause this behavior.

I've checked upstream GCC 9.3.1 and it seems to match what is used by
the kernel for that GCC version. Could you provide the exact version of
the compiler you are using? Both 'gcc --version' output and the GCC
package version should help. Also what architecture are you seeing this on?


Regards,
  Peter Oberparleiter

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany

Re: gcov: NULL pointer dereference with gcc 9.3.1

[Luis Henriques]

On Wed, Jun 02, 2021 at 02:35:31PM +0200, Peter Oberparleiter wrote:
> On 01.06.2021 17:56, Luis Henriques wrote:
> > Hi!
> > 
> > Maybe this is a known (gcc?) issue, but I'm seeing a NULL pointer splat if
> > I instrument my kernel (or a module, more specifically) using gcc 9.3.1.
> > 
> > It looks like, during initialization in __gcov_init(), gcov_info struct is
> > invalid: the filename seems to be correct but ->function is NULL and
> > ->n_functions contains garbage.
> 
> Thanks for reporting this issue. The symptoms you're seeing look similar
> to those that occur when the struct gcov_info layout emitted by GCC does
> not match the one used by the kernel. In particular a change in the
> GCOV_COUNTER value can cause this behavior.
> 
> I've checked upstream GCC 9.3.1 and it seems to match what is used by
> the kernel for that GCC version. Could you provide the exact version of
> the compiler you are using? Both 'gcc --version' output and the GCC
> package version should help. Also what architecture are you seeing this on?

Here's the output of 'gcc --version':

gcc (SUSE Linux) 9.3.1 20200903 [revision 9790fa53b48f3a48e0f7a7ad65e2bbf3b206a7b0]
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

This is the version shipped with openSUSE Tumbleweed, and I'm using it to
compile an x86_64 kernel.  Regarding the 'package version', I'm assuming
the packages as per the distro package version, right?  Here's the data
from 'zypper info':

Information for package gcc9:
-----------------------------
Repository     : Main Repository (OSS)
Name           : gcc9
Version        : 9.3.1+git1684-3.5
Arch           : x86_64
Vendor         : openSUSE
Installed Size : 94.6 MiB
Installed      : Yes (automatically)
Status         : up-to-date
Source package : gcc9-9.3.1+git1684-3.5.src

Do you have a link with binaries I could test for upstream 9.3.1?  I
checked [1] but there's only 9.3.0.

[1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/

Cheers,
--
Luís

Re: gcov: NULL pointer dereference with gcc 9.3.1

[Peter Oberparleiter]

On 02.06.2021 16:22, Luis Henriques wrote:
> On Wed, Jun 02, 2021 at 02:35:31PM +0200, Peter Oberparleiter wrote:
>> On 01.06.2021 17:56, Luis Henriques wrote:
>>> Hi!
>>>
>>> Maybe this is a known (gcc?) issue, but I'm seeing a NULL pointer splat if
>>> I instrument my kernel (or a module, more specifically) using gcc 9.3.1.
>>>
>>> It looks like, during initialization in __gcov_init(), gcov_info struct is
>>> invalid: the filename seems to be correct but ->function is NULL and
>>> ->n_functions contains garbage.
>>
>> Thanks for reporting this issue. The symptoms you're seeing look similar
>> to those that occur when the struct gcov_info layout emitted by GCC does
>> not match the one used by the kernel. In particular a change in the
>> GCOV_COUNTER value can cause this behavior.
>>
>> I've checked upstream GCC 9.3.1 and it seems to match what is used by
>> the kernel for that GCC version. Could you provide the exact version of
>> the compiler you are using? Both 'gcc --version' output and the GCC
>> package version should help. Also what architecture are you seeing this on?
> 
> Here's the output of 'gcc --version':
> 
> gcc (SUSE Linux) 9.3.1 20200903 [revision 9790fa53b48f3a48e0f7a7ad65e2bbf3b206a7b0]
> Copyright (C) 2019 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> This is the version shipped with openSUSE Tumbleweed, and I'm using it to
> compile an x86_64 kernel.  Regarding the 'package version', I'm assuming
> the packages as per the distro package version, right?  Here's the data
> from 'zypper info':
> 
> Information for package gcc9:
> -----------------------------
> Repository     : Main Repository (OSS)
> Name           : gcc9
> Version        : 9.3.1+git1684-3.5
> Arch           : x86_64
> Vendor         : openSUSE
> Installed Size : 94.6 MiB
> Installed      : Yes (automatically)
> Status         : up-to-date
> Source package : gcc9-9.3.1+git1684-3.5.src

I've checked the source you referenced and found that it contains a
backport of a change to gcov_info that was only introduced with GCC 10
to upstream source: the value of GCOV_COUNTERS was reduced from 9 to 8.

Since I don't think it's feasible to implement support for such
vendor-specific changes in the upstream kernel source my suggestion for
you would be to either

a) fall back to a vanilla GCC version,
b) fall back to a known-to-work vendor-specific GCC version (GCC 10
   should be fine), or
c) to manually change the GCOV_COUNTERS value in
   linux/kernel/gcov/gcc_4_7.c to 8.

> Do you have a link with binaries I could test for upstream 9.3.1?  I
> checked [1] but there's only 9.3.0.

I'm not sure there is any. My analysis was based on source code for
9.3.0 alone.


Regards,
  Peter Oberparleiter

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany

Re: gcov: NULL pointer dereference with gcc 9.3.1

[Luis Henriques]

On Mon, Jun 07, 2021 at 04:32:14PM +0200, Peter Oberparleiter wrote:
> On 02.06.2021 16:22, Luis Henriques wrote:
> > On Wed, Jun 02, 2021 at 02:35:31PM +0200, Peter Oberparleiter wrote:
> >> On 01.06.2021 17:56, Luis Henriques wrote:
> >>> Hi!
> >>>
> >>> Maybe this is a known (gcc?) issue, but I'm seeing a NULL pointer splat if
> >>> I instrument my kernel (or a module, more specifically) using gcc 9.3.1.
> >>>
> >>> It looks like, during initialization in __gcov_init(), gcov_info struct is
> >>> invalid: the filename seems to be correct but ->function is NULL and
> >>> ->n_functions contains garbage.
> >>
> >> Thanks for reporting this issue. The symptoms you're seeing look similar
> >> to those that occur when the struct gcov_info layout emitted by GCC does
> >> not match the one used by the kernel. In particular a change in the
> >> GCOV_COUNTER value can cause this behavior.
> >>
> >> I've checked upstream GCC 9.3.1 and it seems to match what is used by
> >> the kernel for that GCC version. Could you provide the exact version of
> >> the compiler you are using? Both 'gcc --version' output and the GCC
> >> package version should help. Also what architecture are you seeing this on?
> > 
> > Here's the output of 'gcc --version':
> > 
> > gcc (SUSE Linux) 9.3.1 20200903 [revision 9790fa53b48f3a48e0f7a7ad65e2bbf3b206a7b0]
> > Copyright (C) 2019 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> > 
> > This is the version shipped with openSUSE Tumbleweed, and I'm using it to
> > compile an x86_64 kernel.  Regarding the 'package version', I'm assuming
> > the packages as per the distro package version, right?  Here's the data
> > from 'zypper info':
> > 
> > Information for package gcc9:
> > -----------------------------
> > Repository     : Main Repository (OSS)
> > Name           : gcc9
> > Version        : 9.3.1+git1684-3.5
> > Arch           : x86_64
> > Vendor         : openSUSE
> > Installed Size : 94.6 MiB
> > Installed      : Yes (automatically)
> > Status         : up-to-date
> > Source package : gcc9-9.3.1+git1684-3.5.src
> 
> I've checked the source you referenced and found that it contains a
> backport of a change to gcov_info that was only introduced with GCC 10
> to upstream source: the value of GCOV_COUNTERS was reduced from 9 to 8.
> 
> Since I don't think it's feasible to implement support for such
> vendor-specific changes in the upstream kernel source my suggestion for
> you would be to either
> 
> a) fall back to a vanilla GCC version,
> b) fall back to a known-to-work vendor-specific GCC version (GCC 10
>    should be fine), or
> c) to manually change the GCOV_COUNTERS value in
>    linux/kernel/gcov/gcc_4_7.c to 8.

Thanks a lot for looking Peter, I've already reported this issue to the
maintainers.  Hopefully it'll soon be looked at.  Anyway, to be honest I
just hit this issue accidentally -- I don't really have a requirement on
using this version specifically.

Cheers,
--
Luís

> > Do you have a link with binaries I could test for upstream 9.3.1?  I
> > checked [1] but there's only 9.3.0.
> 
> I'm not sure there is any. My analysis was based on source code for
> 9.3.0 alone.
> 
> 
> Regards,
>   Peter Oberparleiter
> 
> -- 
> Peter Oberparleiter
> Linux on Z Development - IBM Germany