Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <ee45aa412acaf7a2c035ad98e966394a7293dd9f.camel@redhat.com> <e0e036cb4e438fdeb5ed1b8a988dcb170907f012.camel@redhat.com>
In-Reply-To: <e0e036cb4e438fdeb5ed1b8a988dcb170907f012.camel@redhat.com>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Fri, 6 Aug 2021 15:14:59 -0400
Message-ID: <CAN-5tyFv58duZSrFW9-DtCnA5DQ8j6FayFKvWqM9LvqL0Njj7Q@mail.gmail.com>
Subject: Re: [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap
To:     Alice Mitchell <ajmitchell@redhat.com>
Cc:     linux-nfs <linux-nfs@vger.kernel.org>,
        Steve Dickson <steved@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Aug 6, 2021 at 12:21 PM Alice Mitchell <ajmitchell@redhat.com> wrote:
>
>
> Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
> ---
>  support/nfsidmap/nss.c   | 4 ++--
>  support/nfsidmap/regex.c | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
> index 669760b..f00bd9b 100644
> --- a/support/nfsidmap/nss.c
> +++ b/support/nfsidmap/nss.c
> @@ -365,9 +365,9 @@ static int _nss_name_to_gid(char *name, gid_t *gid,
> int dostrip)
>  out_buf:
>         free(buf);
>  out_name:
> -       if (dostrip)
> +       if (localname)
>                 free(localname);
> -       if (get_reformat_group())
> +       if (ref_name)
>                 free(ref_name);

Do we even need to check for null before freeing these days? man page
says if null is passed then it's a no-op.

If we are not allowed to free a null then there is another patch in
the series in the mountd code that does intentionally do a free of
null pointers.

>  out:
>         return err;
> diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c
> index fdbb2e2..958b4ac 100644
> --- a/support/nfsidmap/regex.c
> +++ b/support/nfsidmap/regex.c
> @@ -157,6 +157,7 @@ again:
>         IDMAP_LOG(4, ("regexp_getpwnam: name '%s' mapped to '%s'",
>                   name, localname));
>
> +       free(localname);
>         *err_p = 0;
>         return pw;
>
> --
> 2.27.0
>
>