[PATCH] overlay: return required buffer size for file handles

[PATCH] overlay: return required buffer size for file handles

[Lubos Dolezel]

Hello,

overlayfs doesn't work well with the fanotify mechanism.

Fanotify first probes for the required buffer size for the file handle,
but overlayfs currently bails out without passing the size back.

That results in errors in the kernel log, such as:

[527944.485384] overlayfs: failed to encode file handle (/, err=-75, buflen=0, len=29, type=1)
[527944.485386] fanotify: failed to encode fid (fsid=ae521e68.a434d95f, type=255, bytes=0, err=-2)

Lubos

Signed-off-by: Lubos Dolezel <lubos@dolezel.info>
---
 fs/overlayfs/export.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 475c61f53..0068bf3a0 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -231,12 +231,9 @@ static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
 	if (IS_ERR(fh))
 		return PTR_ERR(fh);
 
-	err = -EOVERFLOW;
 	len = OVL_FH_LEN(fh);
-	if (len > buflen)
-		goto fail;
-
-	memcpy(fid, fh, len);
+	if (buflen >= len)
+		memcpy(fid, fh, len);
 	err = len;
 
 out:
@@ -255,6 +252,7 @@ static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
 {
 	struct dentry *dentry;
 	int bytes = *max_len << 2;
+	int bytes_used;
 
 	/* TODO: encode connectable file handles */
 	if (parent)
@@ -264,12 +262,14 @@ static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
 	if (WARN_ON(!dentry))
 		return FILEID_INVALID;
 
-	bytes = ovl_dentry_to_fid(dentry, fid, bytes);
+	bytes_used = ovl_dentry_to_fid(dentry, fid, bytes);
 	dput(dentry);
-	if (bytes <= 0)
+	if (bytes_used <= 0)
 		return FILEID_INVALID;
 
-	*max_len = bytes >> 2;
+	*max_len = bytes_used >> 2;
+	if (bytes_used > bytes)
+		return FILEID_INVALID;
 
 	return OVL_FILEID_V1;
 }
-- 
2.26.2

Re: [PATCH] overlay: return required buffer size for file handles

[Amir Goldstein]

On Mon, May 4, 2020 at 10:50 PM Lubos Dolezel <lubos@dolezel.info> wrote:
>
> Hello,
>
> overlayfs doesn't work well with the fanotify mechanism.
>
> Fanotify first probes for the required buffer size for the file handle,
> but overlayfs currently bails out without passing the size back.
>
> That results in errors in the kernel log, such as:
>
> [527944.485384] overlayfs: failed to encode file handle (/, err=-75, buflen=0, len=29, type=1)
> [527944.485386] fanotify: failed to encode fid (fsid=ae521e68.a434d95f, type=255, bytes=0, err=-2)
>
> Lubos

Hi Lubos,

Thank you for the fix.
Please leave greeting (Hello) and this line outside of the commit message.
You may add extra notes for email after --- line

>
> Signed-off-by: Lubos Dolezel <lubos@dolezel.info>

After fixing below you may add for v2:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/overlayfs/export.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> index 475c61f53..0068bf3a0 100644
> --- a/fs/overlayfs/export.c
> +++ b/fs/overlayfs/export.c
> @@ -231,12 +231,9 @@ static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
>         if (IS_ERR(fh))
>                 return PTR_ERR(fh);
>
> -       err = -EOVERFLOW;
>         len = OVL_FH_LEN(fh);
> -       if (len > buflen)
> -               goto fail;

in pr_warn_ratelimited() under fail label, all printed values after 'err' are
now irrelevant for the warning. please remove them.

> -
> -       memcpy(fid, fh, len);
> +       if (buflen >= len)
> +               memcpy(fid, fh, len);
>         err = len;
>
>  out:
> @@ -255,6 +252,7 @@ static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
>  {
>         struct dentry *dentry;
>         int bytes = *max_len << 2;
> +       int bytes_used;

Please use these variable names instead to make code clearer:

         int bytes, buflen = *max_len << 2;

BTW, do you plan to backport this patch to stable kernel 5.4?
Because this patch will not apply cleanly, but we should fix this in stable.

Thanks,
Amir.

Re: [PATCH] overlay: return required buffer size for file handles

[Amir Goldstein]

On Tue, May 5, 2020 at 5:58 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 10:50 PM Lubos Dolezel <lubos@dolezel.info> wrote:
> >
> > Hello,
> >
> > overlayfs doesn't work well with the fanotify mechanism.
> >
> > Fanotify first probes for the required buffer size for the file handle,
> > but overlayfs currently bails out without passing the size back.
> >
> > That results in errors in the kernel log, such as:
> >
> > [527944.485384] overlayfs: failed to encode file handle (/, err=-75, buflen=0, len=29, type=1)
> > [527944.485386] fanotify: failed to encode fid (fsid=ae521e68.a434d95f, type=255, bytes=0, err=-2)
> >
> > Lubos
>
> Hi Lubos,
>
> Thank you for the fix.
> Please leave greeting (Hello) and this line outside of the commit message.
> You may add extra notes for email after --- line
>
> >
> > Signed-off-by: Lubos Dolezel <lubos@dolezel.info>
>
> After fixing below you may add for v2:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>

Pushed my v2 to:
https://github.com/amir73il/linux/commits/ovl-fixes

Along with Dan's patch.

Tests are at:
https://github.com/amir73il/xfstests/commits/ovl-fixes

Thanks,
Amir.