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

[Abdurrahman Hussain]

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.

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,
Abdurrahman