Re: [PATCH v8 7/7] x86/hygon: Add Family 0x18 SMN access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] x86/hygon: unreferenced reserved_roots allocation in smn_setup()
--

commit 9932a3551c6b7f805f9889c0454dbe666f4f46a0
Author: Lin Wang <wanglin@open-hieco.net>

x86/hygon: Add Family 0x18 SMN access

This commit adds system management network access support for Hygon
Family 0x18 processors by mapping data fabric nodes to per-socket root
complexes and providing serialized read and write helpers.

> diff --git a/arch/x86/kernel/hygon_node.c b/arch/x86/kernel/hygon_node.c
> index 03c723aab97e3..7b0a18cd7027a 100644
> --- a/arch/x86/kernel/hygon_node.c
> +++ b/arch/x86/kernel/hygon_node.c
[ ... ]
> @@ -629,6 +631,229 @@ static int __maybe_unused hygon_pci_dev_to_df_node(struct pci_dev *pdev)
[ ... ]
> +static struct pci_dev	**smn_roots;
> +static struct pci_dev	**smn_reserved_roots;
> +static u16		smn_num_nodes;
> +static DEFINE_MUTEX(smn_mutex);
[ ... ]
> +static int __init smn_setup(void)
> +{
> +	u16 num_roots, roots_per_socket, node, num_nodes, num_sockets, reserved;
> +	struct pci_dev **reserved_roots, **roots;
> +	struct pci_dev *root = NULL;
> +	u8 socket_id;
> +	int ret;
[ ... ]
> +	roots = kzalloc_objs(*roots, num_nodes);
> +	if (!roots)
> +		return -ENOMEM;
> +
> +	reserved_roots = kzalloc_objs(*reserved_roots, num_roots);
> +	if (!reserved_roots) {
> +		kfree(roots);
> +		return -ENOMEM;
> +	}
[ ... ]
> +	for (node = 0; node < num_nodes; node++) {
> +		socket_id = df_cache.nodes[node].socket_id;
> +
> +		if (socket_id >= num_sockets) {
> +			ret = -ENODEV;
> +			goto err_release;
> +		}
> +
> +		roots[node] = reserved_roots[socket_id * roots_per_socket];
> +
> +		pci_dbg(roots[node],
> +			"is root for Hygon node %u (socket %u)\n",
> +			node, socket_id);
> +	}
> +
> +	smn_reserved_roots = reserved_roots;
> +	smn_num_nodes = num_nodes;
> +	smn_roots = roots;
> +	return 0;

[Severity: Medium]
Does df_node_init() -> smn_setup() in arch/x86/kernel/hygon_node.c leak the
reserved_roots array once initialization completes?

In smn_setup(), reserved_roots is allocated with kzalloc_objs() to track
reserved root devices for error rollback via release_reserved_roots() and
to populate roots[node]. At the end of smn_setup(), reserved_roots is
assigned to the file-scope static variable smn_reserved_roots, which is
never read anywhere in arch/x86/kernel/hygon_node.c.

Because smn_reserved_roots is a write-only static variable with internal
linkage, the compiler optimizes away both the variable and the store to it.
Once smn_setup() returns, no reference to the reserved_roots allocation
remains in memory (as smn_roots only holds pointers to the individual
pci_dev objects), which leaves the array leaked and triggers a kmemleak
report.

Since the device references from pci_dev_get() and the config space
reservations from pci_request_config_region_exclusive() are held by the
pci_dev objects themselves, could smn_reserved_roots be removed and
reserved_roots freed with kfree(reserved_roots) before smn_setup() returns
0?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022074218.667049-1-wanglin@open-hieco.net?part=7