From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from asavdk3.altibox.net ([109.247.116.14]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1efBGh-0001vN-CF for barebox@lists.infradead.org; Fri, 26 Jan 2018 21:10:41 +0000 Date: Fri, 26 Jan 2018 22:10:25 +0100 From: Sam Ravnborg Message-ID: <20180126211025.GB3317@ravnborg.org> References: <20180126194227.32088-1-s.hauer@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180126194227.32088-1-s.hauer@pengutronix.de> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] fs: Fix memory leak in mount() To: Sascha Hauer Cc: Barebox List Hi Sasha. On Fri, Jan 26, 2018 at 08:42:27PM +0100, Sascha Hauer wrote: > "path" is allocated by normalise_path() and thus must be > freed. This was done in the error path, but not in the success > path. > > Signed-off-by: Sascha Hauer > --- > fs/fs.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/fs.c b/fs/fs.c > index 6f15e93ba9..7d0d97906d 100644 > --- a/fs/fs.c > +++ b/fs/fs.c > @@ -1392,6 +1392,8 @@ int mount(const char *device, const char *fsname, const char *_path, > fsdev_set_linux_rootarg(fsdev, str); > } > > + free(path); > + > return 0; > > err_no_driver: An out: label so we only called free once was also an option. Both patterns are used in this file - so either should be OK. While browsing this file I think there is a few similar cases involving normalise_path(): automount_remove() path is assigned, but never freed. automount_add() am is allocated, but not freed if we return -ENOTDIR. And thus all of path, cmd, and am is leaked I also noticed: opendir() No check for PTR_ERR after calling canonalize_path() unlink() No check for PTR_ERR after call to canonalize_dir() readlink() No check for PTR_ERR after call to canonalize_dir() I can create patch for these - but I cannot do any thrustworthy testing. And for the canonalize() I am uncertain if the check is requied. Let me know your preference and I will take care. Sam _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox