Re: [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 10/5/26 17:26, Thorsten Blum wrote:
> On Mon, Oct 05, 2026 at 01:42:01PM +0530, Sourabh Jain wrote:
>> On 03/10/26 14:23, Thorsten Blum wrote:
>>> On Fri, Sep 25, 2026 at 11:49:04AM +0530, Sourabh Jain wrote:
>>>> On 30/07/26 18:32, Thorsten Blum wrote:
>>>>> Add __counted_by_ptr() to umem_info::buf and umem_info::ranges to
>>>>> improve access bounds checking via CONFIG_UBSAN_BOUNDS and
>>>>> CONFIG_FORTIFY_SOURCE.
>>>>>
>>>>> Set the count fields before assigning the corresponding pointers, return
>>>>> early on krealloc() failure, and use sizeof(*buf) when deriving
>>>>> max_entries from the allocation size.
>>>>>
>>>>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>>>>> ---
>>>>>     arch/powerpc/kexec/file_load_64.c | 22 ++++++++++++----------
>>>>>     1 file changed, 12 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>>>>> index 8c72e12ea44e..6424b668f0e9 100644
>>>>> --- a/arch/powerpc/kexec/file_load_64.c
>>>>> +++ b/arch/powerpc/kexec/file_load_64.c
>>>>> @@ -34,14 +34,15 @@
>>>>>     #include <asm/cputhreads.h>
>>>>>     struct umem_info {
>>>>> -	__be64 *buf;		/* data buffer for usable-memory property */
>>>>> +	/* data buffer for usable-memory property */
>>>>> +	__be64 *buf __counted_by_ptr(max_entries);
>>>>>     	u32 size;		/* size allocated for the data buffer */
>>>>>     	u32 max_entries;	/* maximum no. of entries */
>>>>>     	u32 idx;		/* index of current entry */
>>>>>     	/* usable memory ranges to look up */
>>>>>     	unsigned int nr_ranges;
>>>>> -	const struct range *ranges;
>>>>> +	const struct range *ranges __counted_by_ptr(nr_ranges);
>>>>>     };
>>>> [...]
>>>>>     const struct kexec_file_ops * const kexec_file_loaders[] = {
>>>>> @@ -83,11 +84,12 @@ static __be64 *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
>>>>>     	new_size = um_info->size + MEM_RANGE_CHUNK_SZ;
>>>>>     	tbuf = krealloc(um_info->buf, new_size, GFP_KERNEL);
>>>>> -	if (tbuf) {
>>>>> -		um_info->buf = tbuf;
>>>>> -		um_info->size = new_size;
>>>>> -		um_info->max_entries = (um_info->size / sizeof(u64));
>>>>> -	}
>>>>> +	if (!tbuf)
>>>>> +		return NULL;
>>>>> +
>>>>> +	um_info->size = new_size;
>>>>> +	um_info->max_entries = um_info->size / sizeof(*um_info->buf);
>>>>> +	um_info->buf = tbuf;
>>>>
>>>> Could you please explain why size and max_entries are updated before the
>>>> buffer itself?
>>> __counted_by_ptr() requires the counter ->max_entries to be set before
>>> the ->buf pointer is assigned; otherwise you have an inconsistent state
>>> where the counter doesn't match the pointer.
>>
>> But isn't updating the counter holding the buffer size before the actual
>> buffer
>> can cause problems?
>>
>> Can you share the document link of __counted_by_ptr() which says that size
>> counter to be
>> updated before the buffer pointer.
>>
>> Here is an example in fs/coredump.c file where size counter is updated after
>> the buffer
>> with __counter_by_ptr():
>>
>> https://github.com/torvalds/linux/blob/a90ee4305c4a5df72c11b31dacfdc76e00fcf78a/fs/coredump.c#L98
>> https://github.com/torvalds/linux/blob/a90ee4305c4a5df72c11b31dacfdc76e00fcf78a/fs/coredump.c#L115
> 
> Maybe the order only matters for flexible arrays and __counted_by(), but
> not for __counted_by_ptr(). I reordered it mostly out of habit from
> __counted_by() annotations and assumed the same rules apply for
> __counted_by_ptr(). Happy to restore the old order if it's not required.
> 
> Kees or Gustavo, what's your take on this?

The order matters when dereferencing the pointer. In this case, it's just
an assignment.

Thanks
-Gustavo