[PATCH] kconfig: check for a NULL pointer before access

[PATCH] kconfig: check for a NULL pointer before access

[Bill Wendling]

The call to 'prop_get_symbol' may return NULL in some cases. The if-then
statement accesses the returned value without checking if it's
non-NULL. After inlining, the compiler may treat the conditional as
'undefined behavior', which the compiler may take the opportunity to do
whatever it wants with the UB path. This patch simply adds a check to
ensure that 'def_sym' is non-NULL to avoid this behavior.

Signed-off-by: Bill Wendling <morbo@google.com>
---
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 scripts/kconfig/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index d57f8cbba291..9c5068225328 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
  if (prop->visible.tri == no)
  continue;
  def_sym = prop_get_symbol(prop);
- if (def_sym->visible != no)
+ if (def_sym && def_sym->visible != no)
  return def_sym;
  }

-- 
2.49.0.1164.gab81da1b16-goog

Re: [PATCH] kconfig: check for a NULL pointer before access

[Randy Dunlap]

Hi,

On 5/22/25 5:07 PM, Bill Wendling wrote:
> The call to 'prop_get_symbol' may return NULL in some cases. The if-then
> statement accesses the returned value without checking if it's
> non-NULL. After inlining, the compiler may treat the conditional as
> 'undefined behavior', which the compiler may take the opportunity to do
> whatever it wants with the UB path. This patch simply adds a check to
> ensure that 'def_sym' is non-NULL to avoid this behavior.
> 
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  scripts/kconfig/symbol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index d57f8cbba291..9c5068225328 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
>   if (prop->visible.tri == no)
>   continue;
>   def_sym = prop_get_symbol(prop);
> - if (def_sym->visible != no)
> + if (def_sym && def_sym->visible != no)
>   return def_sym;
>   }
> 

The patch is missing the source file's indentation.
(spaces/tabs are lost)

-- 
~Randy

Re: [PATCH] kconfig: check for a NULL pointer before access

[Bill Wendling]

On Thu, May 22, 2025 at 5:16 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi,
>
> On 5/22/25 5:07 PM, Bill Wendling wrote:
> > The call to 'prop_get_symbol' may return NULL in some cases. The if-then
> > statement accesses the returned value without checking if it's
> > non-NULL. After inlining, the compiler may treat the conditional as
> > 'undefined behavior', which the compiler may take the opportunity to do
> > whatever it wants with the UB path. This patch simply adds a check to
> > ensure that 'def_sym' is non-NULL to avoid this behavior.
> >
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: linux-kbuild@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  scripts/kconfig/symbol.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> > index d57f8cbba291..9c5068225328 100644
> > --- a/scripts/kconfig/symbol.c
> > +++ b/scripts/kconfig/symbol.c
> > @@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
> >   if (prop->visible.tri == no)
> >   continue;
> >   def_sym = prop_get_symbol(prop);
> > - if (def_sym->visible != no)
> > + if (def_sym && def_sym->visible != no)
> >   return def_sym;
> >   }
> >
>
> The patch is missing the source file's indentation.
> (spaces/tabs are lost)
>
Crud! My mailer borked. I sent v2 and it looks to have kept the whitespaces.

-bw

Re: [PATCH] kconfig: check for a NULL pointer before access

[Randy Dunlap]

On 5/23/25 3:56 PM, Bill Wendling wrote:
> On Thu, May 22, 2025 at 5:16 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> Hi,
>>
>> On 5/22/25 5:07 PM, Bill Wendling wrote:
>>> The call to 'prop_get_symbol' may return NULL in some cases. The if-then
>>> statement accesses the returned value without checking if it's
>>> non-NULL. After inlining, the compiler may treat the conditional as
>>> 'undefined behavior', which the compiler may take the opportunity to do
>>> whatever it wants with the UB path. This patch simply adds a check to
>>> ensure that 'def_sym' is non-NULL to avoid this behavior.
>>>
>>> Signed-off-by: Bill Wendling <morbo@google.com>
>>> ---
>>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>>> Cc: linux-kbuild@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>  scripts/kconfig/symbol.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>> index d57f8cbba291..9c5068225328 100644
>>> --- a/scripts/kconfig/symbol.c
>>> +++ b/scripts/kconfig/symbol.c
>>> @@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
>>>   if (prop->visible.tri == no)
>>>   continue;
>>>   def_sym = prop_get_symbol(prop);
>>> - if (def_sym->visible != no)
>>> + if (def_sym && def_sym->visible != no)
>>>   return def_sym;
>>>   }
>>>
>>
>> The patch is missing the source file's indentation.
>> (spaces/tabs are lost)
>>
> Crud! My mailer borked. I sent v2 and it looks to have kept the whitespaces.

I don't think v2 worked either.
See  https://lore.kernel.org/linux-kbuild/CAGG=3QXQkJ6n0J1gZcgxfEb68NWN2y200ZCuxxDtqPRgWPci=A@mail.gmail.com/T/#mf64c7afd19235d3dee4e572f96ff76936f921c6e

-- 
~Randy

