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 the 'smn_exclusive' flag, 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'. The setting of '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 fix a potential NULL pointer dereference and to simplify the logic. [ bp: Reorg commit message, touchup comment. ] [ mingo: Rebase & further touchups. ]
Unified · Split
| @@ -38,7 +38,6 @@ static struct pci_dev **amd_roots; · 37 unmodified lines | |||
| 38 | 38 | ||
| 39 | /* Protect the PCI config register pairs used for SMN. */ | 39 | /* Protect the PCI config register pairs used for SMN. */ |
| 40 | static DEFINE_MUTEX(smn_mutex); | 40 | static DEFINE_MUTEX(smn_mutex); |
| 41 | static bool smn_exclusive; | ||
| 42 | 41 | ||
| 43 | #define SMN_INDEX_OFFSET 0x60 | 42 | #define SMN_INDEX_OFFSET 0x60 |
| 44 | #define SMN_DATA_OFFSET 0x64 | 43 | #define SMN_DATA_OFFSET 0x64 |
| @@ -91,11 +90,16 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b · 46 unmodified lines | |||
| 91 | if (node >= amd_num_nodes()) | 90 | if (node >= amd_num_nodes()) |
| 92 | return err; | 91 | return err; |
| 93 | 92 | ||
| 94 | root = amd_roots[node]; | 93 | /* |
| 95 | if (!root) | 94 | * Uninitialized amd_roots indicates pci_request_config_region_exclusive() |
| 95 | * didn't run or failed and thus the kernel cannot rely on having | ||
| 96 | * exclusive access to SMN registers so prevent that. | ||
| 97 | */ | ||
| 98 | if (!amd_roots) | ||
| 96 | return err; | 99 | return err; |
| 97 | 100 | ||
| 98 | if (!smn_exclusive) | 101 | root = amd_roots[node]; |
| 102 | if (!root) | ||
| 99 | return err; | 103 | return err; |
| 100 | 104 | ||
| 101 | guard(mutex)(&smn_mutex); | 105 | guard(mutex)(&smn_mutex); |
| @@ -313,8 +317,6 @@ static int __init amd_smn_init(void) · 211 unmodified lines | |||
| 313 | debugfs_create_file("value", 0600, debugfs_dir, NULL, &smn_value_fops); | 317 | debugfs_create_file("value", 0600, debugfs_dir, NULL, &smn_value_fops); |
| 314 | } | 318 | } |
| 315 | 319 | ||
| 316 | smn_exclusive = true; | ||
| 317 | |||
| 318 | return 0; | 320 | return 0; |
| 319 | } | 321 | } |
| 320 | 322 | ||