On Fri, Jun 20, 2025 at 12:53:00AM +0800, Alan Huang wrote:
>
> On Jun 20, 2025, at 00:37, Kent Overstreet <kent.overstr...@linux.dev> wrote:
> >
> > Don't change buf->size on error - this would usually be a transaction
> > restart, but it could also be -ENOMEM - when we've exceeded the bump
> > allocator max).
> >
> > Fixes: 247abee6ae6d ("bcachefs: btree_trans_subbuf")
> > Signed-off-by: Kent Overstreet <kent.overstr...@linux.dev>
> > ---
> > fs/bcachefs/btree_update.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/bcachefs/btree_update.c b/fs/bcachefs/btree_update.c
> > index f7949dbe8f70..668d4feb2879 100644
> > --- a/fs/bcachefs/btree_update.c
> > +++ b/fs/bcachefs/btree_update.c
> > @@ -550,19 +550,26 @@ void *__bch2_trans_subbuf_alloc(struct btree_trans
> > *trans,
> > {
> > unsigned new_top = buf->u64s + u64s;
> > unsigned old_size = buf->size;
> > + unsigned new_size = buf->size;
> >
> > - if (new_top > buf->size)
> > - buf->size = roundup_pow_of_two(new_top);
> > + BUG_ON(roundup_pow_of_two(new_top) > U16_MAX);
> >
> > - void *n = bch2_trans_kmalloc_nomemzero_ip(trans, buf->size * sizeof(u64),
> > ip);
> > + if (new_top > new_size)
> > + new_size = roundup_pow_of_two(new_top);
> > +
> > + void *n = bch2_trans_kmalloc_nomemzero_ip(trans, new_size * sizeof(u64),
> > ip);
> > if (IS_ERR(n))
> > return n;
> >
> > + unsigned offset = (u64 *) n - (u64 *) trans->mem;
> > + BUG_ON(offset > U16_MAX);
>
> offset * sizeof(u64) ?
btree_trans_subbuf fields are u16s, we're just concerned about integer
overflows here
>
> > +
> > if (buf->u64s)
> > memcpy(n,
> > btree_trans_subbuf_base(trans, buf),
> > old_size * sizeof(u64));
>
> copy buf->u64s is enough.
yup, already spotted that
>
> > buf->base = (u64 *) n - (u64 *) trans->mem;
> > + buf->size = new_size;
> >
> > void *p = btree_trans_subbuf_top(trans, buf);
> > buf->u64s = new_top;
> > --
> > 2.50.0
> >
>
