Re: [PATCH v12 06/12] spmi: Add helper to get a parent SPMI device

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

On 7/7/26 13:27, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Time-of-check to time-of-use (TOCTOU) race on `dev->parent` can lead to a NULL pointer dereference.
> --
> 
> --- Patch [6]: [PATCH v12 06/12] spmi: Add helper to get a parent SPMI device ---
> commit fb3218d6b528eabdc83ac90ed869685f4b9bd99e
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> spmi: Add helper to get a parent SPMI device
> 
> This commit adds a new helper function spmi_get_parent_spmi_device()
> which takes a struct device and returns a pointer to its parent SPMI
> device if one exists, without modifying the reference count.
> 
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index e218c81405458..a0a6ea9b1d2db 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
> 
> [ ... ]
> 
>> @@ -433,6 +433,29 @@ struct spmi_device *spmi_find_device_by_of_node(struct device_node *np)
>>   }
>>   EXPORT_SYMBOL_GPL(spmi_find_device_by_of_node);
>>   
>> +/**
>> + * spmi_get_parent_spmi_device() - get the parent SPMI device from current dev
>> + * @dev:        pointer to a subdevice on SPMI bus
>> + *
>> + * Checks if the passed device is a child of an SPMI device and returns a
>> + * handle to the parent SPMI device without incrementing any refcount.
>> + *
>> + * Return: Handle to the parent SPMI device or NULL
>> + */
>> +struct spmi_device *spmi_get_parent_spmi_device(struct device *dev)
>> +{
>> +	struct device *parent;
>> +
>> +	if (dev && dev->parent) {
>> +		parent = dev->parent;
>> +
>> +		if (parent->type == &spmi_dev_type)
> 
> [Severity: High]
> Can a concurrent device topology change (like device_move()) cause a NULL
> pointer dereference here?
> 

That's not happening.

> If dev->parent is modified to NULL between the check in the if condition
> and the assignment to the local parent variable, parent will become NULL
> and this parent->type check will panic.
> 
> Since the compiler is permitted to emit two separate memory loads for
> dev->parent in this sequence, would it be safer to read it exactly once
> into the local variable before checking its validity?
> 

Yes, but it's overcautious, and purely a nitpick. Not severe at all, and I'm not
fixing that as it's still safe.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy