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 <[email protected]>
> > > > ---
> > > >    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

Reply via email to