mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr
@ 2026-07-30 13:02 Thorsten Blum
  2026-09-25  6:19 ` Sourabh Jain
  0 siblings, 1 reply; 5+ messages in thread
From: Thorsten Blum @ 2026-07-30 13:02 UTC (permalink / raw)
  To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy (CS GROUP),
	Kees Cook, Gustavo A. R. Silva, Sourabh Jain, Aditya Gupta,
	Hari Bathini
  Cc: Thorsten Blum, linuxppc-dev, linux-kernel, linux-hardening

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;
 
 	return tbuf;
 }
@@ -288,13 +290,13 @@ static int update_usable_mem_fdt(void *fdt, struct crash_mem *usable_mem)
 		return -EINVAL;
 	}
 
-	um_info.buf  = NULL;
 	um_info.size = 0;
 	um_info.max_entries = 0;
-	um_info.idx  = 0;
+	um_info.buf = NULL;
+	um_info.idx = 0;
 	/* Memory ranges to look up */
-	um_info.ranges = &(usable_mem->ranges[0]);
 	um_info.nr_ranges = usable_mem->nr_ranges;
+	um_info.ranges = usable_mem->ranges;
 
 	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
 	if (dn) {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr
  2026-07-30 13:02 [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr Thorsten Blum
@ 2026-09-25  6:19 ` Sourabh Jain
  2026-10-03  8:53   ` Thorsten Blum
  0 siblings, 1 reply; 5+ messages in thread
From: Sourabh Jain @ 2026-09-25  6:19 UTC (permalink / raw)
  To: Thorsten Blum, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Kees Cook, Gustavo A. R. Silva, Aditya Gupta, Hari Bathini
  Cc: linuxppc-dev, linux-kernel, linux-hardening



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?

- Sourabh Jain


>   
>   	return tbuf;
>   }
> @@ -288,13 +290,13 @@ static int update_usable_mem_fdt(void *fdt, struct crash_mem *usable_mem)
>   		return -EINVAL;
>   	}
>   
> -	um_info.buf  = NULL;
>   	um_info.size = 0;
>   	um_info.max_entries = 0;
> -	um_info.idx  = 0;
> +	um_info.buf = NULL;
> +	um_info.idx = 0;
>   	/* Memory ranges to look up */
> -	um_info.ranges = &(usable_mem->ranges[0]);
>   	um_info.nr_ranges = usable_mem->nr_ranges;
> +	um_info.ranges = usable_mem->ranges;
>   
>   	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>   	if (dn) {
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr
  2026-09-25  6:19 ` Sourabh Jain
@ 2026-10-03  8:53   ` Thorsten Blum
  2026-10-05  8:12     ` Sourabh Jain
  0 siblings, 1 reply; 5+ messages in thread
From: Thorsten Blum @ 2026-10-03  8:53 UTC (permalink / raw)
  To: Sourabh Jain
  Cc: Thorsten Blum, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Kees Cook, Gustavo A. R. Silva, Aditya Gupta, Hari Bathini,
	linuxppc-dev, linux-kernel, linux-hardening

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.

And ->size is moved up because ->max_entries depends on it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr
  2026-10-03  8:53   ` Thorsten Blum
@ 2026-10-05  8:12     ` Sourabh Jain
  2026-10-05  8:26       ` Thorsten Blum
  0 siblings, 1 reply; 5+ messages in thread
From: Sourabh Jain @ 2026-10-05  8:12 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: Thorsten Blum, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Kees Cook, Gustavo A. R. Silva, Aditya Gupta, Hari Bathini,
	linuxppc-dev, linux-kernel, linux-hardening



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

- Sourabh Jain

> And ->size is moved up because ->max_entries depends on it.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr
  2026-10-05  8:12     ` Sourabh Jain
@ 2026-10-05  8:26       ` Thorsten Blum
  0 siblings, 0 replies; 5+ messages in thread
From: Thorsten Blum @ 2026-10-05  8:26 UTC (permalink / raw)
  To: Sourabh Jain
  Cc: Thorsten Blum, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Kees Cook, Gustavo A. R. Silva, Aditya Gupta, Hari Bathini,
	linuxppc-dev, linux-kernel, linux-hardening

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?

Thanks,
Thorsten

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-05  8:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-30 13:02 [PATCH] powerpc/kexec: Annotate umem_info members with __counted_by_ptr Thorsten Blum
2026-09-25  6:19 ` Sourabh Jain
2026-10-03  8:53   ` Thorsten Blum
2026-10-05  8:12     ` Sourabh Jain
2026-10-05  8:26       ` Thorsten Blum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®