Re: [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive

[Jason Andryuk <jason.andryuk@amd.com>]

On 2026-08-31 20:27, Borislav Petkov wrote:
> On Mon, Aug 31, 2026 at 01:52:08PM -0400, Jason Andryuk wrote:
>> No, the code and the comment are both correct as posted.  It's just that I
>> wrote the comment reversed from the code to state the positive property we
>> want.
> 
> So if amd_smn_init() succeeds, that means we have registered the PCI config
> regions of the PCI roots, including the SMN ones, as IORESOURCE_EXCLUSIVE and
> they won't be exported to userspace through /dev/mem and what not.
> 
> So, any caller of __amd_smn_rw() should fail because it is coming from kernel
> space and it can expect that userspace could interfere with the hw and thus
> not a good idea.
> 
> So yes, this is what got me confused and the (!amd_roots) check is correct.
> 
>> I'll change this whichever way you want.  It's not a big deal, which is why
>> I added the comment like you requested originally.
> 
> I think I already "intercepted" your suggestion, see here:
> 
>  From 1a1c4addc5a14e9ef2bb7d351a62ff4563d0f10a Mon Sep 17 00:00:00 2001
> From: Jason Andryuk <jason.andryuk@amd.com>
> Date: Tue, 25 Aug 2026 17:48:03 -0400
> Subject: [PATCH] x86/amd_node: Prevent potential NULL pointer dereference
> 
> amd_smn_read/write() are exported functions around __amd_smn_rw(), so
> they are always available even if amd_smn_init() fails. In that case,
> amd_roots is NULL and __amd_smn_rw() will access uninitialized memory.
> 
> Then, commit
> 
>    83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> 
> added smn_exclusive which indicated the calls to
> pci_request_config_region_exclusive() succeeded to prevent concurrent
> userspace access.
> 
> Commit
> 
>    0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
> 
> re-ordered initialization so pci_request_config_region_exclusive() is called
> earlier and a failure exits amd_smn_init() before allocating amd_roots.
> Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is
> allocated. It became redundant and can be removed.
> 
> Replace smn_exclusive with directly checking amd_roots to avoid a potential
> NULL pointer dereference.
> 
>    [ bp: Reorg commit message, touchup comment. ]
> 
> Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Cc: stable@vger.kernel.org
> Link: https://patch.msgid.link/20260825214805.39148-3-jason.andryuk@amd.com
Looks good to me.

Thanks,
Jason