mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ovl: reject lowerdir ending with unescaped backslash
@ 2026-10-12  6:08 Guanglei Zhu
  2026-10-12  9:47 ` Amir Goldstein
  2026-10-12  9:55 ` [PATCH v2] " Guanglei Zhu
  0 siblings, 2 replies; 6+ messages in thread
From: Guanglei Zhu @ 2026-10-12  6:08 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, linux-kernel, liuc63

In ovl_parse_param_split_lowerdirs() the escape handling consumes
the backslash and the character after it.  When the backslash is the
last character of the lowerdir value, the loop increment moves the
cursor past the NUL terminator, so the loop scans heap memory beyond
the kstrdup'd string until it happens to hit a zero byte; a colon
found there is written over as well.

A trailing unescaped backslash escapes nothing, so reject it with
EINVAL.

Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
Found by code inspection.  Behavior check in a QEMU guest:
fsconfig(lowerdir="...\") on an unpatched kernel returns -ENOENT
after the loop scanned past the allocation; with this patch it
returns -EINVAL.

 fs/overlayfs/params.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index c93fcaa45..5a15e5478 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -216,6 +216,9 @@ static ssize_t ovl_parse_param_split_lowerdirs(char *str)
 		if (*s == '\\') {
 			/* keep esc chars in split lowerdir */
 			*d++ = *s++;
+			/* A trailing backslash escapes nothing. */
+			if (!*s)
+				return -EINVAL;
 		} else if (*s == ':') {
 			bool next_colon = (*(s + 1) == ':');
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ovl: reject lowerdir ending with unescaped backslash
  2026-10-12  6:08 [PATCH] ovl: reject lowerdir ending with unescaped backslash Guanglei Zhu
@ 2026-10-12  9:47 ` Amir Goldstein
  2026-10-12  9:55 ` [PATCH v2] " Guanglei Zhu
  1 sibling, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2026-10-12  9:47 UTC (permalink / raw)
  To: Guanglei Zhu
  Cc: Miklos Szeredi, linux-unionfs, linux-kernel, liuc63, Christian Brauner

On Mon, Oct 12, 2026 at 8:08 AM Guanglei Zhu <zhugl3@xiaopeng.com> wrote:
>
> In ovl_parse_param_split_lowerdirs() the escape handling consumes
> the backslash and the character after it.  When the backslash is the
> last character of the lowerdir value, the loop increment moves the
> cursor past the NUL terminator, so the loop scans heap memory beyond
> the kstrdup'd string until it happens to hit a zero byte; a colon
> found there is written over as well.
>
> A trailing unescaped backslash escapes nothing, so reject it with
> EINVAL.
>
> Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> ---
> Found by code inspection.  Behavior check in a QEMU guest:
> fsconfig(lowerdir="...\") on an unpatched kernel returns -ENOENT
> after the loop scanned past the allocation; with this patch it
> returns -EINVAL.
>
>  fs/overlayfs/params.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index c93fcaa45..5a15e5478 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -216,6 +216,9 @@ static ssize_t ovl_parse_param_split_lowerdirs(char *str)
>                 if (*s == '\\') {
>                         /* keep esc chars in split lowerdir */
>                         *d++ = *s++;
> +                       /* A trailing backslash escapes nothing. */
> +                       if (!*s)
> +                               return -EINVAL;
>                 } else if (*s == ':') {
>                         bool next_colon = (*(s + 1) == ':');
>
> --
> 2.43.0
>

Hi Guanglei Zhu,

The fix looks correct, but the commit message is inaccurately making
wrong claims about heap memory and scanning past the allocation,
which makes this sound more severe than it actually is.

See Sashiko review comment:
https://sashiko.dev/#/patchset/20261012060847.836027-1-zhugl3%40xiaopeng.com

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] ovl: reject lowerdir ending with unescaped backslash
  2026-10-12  6:08 [PATCH] ovl: reject lowerdir ending with unescaped backslash Guanglei Zhu
  2026-10-12  9:47 ` Amir Goldstein
@ 2026-10-12  9:55 ` Guanglei Zhu
  2026-10-12 10:11   ` Amir Goldstein
  2026-10-12 10:44   ` Christian Brauner
  1 sibling, 2 replies; 6+ messages in thread
From: Guanglei Zhu @ 2026-10-12  9:55 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, linux-kernel, liuwb

In ovl_parse_param_split_lowerdirs() a backslash consumes the
character that follows it.  When the backslash is the last character
of the lowerdir value, it consumes the NUL terminator: the loop exits
normally and the backslash is kept in the output, so the lowerdir is
passed downstream with a trailing backslash and the mount fails later
with a confusing ENOENT from path lookup.

A trailing unescaped backslash escapes nothing, so reject it with
EINVAL.

Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v2: fix the commit message.  v1 wrongly claimed that the loop
scans heap memory past the allocation; there is no out-of-bounds
access, the backslash merely consumes the NUL terminator.  Thanks
to Amir Goldstein and Sashiko for catching this.

Verified with an ASAN build of a user-space replica of the loop:
no out-of-bounds access, the output retains the trailing backslash.
With this patch fsconfig(lowerdir="...\") returns -EINVAL instead
of -ENOENT.

 fs/overlayfs/params.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index c93fcaa45..5a15e5478 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -216,6 +216,9 @@ static ssize_t ovl_parse_param_split_lowerdirs(char *str)
 		if (*s == '\\') {
 			/* keep esc chars in split lowerdir */
 			*d++ = *s++;
+			/* A trailing backslash escapes nothing. */
+			if (!*s)
+				return -EINVAL;
 		} else if (*s == ':') {
 			bool next_colon = (*(s + 1) == ':');
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ovl: reject lowerdir ending with unescaped backslash
  2026-10-12  9:55 ` [PATCH v2] " Guanglei Zhu
@ 2026-10-12 10:11   ` Amir Goldstein
  2026-10-12 10:45     ` Christian Brauner
  2026-10-12 10:44   ` Christian Brauner
  1 sibling, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2026-10-12 10:11 UTC (permalink / raw)
  To: Guanglei Zhu, Christian Brauner
  Cc: Miklos Szeredi, linux-unionfs, linux-kernel, liuwb

On Mon, Oct 12, 2026 at 11:55 AM Guanglei Zhu <zhugl3@xiaopeng.com> wrote:
>
> In ovl_parse_param_split_lowerdirs() a backslash consumes the
> character that follows it.  When the backslash is the last character
> of the lowerdir value, it consumes the NUL terminator: the loop exits
> normally and the backslash is kept in the output, so the lowerdir is
> passed downstream with a trailing backslash and the mount fails later
> with a confusing ENOENT from path lookup.
>
> A trailing unescaped backslash escapes nothing, so reject it with
> EINVAL.
>
> Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> ---
> v2: fix the commit message.  v1 wrongly claimed that the loop
> scans heap memory past the allocation; there is no out-of-bounds
> access, the backslash merely consumes the NUL terminator.  Thanks
> to Amir Goldstein and Sashiko for catching this.
>
> Verified with an ASAN build of a user-space replica of the loop:
> no out-of-bounds access, the output retains the trailing backslash.
> With this patch fsconfig(lowerdir="...\") returns -EINVAL instead
> of -ENOENT.
>
>  fs/overlayfs/params.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index c93fcaa45..5a15e5478 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -216,6 +216,9 @@ static ssize_t ovl_parse_param_split_lowerdirs(char *str)
>                 if (*s == '\\') {
>                         /* keep esc chars in split lowerdir */
>                         *d++ = *s++;
> +                       /* A trailing backslash escapes nothing. */
> +                       if (!*s)
> +                               return -EINVAL;
>                 } else if (*s == ':') {
>                         bool next_colon = (*(s + 1) == ':');
>
> --
> 2.43.0
>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Christian,

Could you pick this up via vfs.fixes?

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ovl: reject lowerdir ending with unescaped backslash
  2026-10-12  9:55 ` [PATCH v2] " Guanglei Zhu
  2026-10-12 10:11   ` Amir Goldstein
@ 2026-10-12 10:44   ` Christian Brauner
  1 sibling, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-12 10:44 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein, Guanglei Zhu
  Cc: linux-unionfs, linux-kernel, liuwb

On Mon, 12 Oct 2026 17:55:38 +0800, Guanglei Zhu wrote:
> In ovl_parse_param_split_lowerdirs() a backslash consumes the
> character that follows it.  When the backslash is the last character
> of the lowerdir value, it consumes the NUL terminator: the loop exits
> normally and the backslash is kept in the output, so the lowerdir is
> passed downstream with a trailing backslash and the mount fails later
> with a confusing ENOENT from path lookup.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] ovl: reject lowerdir ending with unescaped backslash
      https://git.kernel.org/vfs/vfs/c/81f4be14beee


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ovl: reject lowerdir ending with unescaped backslash
  2026-10-12 10:11   ` Amir Goldstein
@ 2026-10-12 10:45     ` Christian Brauner
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-12 10:45 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Guanglei Zhu, Miklos Szeredi, linux-unionfs, linux-kernel, liuwb

> Christian,
> 
> Could you pick this up via vfs.fixes?

In vfs.fixes now. Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-12 10:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12  6:08 [PATCH] ovl: reject lowerdir ending with unescaped backslash Guanglei Zhu
2026-10-12  9:47 ` Amir Goldstein
2026-10-12  9:55 ` [PATCH v2] " Guanglei Zhu
2026-10-12 10:11   ` Amir Goldstein
2026-10-12 10:45     ` Christian Brauner
2026-10-12 10:44   ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®