[PATCH 2/2] stdlib: Re-implement free (environ) compatibility kludge for setenv

[Sam James]

Florian Weimer <fweimer@redhat.com> writes:

> For the originally failing application (userhelper from usermode),
> it is not actually necessary to call realloc on the environ
> pointer.  Yes, there will be a memory leak because the application
> assigns a heap-allocated pointer to environ that it never frees,
> but this leak was always there: the old realloc-based setenv had
> a hidden internal variable, last_environ, that was used in a similar
> way to __environ_array_list.  The application is not impacted by
> the leak anyway because the relevant operations do not happen in
> a loop.
>
> The change here just uses a separte heap allocation and points
> environ to that.  This means that if an application calls
> free (environ) and restores the environ pointer to the value
> at process start, and does not modify the environment further,
> nothing bad happens.
>
> This change should not invalidate any previous testing that went into
> the original getenv thread safety change, commit 7a61e7f557a97ab597d6
> ("stdlib: Make getenv thread-safe in more cases").
>
> The new test cases are modeled in part on the env -i use case from
> bug 32588 (with !DO_MALLOC && !DO_EARLY_SETENV), and the previous
> stdlib/tst-setenv-malloc test.  The DO_MALLOC && !DO_EARLY_SETENV
> case in the new test should approximate what userhelper from the
> usermode package does.
>
> Tested on x86_64-linux-gnu, and with an unpatched userhelper binary from
> usermode.

Thanks for the quick revert and followup. I don't feel able to review it
but hopefully carlos or azanella can take a look. I am still a bit
worried about impact in case there's worse edge-cases we haven't seen
yet but that's speculation (I agree with you that the previous state is
pretty well-tested and we shouldn't need to revert that).