[PATCH] ksmbd: make __dir_empty() compatible with POSIX

[PATCH] ksmbd: make __dir_empty() compatible with POSIX

[Hobin Woo]

Some file systems may not provide dot (.) and dot-dot (..) as they are
optional in POSIX. ksmbd can misjudge emptiness of a directory in those
file systems, since it assumes there are always at least two entries:
dot and dot-dot.
Just set the dirent_count to 2, if the first entry is not a dot.

Signed-off-by: Hobin Woo <hobin.woo@samsung.com>
---
 fs/smb/server/vfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 9e859ba010cf..bb836fa0aaa6 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1115,6 +1115,8 @@ static bool __dir_empty(struct dir_context *ctx, const char *name, int namlen,
 	struct ksmbd_readdir_data *buf;
 
 	buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
+	if (offset == 0 && !(namlen == 1 && name[0] == '.'))
+		buf->dirent_count = 2;
 	buf->dirent_count++;
 
 	return buf->dirent_count <= 2;
-- 
2.43.0

Re: [PATCH] ksmbd: make __dir_empty() compatible with POSIX

[Steve French]

Is there an example where you have seen an fs not report . and .. ?

On Tue, Sep 3, 2024 at 12:19 AM Hobin Woo <hobin.woo@samsung.com> wrote:
>
> Some file systems may not provide dot (.) and dot-dot (..) as they are
> optional in POSIX. ksmbd can misjudge emptiness of a directory in those
> file systems, since it assumes there are always at least two entries:
> dot and dot-dot.
> Just set the dirent_count to 2, if the first entry is not a dot.
>
> Signed-off-by: Hobin Woo <hobin.woo@samsung.com>
> ---
>  fs/smb/server/vfs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index 9e859ba010cf..bb836fa0aaa6 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -1115,6 +1115,8 @@ static bool __dir_empty(struct dir_context *ctx, const char *name, int namlen,
>         struct ksmbd_readdir_data *buf;
>
>         buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
> +       if (offset == 0 && !(namlen == 1 && name[0] == '.'))
> +               buf->dirent_count = 2;
>         buf->dirent_count++;
>
>         return buf->dirent_count <= 2;
> --
> 2.43.0
>
>


-- 
Thanks,

Steve

RE: [PATCH] ksmbd: make __dir_empty() compatible with POSIX

[Hobin Woo]

Android's FUSE file system, /storage/emulated/<USER>, is an example of this.
https://cs.android.com/android/platform/superproject/main/+/main:packages/providers/MediaProvider/jni/ReaddirHelper.cpp;l=52;drc=01c2d29eeefc2d344794429af3df08c38adfd57f

> -----Original Message-----
> From: Steve French <smfrench@gmail.com>
> Sent: Tuesday, September 3, 2024 2:45 PM
> To: Hobin Woo <hobin.woo@samsung.com>
> Cc: linux-cifs@vger.kernel.org; linkinjeon@kernel.org; sfrench@samba.org;
> senozhatsky@chromium.org; tom@talpey.com; sj1557.seo@samsung.com;
> kiras.lee@samsung.com
> Subject: Re: [PATCH] ksmbd: make __dir_empty() compatible with POSIX
> 
> Is there an example where you have seen an fs not report . and .. ?
> 
> On Tue, Sep 3, 2024 at 12:19 AM Hobin Woo <hobin.woo@samsung.com> wrote:
> >
> > Some file systems may not provide dot (.) and dot-dot (..) as they are
> > optional in POSIX. ksmbd can misjudge emptiness of a directory in
> those
> > file systems, since it assumes there are always at least two entries:
> > dot and dot-dot.
> > Just set the dirent_count to 2, if the first entry is not a dot.
> >
> > Signed-off-by: Hobin Woo <hobin.woo@samsung.com>
> > ---
> >  fs/smb/server/vfs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> > index 9e859ba010cf..bb836fa0aaa6 100644
> > --- a/fs/smb/server/vfs.c
> > +++ b/fs/smb/server/vfs.c
> > @@ -1115,6 +1115,8 @@ static bool __dir_empty(struct dir_context *ctx,
> const char *name, int namlen,
> >         struct ksmbd_readdir_data *buf;
> >
> >         buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
> > +       if (offset == 0 && !(namlen == 1 && name[0] == '.'))
> > +               buf->dirent_count = 2;
> >         buf->dirent_count++;
> >
> >         return buf->dirent_count <= 2;
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Thanks,
> 
> Steve

RE: [PATCH] ksmbd: make __dir_empty() compatible with POSIX

[Hobin Woo]

> 
> On Tue, Sep 3, 2024 at 2:19 PM Hobin Woo <hobin.woo@samsung.com>
> wrote:
> >
> > Some file systems may not provide dot (.) and dot-dot (..) as they are
> > optional in POSIX. ksmbd can misjudge emptiness of a directory in
> those
> > file systems, since it assumes there are always at least two entries:
> > dot and dot-dot.
> > Just set the dirent_count to 2, if the first entry is not a dot.
> >
> > Signed-off-by: Hobin Woo <hobin.woo@samsung.com>
> > ---
> >  fs/smb/server/vfs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> > index 9e859ba010cf..bb836fa0aaa6 100644
> > --- a/fs/smb/server/vfs.c
> > +++ b/fs/smb/server/vfs.c
> > @@ -1115,6 +1115,8 @@ static bool __dir_empty(struct dir_context *ctx,
> const char *name, int namlen,
> >         struct ksmbd_readdir_data *buf;
> >
> >         buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
> > +       if (offset == 0 && !(namlen == 1 && name[0] == '.'))
> > +               buf->dirent_count = 2;
> How about not counting the dot, dotdot entry? Because the dot, dotdot
> entry is not always at the front.
> and we change the following code together if we do not count them.
Thanks for pointing out.
We should also consider the stop condition. So the v2 code would look like this:
	buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
+       if (!is_dot_dotdot(name, namlen))
+               buf->dirent_count++;

-       return buf->dirent_count <= 2;
+       return !buf->dirent_count;

Thanks.


> 
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index 9e859ba010cf..243bb2f3aa1f 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -1137,7 +1137,7 @@ int ksmbd_vfs_empty_dir(struct ksmbd_file *fp)
>         readdir_data.dirent_count = 0;
> 
>         err = iterate_dir(fp->filp, &readdir_data.ctx);
> -       if (readdir_data.dirent_count > 2)
> +       if (readdir_data.dirent_count)
>                 err = -ENOTEMPTY;
>         else
>                 err = 0;
> 
> Thanks.
> >         buf->dirent_count++;
> >
> >         return buf->dirent_count <= 2;
> > --
> > 2.43.0
> >
>