All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Aurélien Aptel" <aaptel@suse.com>
To: Zhiqiang Liu <liuzhiqiang26@huawei.com>,
	smfrench@gmail.com, liujiawen10@huawei.com,
	pshilov@microsoft.com, kdsouza@redhat.com, lsahlber@redhat.com,
	ab@samba.org, palcantara@suse.de, linux-cifs@vger.kernel.org
Cc: dujin1@huawei.com, Mingfangsen <mingfangsen@huawei.com>,
	zhangsaisai <zhangsaisai@huawei.com>
Subject: Re: [PATCH cifs-utils] mount.cifs.c: fix memory leaks in main func
Date: Thu, 01 Aug 2019 11:15:53 +0200	[thread overview]
Message-ID: <87h871s0ty.fsf@suse.com> (raw)
In-Reply-To: <d4bf65ab-42e1-606c-be35-a5cb3b7b77b0@huawei.com>

Hi Zhiqiang,

You are on the right list :)

Unfortunately it seems you have sent the exact same patch as before so
I'll post my comments again:

Zhiqiang Liu <liuzhiqiang26@huawei.com> writes:
> index ae7a899..029f01a 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -1830,6 +1830,7 @@ assemble_mountinfo(struct parsed_mount_info *parsed_info,
>  	}
>
>  assemble_exit:
> +	free(orgoptions);
>  	return rc;
>  }

Since orgoptions is allocated in main() you should also free it
there. In fact it is already freed there so the return have to be
changed to goto.

>
> @@ -1994,8 +1995,11 @@ int main(int argc, char **argv)
>
>  	/* chdir into mountpoint as soon as possible */
>  	rc = acquire_mountpoint(&mountpoint);
> -	if (rc)
> +	if (rc) {
> +		free(mountpoint);
> +		free(orgoptions);
>  		return rc;
> +	}

Since mountpoint is allocated in acquire_mountpoint() you should free it
there if there's an error.

>  	/*
>  	 * mount.cifs does privilege separation. Most of the code to handle
> @@ -2014,6 +2018,7 @@ int main(int argc, char **argv)
>  		/* child */
>  		rc = assemble_mountinfo(parsed_info, thisprogram, mountpoint,
>  					orig_dev, orgoptions);
> +		free(mountpoint);

Since this code block is only run by the child I think it's ok to not
use goto. Don't forget to free(orgoptions) if you remove it from
assemble_mountinfo()

>  		return rc;
>  	} else {
>  		/* parent */
> @@ -2149,5 +2154,6 @@ mount_exit:
>  	}
>  	free(options);
>  	free(orgoptions);
> +	free(mountpoint);

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)

  reply	other threads:[~2019-08-01  9:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-01  2:02 [PATCH cifs-utils] mount.cifs.c: fix memory leaks in main func Zhiqiang Liu
2019-08-01  9:15 ` Aurélien Aptel [this message]
2019-08-01 14:06   ` Zhiqiang Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h871s0ty.fsf@suse.com \
    --to=aaptel@suse.com \
    --cc=ab@samba.org \
    --cc=dujin1@huawei.com \
    --cc=kdsouza@redhat.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=liujiawen10@huawei.com \
    --cc=liuzhiqiang26@huawei.com \
    --cc=lsahlber@redhat.com \
    --cc=mingfangsen@huawei.com \
    --cc=palcantara@suse.de \
    --cc=pshilov@microsoft.com \
    --cc=smfrench@gmail.com \
    --cc=zhangsaisai@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.