Re: [PATCH v5 5/9] of/overlay: don't keep a negative id in ovcs->id on idr_alloc() failure

[Geert Uytterhoeven]

Hi Abdurrahman,

On Wed, 5 Aug 2026 at 20:47, Abdurrahman Hussain <abdurrahman@xxxxxxxxxx> wrote:
> On Wed Aug 5, 2026 at 1:26 AM PDT, Geert Uytterhoeven wrote:
> > (Alternatively, free_overlay_changeset() could check for a
> > strict-positive id)
> >
> > However, I am wondering if this is the right fix?
> >
> > All error cases above just return an error code, without doing
> > any cleanup. Failing to allocate an ID doesn't add any extra state...
> >
> > > goto err_free_ovcs;
> >
> > ... so shouldn't this just be changed to "goto out_unlock" instead?
>
> ovcs itself is already allocated at that point, and out_unlock does
> not free it, so that would leak the kzalloc. Going through
> free_overlay_changeset() is intentional: it copes with a partially
> initialized ovcs (the cset.entries.next check, the id check, and
> notify_state == OF_OVERLAY_INIT from kzalloc) and frees the struct.

Thanks, I had missed the kzalloc_obj().

> Your alternative is nicer though: the next revision makes
> free_overlay_changeset() treat only a strict-positive id as
> registered and drops the reset at the error site. Added your
> Suggested-by.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds