Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Missing bounds validation on `nid` in `node_private_register()` and `node_private_unregister()` leads to out-of-bounds reads. - [Medium] Lockless read of an `__rcu` pointer in `pgdat_is_private()` without using `rcu_access_pointer()`. --
--- Patch [4]: [PATCH v5 04/36] numa: introduce N_MEMORY_PRIVATE --- commit 69b3cbf2395b52c525233c59376e585ae83c3f31 Author: Gregory Price <[email protected]> numa: introduce N_MEMORY_PRIVATE Some devices want to hotplug their memory onto a node, but not have it exposed as "general purpose". This commit adds basic infrastructure for N_MEMORY_PRIVATE nodes, intended for memory isolated from the page allocator's normal and fallback zonelists. > diff --git a/drivers/base/node.c b/drivers/base/node.c > index 3da91929ad4e3..94cd51f51b7e8 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c [ ... ] > @@ -868,6 +869,116 @@ void register_memory_blocks_under_node_hotplug(int nid, > unsigned long start_pfn, > (void *)&nid, register_mem_block_under_node_hotplug); > return; > } > + > +static DEFINE_MUTEX(node_private_lock); > + > +/** > + * node_private_register - Register a private node > + * @nid: Node identifier [ ... ] > + */ > +int node_private_register(int nid, struct node_private *np) > +{ > + struct node_private *existing; > + pg_data_t *pgdat; > + int ret = 0; > + > + if (!np || !node_possible(nid)) > + return -EINVAL; [Severity: Medium] Could this lead to an out-of-bounds read if a driver passes an invalid nid like NUMA_NO_NODE (-1)? node_possible() and NODE_DATA() do not perform bounds checking internally. Should we verify nid >= 0 && nid < MAX_NUMNODES before using it? > + > + mutex_lock(&node_private_lock); > + mem_hotplug_begin(); > + > + /* N_MEMORY_PRIVATE and N_MEMORY are mutually exclusive */ > + if (node_state(nid, N_MEMORY)) { > + ret = -EBUSY; > + goto out; > + } > + > + pgdat = NODE_DATA(nid); [ ... ] > +/** > + * node_private_unregister - Unregister a private node > + * @nid: Node identifier [ ... ] > + */ > +int node_private_unregister(int nid) > +{ > + struct node_private *np; > + pg_data_t *pgdat; > + > + if (!node_possible(nid)) > + return 0; [Severity: Medium] Is it possible for a driver to pass NUMA_NO_NODE here as well, leading to the same out-of-bounds read issue when node_possible() and NODE_DATA() are evaluated? > + > + mutex_lock(&node_private_lock); > + mem_hotplug_begin(); > + > + pgdat = NODE_DATA(nid); [ ... ] > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index 0507193b3ae34..9815e48c03b97 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h [ ... ] > @@ -1596,12 +1598,25 @@ typedef struct pglist_data { > atomic_long_t vm_stat[NR_VM_NODE_STAT_ITEMS]; > #ifdef CONFIG_NUMA > struct memory_tier __rcu *memtier; > + struct node_private __rcu *node_private; > #endif > #ifdef CONFIG_MEMORY_FAILURE > struct memory_failure_stats mf_stats; > #endif > } pg_data_t; > > +#ifdef CONFIG_NUMA > +static inline bool pgdat_is_private(pg_data_t *pgdat) > +{ > + return !!pgdat->node_private; [Severity: Medium] Does this lockless read of the __rcu pointer require rcu_access_pointer()? Without it, sparse will likely complain about an __rcu address space violation, and we might be missing the READ_ONCE() barrier that prevents compiler load tearing. > +} > +#else > +static inline bool pgdat_is_private(pg_data_t *pgdat) > +{ > + return false; > +} > +#endif -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
