[musl] Memory Leak

[musl] Memory Leak

[Luca]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

Hi,

I believe I found a memory leak in your code.

In the file: /src/env/putenv.c

In the function: int __putenv(char *s, size_t l, char *r);

The variable `static char **oldenv` is passed to a free in line 29:
`free(oldenv);`.
The variable is a 2d pointer and therefore all contents within it should be
freed.
By freeing only oldenv all the lines of `__environ` are lost.

Possible hotfix:
```
for (int j = 0; oldenv[j]; ++j) free(oldenv[j]);
free(oldenv);
```

Thank you for your support and dedication,
Luca

[-- Attachment #2: Type: text/html, Size: 731 bytes --]

Re: [musl] Memory Leak

[Markus Wichmann]

Am Wed, Jul 17, 2024 at 06:54:59PM +0200 schrieb Luca:
> The variable `static char **oldenv` is passed to a free in line 29:
> `free(oldenv);`.
> The variable is a 2d pointer and therefore all contents within it should be
> freed.
> By freeing only oldenv all the lines of `__environ` are lost.
>
> Possible hotfix:
> ```
> for (int j = 0; oldenv[j]; ++j) free(oldenv[j]);
> free(oldenv);
> ```

No, that is invalid. You can only call free() on pointers that you own,
and that came from malloc(). The first property is not fulfilled in
putenv(). putenv() doesn't own any of the pointers given to it. It only
places them inside the environment.

And that's the end of that, really. Even those environment pointers
that were allocated are invalid to free() here because putenv() doesn't
own them. setenv() might, but that's what __env_rm_add() is for.

Also, many of the pointers in oldenv are shared with newenv. Freeing
them would leave dangling pointers in the environment list.

Ciao,
Markus