arch/x86/include/asm/amd_nb.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
From: Arnd Bergmann <arnd@arndb.de>
node_to_amd_nb() is defined to NULL in non-AMD configs:
drivers/platform/x86/amd/hsmp/plat.c: In function 'init_platform_device':
drivers/platform/x86/amd/hsmp/plat.c:165:68: error: dereferencing 'void *' pointer [-Werror]
165 | sock->root = node_to_amd_nb(i)->root;
| ^~
drivers/platform/x86/amd/hsmp/plat.c:165:68: error: request for member 'root' in something not a structure or union
Change the definition to something that builds. This does introduce a
NULL pointer dereference but the code is never called since the driver
won't probe successfully.
Fixes: 7d3135d16356 ("platform/x86/amd/hsmp: Create separate ACPI, plat and common drivers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/x86/include/asm/amd_nb.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 6f3b6aef47ba..d0caac26533f 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -116,7 +116,10 @@ static inline bool amd_gart_present(void)
#define amd_nb_num(x) 0
#define amd_nb_has_feature(x) false
-#define node_to_amd_nb(x) NULL
+static inline struct amd_northbridge *node_to_amd_nb(int node)
+{
+ return NULL;
+}
#define amd_gart_present(x) false
#endif
--
2.39.5On Tue, 29 Oct 2024, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> node_to_amd_nb() is defined to NULL in non-AMD configs:
>
> drivers/platform/x86/amd/hsmp/plat.c: In function 'init_platform_device':
> drivers/platform/x86/amd/hsmp/plat.c:165:68: error: dereferencing 'void *' pointer [-Werror]
> 165 | sock->root = node_to_amd_nb(i)->root;
> | ^~
> drivers/platform/x86/amd/hsmp/plat.c:165:68: error: request for member 'root' in something not a structure or union
>
> Change the definition to something that builds. This does introduce a
> NULL pointer dereference but the code is never called since the driver
> won't probe successfully.
I don't like this very wording because what the code very much does is
NULL check on node_to_amd_nb() which leads to immediate failure of
.probe(). (We don't call other deferences after a NULL check "NULL pointer
dereference" either so none is introduced by this patch, IMO.)
With that fixed,
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
> Fixes: 7d3135d16356 ("platform/x86/amd/hsmp: Create separate ACPI, plat and common drivers")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/x86/include/asm/amd_nb.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index 6f3b6aef47ba..d0caac26533f 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -116,7 +116,10 @@ static inline bool amd_gart_present(void)
>
> #define amd_nb_num(x) 0
> #define amd_nb_has_feature(x) false
> -#define node_to_amd_nb(x) NULL
> +static inline struct amd_northbridge *node_to_amd_nb(int node)
> +{
> + return NULL;
> +}
> #define amd_gart_present(x) false
>
> #endif
>
On Tue, Oct 29, 2024 at 02:39:41PM +0200, Ilpo Järvinen wrote:
> I don't like this very wording because what the code very much does is
> NULL check on node_to_amd_nb() which leads to immediate failure of
> .probe(). (We don't call other deferences after a NULL check "NULL pointer
> dereference" either so none is introduced by this patch, IMO.)
I was wondering that too: where does this line
sock->root = node_to_amd_nb(i)->root;
quoted by gcc come from?
IOW, what is the correct Fixes: tag?
The commit 7d3135d16356 ("platform/x86/amd/hsmp: Create separate ACPI, plat and common drivers
is only in next AFAICT, so I'll drop the Fixes: tag when sending...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, 29 Oct 2024, Borislav Petkov wrote:
> On Tue, Oct 29, 2024 at 02:39:41PM +0200, Ilpo Järvinen wrote:
> > I don't like this very wording because what the code very much does is
> > NULL check on node_to_amd_nb() which leads to immediate failure of
> > .probe(). (We don't call other deferences after a NULL check "NULL pointer
> > dereference" either so none is introduced by this patch, IMO.)
>
> I was wondering that too: where does this line
>
> sock->root = node_to_amd_nb(i)->root;
>
> quoted by gcc come from?
>
> IOW, what is the correct Fixes: tag?
>
> The commit 7d3135d16356 ("platform/x86/amd/hsmp: Create separate ACPI, plat and common drivers
>
> is only in next AFAICT, so I'll drop the Fixes: tag when sending...
To clarify,
The assignment line is old (from 287a821c76be8 or even before that in
some form which would have not lead to compiler warning though).
It's the COMPILE_TEST that got enabled in 7d3135d16356, before that hsmp
depended on AMD_NB so the condition could never trigger.
--
i.
On Tue, Oct 29, 2024 at 05:19:30PM +0200, Ilpo Järvinen wrote:
> It's the COMPILE_TEST that got enabled in 7d3135d16356, before that hsmp
> depended on AMD_NB so the condition could never trigger.
This COMPILE_TEST thing is only causing more trouble than it is solving... :-\
Lemme fixup Arnd's commit message and queue it.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Oct 29, 2024 at 09:23:20AM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> node_to_amd_nb() is defined to NULL in non-AMD configs:
>
> drivers/platform/x86/amd/hsmp/plat.c: In function 'init_platform_device':
> drivers/platform/x86/amd/hsmp/plat.c:165:68: error: dereferencing 'void *' pointer [-Werror]
> 165 | sock->root = node_to_amd_nb(i)->root;
> | ^~
> drivers/platform/x86/amd/hsmp/plat.c:165:68: error: request for member 'root' in something not a structure or union
>
> Change the definition to something that builds. This does introduce a
> NULL pointer dereference but the code is never called since the driver
> won't probe successfully.
>
> Fixes: 7d3135d16356 ("platform/x86/amd/hsmp: Create separate ACPI, plat and common drivers")
^^^^^^^^^^^^
I'm guessing this is queued for 6.13...
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/x86/include/asm/amd_nb.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index 6f3b6aef47ba..d0caac26533f 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -116,7 +116,10 @@ static inline bool amd_gart_present(void)
>
> #define amd_nb_num(x) 0
> #define amd_nb_has_feature(x) false
> -#define node_to_amd_nb(x) NULL
> +static inline struct amd_northbridge *node_to_amd_nb(int node)
> +{
> + return NULL;
> +}
> #define amd_gart_present(x) false
... so this fix should go to Linus now so that build testing doesn't break?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Oct 29, 2024, at 10:33, Borislav Petkov wrote:
> On Tue, Oct 29, 2024 at 09:23:20AM +0000, Arnd Bergmann wrote:
>>
>> #define amd_nb_num(x) 0
>> #define amd_nb_has_feature(x) false
>> -#define node_to_amd_nb(x) NULL
>> +static inline struct amd_northbridge *node_to_amd_nb(int node)
>> +{
>> + return NULL;
>> +}
>> #define amd_gart_present(x) false
>
> ... so this fix should go to Linus now so that build testing doesn't break?
>
That would work, or Ilpo can pick it into the tree that
has the driver changes, possibly folding the fix into the
other changes.
Arnd
On Tue, Oct 29, 2024 at 10:56:12AM +0000, Arnd Bergmann wrote:
> That would work, or Ilpo can pick it into the tree that
> has the driver changes, possibly folding the fix into the
> other changes.
Right, while it shouldn't be a problem, I'd like to keep all amd_nb changes in
one tree because we have stuff in-flight touching and changing exactly that
area so...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, 29 Oct 2024, Borislav Petkov wrote: > On Tue, Oct 29, 2024 at 10:56:12AM +0000, Arnd Bergmann wrote: > > That would work, or Ilpo can pick it into the tree that > > has the driver changes, possibly folding the fix into the > > other changes. > > Right, while it shouldn't be a problem, I'd like to keep all amd_nb changes in > one tree because we have stuff in-flight touching and changing exactly that > area so... Hi Boris, I'm unfortunately left a bit unsure what exactly is your suggestion here, so could you please elaborate? (I'm assuming the big amd_nb series will go through the x86 tree.) Thanks in advance. -- i.
Hi,
On Tue, Oct 29, 2024 at 02:40:00PM +0200, Ilpo Järvinen wrote:
> I'm unfortunately left a bit unsure what exactly is your suggestion here,
> so could you please elaborate?
My suggestion is, I send it to Linus now so it appears in 6.12-rc6 and thus
the build error is gone in every other tree.
> (I'm assuming the big amd_nb series will go through the x86 tree.)
Yeah, eventually. Not now but see above: if the fix appears in mainline now,
no problems.
Right?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, 29 Oct 2024, Borislav Petkov wrote: > On Tue, Oct 29, 2024 at 02:40:00PM +0200, Ilpo Järvinen wrote: > > I'm unfortunately left a bit unsure what exactly is your suggestion here, > > so could you please elaborate? > > My suggestion is, I send it to Linus now so it appears in 6.12-rc6 and thus > the build error is gone in every other tree. > > > (I'm assuming the big amd_nb series will go through the x86 tree.) > > Yeah, eventually. Not now but see above: if the fix appears in mainline now, > no problems. > > Right? Yes, I'm fine with you sending it to Linus in this cycle. Thanks. -- i.
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: fce9642c765a18abd1db0339a7d832c29b68456a
Gitweb: https://git.kernel.org/tip/fce9642c765a18abd1db0339a7d832c29b68456a
Author: Arnd Bergmann <arnd@arndb.de>
AuthorDate: Tue, 29 Oct 2024 09:23:20
Committer: Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Tue, 29 Oct 2024 18:16:05 +01:00
x86/amd_nb: Fix compile-testing without CONFIG_AMD_NB
node_to_amd_nb() is defined to NULL in non-AMD configs:
drivers/platform/x86/amd/hsmp/plat.c: In function 'init_platform_device':
drivers/platform/x86/amd/hsmp/plat.c:165:68: error: dereferencing 'void *' pointer [-Werror]
165 | sock->root = node_to_amd_nb(i)->root;
| ^~
drivers/platform/x86/amd/hsmp/plat.c:165:68: error: request for member 'root' in something not a structure or union
Users of the interface who also allow COMPILE_TEST will cause the above build
error so provide an inline stub to fix that.
[ bp: Massage commit message. ]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20241029092329.3857004-1-arnd@kernel.org
---
arch/x86/include/asm/amd_nb.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 6f3b6ae..d0caac2 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -116,7 +116,10 @@ static inline bool amd_gart_present(void)
#define amd_nb_num(x) 0
#define amd_nb_has_feature(x) false
-#define node_to_amd_nb(x) NULL
+static inline struct amd_northbridge *node_to_amd_nb(int node)
+{
+ return NULL;
+}
#define amd_gart_present(x) false
#endif© 2016 - 2026 Red Hat, Inc.