[PATCH] ti: sci: Drop fake 'const' on handle pointer
Andrew Davis
afd at ti.com
Thu Mar 5 07:52:46 PST 2026
On 3/5/26 8:59 AM, Krzysztof Kozlowski wrote:
> On 02/03/2026 20:12, Andrew Davis wrote:
>> On 2/23/26 2:24 PM, Krzysztof Kozlowski wrote:
>>> All the functions operating on the 'handle' pointer are claiming it is a
>>> pointer to const thus they should not modify the handle. In fact that's
>>> a false statement, because first thing these functions do is drop the
>>> cast to const with container_of:
>>>
>>> struct ti_sci_info *info = handle_to_ti_sci_info(handle);
>>>
>>> And with such cast the handle is easily writable with simple:
>>>
>>> info->handle.version.abi_major = 0;
>>>
>>
>> The const is for all the consumers drivers of the handle. Those
>> consumers cannot do the above becouse both handle_to_ti_sci_info()
>> and struct ti_sci_info itself are only defined inside ti_sci.c.
>>
>>> The code is not correct logically, either, because functions like
>>> ti_sci_get_handle() and ti_sci_put_handle() are meant to modify the
>>> handle reference counting, thus they must modify the handle.
>>
>> The reference counting is handled outside of the ti_sci_handle struct,
>> the contents of the handle are never modified after it is created.
>>
>> The const is only added by functions return a handle to consumers.
>> We cannot return non-const to consumer drivers or then they would
>> be able to modify the content without a compiler warning, which would
>> be a real problem.
>
> This is the same argument as making pointer to const the pointer freed
> via kfree() (or free() in userspace). kfree() does not modify the
> contents of the pointer, right? The same as getting putting handle does
> not modify the handle...
>
In that argument, if we wanted the consumer of the pointer to not free()
it we would return a const pointer, free()'ing that would result in the
warning we want (discards const qualifier).
If you could somehow malloc() from a const area in memory then free()
doesn't modify the pointed to values, only the non-const record keeping
which would be stored outside of the const memory. So even in this analogy
there isn't a problem.
> The point is that storing the reference counter outside of handle does
> not make the argument correct. Logically when you get a reference, you
> increase the counter, so it is not a pointer to const. And the code
> agrees, because you must drop the const.
>
The record keeping memory is not const and can be modified.
And where do we drop the const? The outer "struct ti_sci_info" was never
const to begin with, so no dropped const.
If the issue is that the handle is not const inside that outer struct
we could fix that,
struct ti_sci_info {
...
- struct ti_sci_handle handle;
+ const struct ti_sci_handle handle;
...
};
And with that change even your original commit message example issue
goes away,
struct ti_sci_info *info = handle_to_ti_sci_info(handle);
info->handle.version.abi_major = 0;
would now fail to work to compile.
Andrew
>
>>
>> Andrew
>>
>>> Modification here happens anyway, even if the reference counting is
>>> stored in the container which the handle is part of.
>>>
>>> The code does not have actual visible bug, but incorrect 'const'
>>> annotations could lead to incorrect compiler decisions.
>>>
>
>
> Please kindly trim the replies from unnecessary context. It makes it
> much easier to find new content.
>
>
> Best regards,
> Krzysztof
More information about the linux-arm-kernel
mailing list