[PATCH libICE 08/13] add check for malloc

Emil Velikov emil.l.velikov at gmail.com
Tue Oct 31 14:25:03 UTC 2017


On 18 October 2017 at 17:02, walter harms <wharms at bfs.de> wrote:
>
> add check for malloc
>
> fix a potential null pointer deference error and
> release already allocated memory
>
> Signed-off-by: Walter Harms <wharms at bfs.de>
>
> ---
>  src/setauth.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/src/setauth.c b/src/setauth.c
> index 1017b02..1f3e5cc 100644
> --- a/src/setauth.c
> +++ b/src/setauth.c
> @@ -102,6 +102,19 @@ IceSetPaAuthData (
>              entries[i].auth_data_length;
I've mentioned before that strdup (effectively malloc) can fail, but
if you prefer one can keep that as follow-up series.

>         _IcePaAuthDataEntries[j].auth_data = malloc (
>              entries[i].auth_data_length);
> +       if (! _IcePaAuthDataEntries[j].auth_data)
> +         {
> +           /*
> +              we can not inform the user about a botched update
> +              but we can release the memory we already have
> +           */
> +
> +           free(_IcePaAuthDataEntries[j].protocol_name);
> +           free(_IcePaAuthDataEntries[j].network_id);
> +           free(_IcePaAuthDataEntries[j].auth_name);
> +           _IcePaAuthDataEntries[j].auth_data_length = 0;
> +           return;
> +         }
Hmm the whole function looks quite iffy - if we have an entry with
same protocol/auth name + id, we free those only to strdup them
again...?

Although in each case, if auth_data = [mc]alloc fails, we should
adjust the size/lookup. Otherwise we'll attempt to address an entry
... meh the whole auth is beyond fragile.
I'd suggest either resolving it correcting or throwing a bunch of
comments how busted it is/how to fix.

This patch just gives false sense that things are OK.
Emil


More information about the xorg-devel mailing list