[STABLE 6.6.y] ovl: fix memory leak in ovl_parse_param()

[STABLE 6.6.y] ovl: fix memory leak in ovl_parse_param()

[Jingbo Xu]

From: Amir Goldstein <amir73il@gmail.com>

commit 37f32f52643869131ec01bb69bdf9f404f6109fb upstream.

On failure to parse parameters in ovl_parse_param_lowerdir(), it is
necessary to update ctx->nr with the correct nr before using
ovl_reset_lowerdirs() to release l->name.

Reported-and-tested-by: syzbot+26eedf3631650972f17c@syzkaller.appspotmail.com
Fixes: c835110b588a ("ovl: remove unused code in lowerdir param parsing")
Co-authored-by: Edward Adam Davis <eadavis@qq.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
Commit c835110b588a ("ovl: remove unused code in lowerdir param
parsing") was back ported to 6.6.y as a "Stable-dep-of" of commit
2824083db76c ("ovl: Always reject mounting over case-insensitive
directories"), while omitting the fix for commit c835110b588a itself.
Maybe that is because by the time commit 37f32f526438 (the fix) is merged
into master branch, commit c835110b588a has not been back ported to 6.6.y
yet.

This causes ltp test warning on vfat (EBUSY when umounting) [1]:
tst_test.c:1701: TINFO: === Testing on vfat ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/ltp-XeGbUgDq5N/LTP_fankzf5WQ/mntpoint fstyp=vfat flags=0
fanotify13.c:152: TINFO: Test #0.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #1.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #2.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #3.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #4.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #5.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
fanotify13.c:157: TCONF: overlayfs not supported on vfat
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  1...
tst_device.c:412: TINFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  2...
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  3...
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  4...

Reproduce:
/opt/ltp/runltp -f syscalls -s fanotify13

The original fix is applied to 6.6.y without conflict.

[1] https://lists.linux.it/pipermail/ltp/2023-November/036189.html
---
 fs/overlayfs/params.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index ad3593a41fb5..488f920f79d2 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -438,7 +438,7 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
 	struct ovl_fs_context *ctx = fc->fs_private;
 	struct ovl_fs_context_layer *l;
 	char *dup = NULL, *iter;
-	ssize_t nr_lower = 0, nr = 0, nr_data = 0;
+	ssize_t nr_lower, nr;
 	bool data_layer = false;
 
 	/*
@@ -490,6 +490,7 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
 	iter = dup;
 	l = ctx->lower;
 	for (nr = 0; nr < nr_lower; nr++, l++) {
+		ctx->nr++;
 		memset(l, 0, sizeof(*l));
 
 		err = ovl_mount_dir(iter, &l->path);
@@ -506,10 +507,10 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
 			goto out_put;
 
 		if (data_layer)
-			nr_data++;
+			ctx->nr_data++;
 
 		/* Calling strchr() again would overrun. */
-		if ((nr + 1) == nr_lower)
+		if (ctx->nr == nr_lower)
 			break;
 
 		err = -EINVAL;
@@ -519,7 +520,7 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
 			 * This is a regular layer so we require that
 			 * there are no data layers.
 			 */
-			if ((ctx->nr_data + nr_data) > 0) {
+			if (ctx->nr_data > 0) {
 				pr_err("regular lower layers cannot follow data lower layers");
 				goto out_put;
 			}
@@ -532,8 +533,6 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
 		data_layer = true;
 		iter++;
 	}
-	ctx->nr = nr_lower;
-	ctx->nr_data += nr_data;
 	kfree(dup);
 	return 0;
 
-- 
2.19.1.6.gb485710b

Re: [STABLE 6.6.y] ovl: fix memory leak in ovl_parse_param()

[Greg KH]

On Tue, Apr 30, 2024 at 11:48:54AM +0800, Jingbo Xu wrote:
> From: Amir Goldstein <amir73il@gmail.com>
> 
> commit 37f32f52643869131ec01bb69bdf9f404f6109fb upstream.
> 
> On failure to parse parameters in ovl_parse_param_lowerdir(), it is
> necessary to update ctx->nr with the correct nr before using
> ovl_reset_lowerdirs() to release l->name.
> 
> Reported-and-tested-by: syzbot+26eedf3631650972f17c@syzkaller.appspotmail.com
> Fixes: c835110b588a ("ovl: remove unused code in lowerdir param parsing")
> Co-authored-by: Edward Adam Davis <eadavis@qq.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> ---
> Commit c835110b588a ("ovl: remove unused code in lowerdir param
> parsing") was back ported to 6.6.y as a "Stable-dep-of" of commit
> 2824083db76c ("ovl: Always reject mounting over case-insensitive
> directories"), while omitting the fix for commit c835110b588a itself.
> Maybe that is because by the time commit 37f32f526438 (the fix) is merged
> into master branch, commit c835110b588a has not been back ported to 6.6.y
> yet.

Now queued up, thanks.

greg k-h

Re: [STABLE 6.6.y] ovl: fix memory leak in ovl_parse_param()

[Amir Goldstein]

On Tue, Apr 30, 2024 at 10:53 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Apr 30, 2024 at 11:48:54AM +0800, Jingbo Xu wrote:
> > From: Amir Goldstein <amir73il@gmail.com>
> >
> > commit 37f32f52643869131ec01bb69bdf9f404f6109fb upstream.
> >
> > On failure to parse parameters in ovl_parse_param_lowerdir(), it is
> > necessary to update ctx->nr with the correct nr before using
> > ovl_reset_lowerdirs() to release l->name.
> >
> > Reported-and-tested-by: syzbot+26eedf3631650972f17c@syzkaller.appspotmail.com
> > Fixes: c835110b588a ("ovl: remove unused code in lowerdir param parsing")
> > Co-authored-by: Edward Adam Davis <eadavis@qq.com>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> > ---
> > Commit c835110b588a ("ovl: remove unused code in lowerdir param
> > parsing") was back ported to 6.6.y as a "Stable-dep-of" of commit
> > 2824083db76c ("ovl: Always reject mounting over case-insensitive
> > directories"), while omitting the fix for commit c835110b588a itself.
> > Maybe that is because by the time commit 37f32f526438 (the fix) is merged
> > into master branch, commit c835110b588a has not been back ported to 6.6.y
> > yet.

This is strange.
The 6.6 backports were posted by Sasha on Mar 2024.
The omitted fix was merged in Nov 2023.

Sasha,

Do you understand what went wrong?

Thanks,
Amir.