Re: [PATCH] kconfig: check for a NULL pointer before access

[Bill Wendling]

On Fri, May 23, 2025 at 4:48 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> On 5/23/25 3:56 PM, Bill Wendling wrote:
> > On Thu, May 22, 2025 at 5:16 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> >>
> >> Hi,
> >>
> >> On 5/22/25 5:07 PM, Bill Wendling wrote:
> >>> The call to 'prop_get_symbol' may return NULL in some cases. The if-then
> >>> statement accesses the returned value without checking if it's
> >>> non-NULL. After inlining, the compiler may treat the conditional as
> >>> 'undefined behavior', which the compiler may take the opportunity to do
> >>> whatever it wants with the UB path. This patch simply adds a check to
> >>> ensure that 'def_sym' is non-NULL to avoid this behavior.
> >>>
> >>> Signed-off-by: Bill Wendling <morbo@google.com>
> >>> ---
> >>> Cc: Masahiro Yamada <masahiroy@kernel.org>
> >>> Cc: linux-kbuild@vger.kernel.org
> >>> Cc: linux-kernel@vger.kernel.org
> >>> ---
> >>>  scripts/kconfig/symbol.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> >>> index d57f8cbba291..9c5068225328 100644
> >>> --- a/scripts/kconfig/symbol.c
> >>> +++ b/scripts/kconfig/symbol.c
> >>> @@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
> >>>   if (prop->visible.tri == no)
> >>>   continue;
> >>>   def_sym = prop_get_symbol(prop);
> >>> - if (def_sym->visible != no)
> >>> + if (def_sym && def_sym->visible != no)
> >>>   return def_sym;
> >>>   }
> >>>
> >>
> >> The patch is missing the source file's indentation.
> >> (spaces/tabs are lost)
> >>
> > Crud! My mailer borked. I sent v2 and it looks to have kept the whitespaces.
>
> I don't think v2 worked either.
> See  https://lore.kernel.org/linux-kbuild/CAGG=3QXQkJ6n0J1gZcgxfEb68NWN2y200ZCuxxDtqPRgWPci=A@mail.gmail.com/T/#mf64c7afd19235d3dee4e572f96ff76936f921c6e
>
Goddamnit! I'll try again...

-bw

[PATCH v2] kconfig: check for a NULL pointer before access

[Bill Wendling]

The call to 'prop_get_symbol' may return NULL in some cases. The if-then
statement accesses the returned value without checking if it's
non-NULL. After inlining, the compiler may treat the conditional as
'undefined behavior', which the compiler may take the opportunity to do
whatever it wants with the UB path. This patch simply adds a check to
ensure that 'def_sym' is non-NULL to avoid this behavior.

Signed-off-by: Bill Wendling <morbo@google.com>
---
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
v2: Fix whitespaces.
---
 scripts/kconfig/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index d57f8cbba291..9c5068225328 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
  if (prop->visible.tri == no)
  continue;
  def_sym = prop_get_symbol(prop);
- if (def_sym->visible != no)
+ if (def_sym && def_sym->visible != no)
  return def_sym;
  }

-- 
2.49.0.1164.gab81da1b16-goog

[PATCH v3] kconfig: check for a NULL pointer before access

[Bill Wendling]

The call to 'prop_get_symbol' may return NULL in some cases. The if-then
statement accesses the returned value without cheecking if it's
non-NULL. After inlining, the compiler may treat the conditional as
'undefined behavior', which the compiler may take the opportunity to do
whatever it wants with the UB path. This patch simply adds a check to
ensure that 'def_sym' is non-NULL to avoid this behavior.

Signed-off-by: Bill Wendling <isanbard@gmail.com>
---
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
v3:
  - Fix whitespace for real now.
  - Patch from another email account so that the whitespace is retained.
v2:
  - Fix whitespace
---
  scripts/kconfig/symbol.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index d57f8cbba291..9c5068225328 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
  		if (prop->visible.tri == no)
  			continue;
  		def_sym = prop_get_symbol(prop);
-		if (def_sym->visible != no)
+		if (def_sym && def_sym->visible != no)
  			return def_sym;
  	}

-- 
2.49.0.1164.gab81da1b16-goog

Re: [PATCH v3] kconfig: check for a NULL pointer before access

[Randy Dunlap]

Hi Bill,

On 5/23/25 5:49 PM, Bill Wendling wrote:
> The call to 'prop_get_symbol' may return NULL in some cases. The if-then
> statement accesses the returned value without cheecking if it's
> non-NULL. After inlining, the compiler may treat the conditional as
> 'undefined behavior', which the compiler may take the opportunity to do
> whatever it wants with the UB path. This patch simply adds a check to
> ensure that 'def_sym' is non-NULL to avoid this behavior.
> 
> Signed-off-by: Bill Wendling <isanbard@gmail.com>

Acked-by: Randy Dunlap <rdunlap@infradead.org>

although see whitespace issue below...


> ---
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> v3:
>  - Fix whitespace for real now.
>  - Patch from another email account so that the whitespace is retained.
> v2:
>  - Fix whitespace
> ---
>  scripts/kconfig/symbol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index d57f8cbba291..9c5068225328 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
>          if (prop->visible.tri == no)
>              continue;
>          def_sym = prop_get_symbol(prop);
> -        if (def_sym->visible != no)
> +        if (def_sym && def_sym->visible != no)
>              return def_sym;
>      }
> 

All of these lines are still indented incorrectly in what I received,
so I downloaded the patch from
https://lore.kernel.org/linux-kbuild/27de0526-0b19-4e14-8c51-1e8b0ddcf490@gmail.com/raw

Running 'patch' (not git) on it gives me:
checking file scripts/kconfig/symbol.c
Hunk #1 FAILED at 272.
1 out of 1 hunk FAILED
done

In looking at the raw patch (link above), the non -/+ lines have
an extra space at the beginning of each line (2 spaces instead of 1).
If I remove one of those spaces, the patch applies cleanly.
Or maybe I could just tell 'patch' to ignore whitespace. Yes, that
also works.

-- 
~Randy

Re: [PATCH v3] kconfig: check for a NULL pointer before access

[Masahiro Yamada]

On Sat, May 24, 2025 at 9:49 AM Bill Wendling <isanbard@gmail.com> wrote:
>
> The call to 'prop_get_symbol' may return NULL in some cases. The if-then
> statement accesses the returned value without cheecking if it's
> non-NULL. After inlining, the compiler may treat the conditional as
> 'undefined behavior', which the compiler may take the opportunity to do
> whatever it wants with the UB path. This patch simply adds a check to
> ensure that 'def_sym' is non-NULL to avoid this behavior.
>
> Signed-off-by: Bill Wendling <isanbard@gmail.com>


Same reaction to this patch

https://lore.kernel.org/linux-kbuild/20250212154537.235297-1-ant.v.moryakov@gmail.com/


Please attach a test case
that causes a segfault with NULL pointer dereference.











> ---
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> v3:
>   - Fix whitespace for real now.
>   - Patch from another email account so that the whitespace is retained.
> v2:
>   - Fix whitespace
> ---
>   scripts/kconfig/symbol.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index d57f8cbba291..9c5068225328 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
>                 if (prop->visible.tri == no)
>                         continue;
>                 def_sym = prop_get_symbol(prop);
> -               if (def_sym->visible != no)
> +               if (def_sym && def_sym->visible != no)
>                         return def_sym;
>         }
>
> --
> 2.49.0.1164.gab81da1b16-goog
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3] kconfig: check for a NULL pointer before access

[Bill Wendling]

On Sat, May 24, 2025 at 10:08 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sat, May 24, 2025 at 9:49 AM Bill Wendling <isanbard@gmail.com> wrote:
> >
> > The call to 'prop_get_symbol' may return NULL in some cases. The if-then
> > statement accesses the returned value without cheecking if it's
> > non-NULL. After inlining, the compiler may treat the conditional as
> > 'undefined behavior', which the compiler may take the opportunity to do
> > whatever it wants with the UB path. This patch simply adds a check to
> > ensure that 'def_sym' is non-NULL to avoid this behavior.
> >
> > Signed-off-by: Bill Wendling <isanbard@gmail.com>
>
> Same reaction to this patch
>
> https://lore.kernel.org/linux-kbuild/20250212154537.235297-1-ant.v.moryakov@gmail.com/
>
I apologize for the whitespace problems. My mailer is crap and
sendmail isn't available on my local machine (it's a long story).

> Please attach a test case
> that causes a segfault with NULL pointer dereference.
>
I don't have a testcase. I discovered this while working on a Clang
feature to isolate paths with undefined behavior. (GCC already has
this pass.) The compiler notices that, after inlining, the path has
UB. It's not necessarily important whether the current compiler messes
up the code path, it's more a matter of *when* the compiler will mess
up the code path, as marking UB paths as "not executable therefore not
executed" is a common trope for some compiler developers.

-bw

[PATCH v4] kconfig: check for a NULL pointer before access

[Bill Wendling]

The call to 'prop_get_symbol' may return NULL in some cases. The if-then
statement accesses the returned value without cheecking if it's
non-NULL. After inlining, the compiler may treat the conditional as
'undefined behavior', which the compiler may take the opportunity to do
whatever it wants with the UB path. This patch simply adds a check to
ensure that 'def_sym' is non-NULL to avoid this behavior.

Signed-off-by: Bill Wendling <morbo@google.com>
---
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
v4:
  - Fix issue with patch formatting.
  - Patch is sent from original email account.
v3:
  - Fix whitespace for real now.
  - Patch from another email account so that the whitespace is retained.
v2:
  - Fix whitespace
---
 scripts/kconfig/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index d57f8cbba291..9c5068225328 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct menu *choice)
 		if (prop->visible.tri == no)
 			continue;
 		def_sym = prop_get_symbol(prop);
-		if (def_sym->visible != no)
+		if (def_sym && def_sym->visible != no)
 			return def_sym;
 	}
 
-- 
2.50.0.727.gbf7dc18ff4-goog