diff mbox series

ksmbd: make __dir_empty() compatible with POSIX

Message ID 20240903051918.728540-1-hobin.woo@samsung.com (mailing list archive)
State New, archived
Headers show
Series ksmbd: make __dir_empty() compatible with POSIX | expand

Commit Message

Hobin Woo Sept. 3, 2024, 5:19 a.m. UTC
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(+)

Comments

Steve French Sept. 3, 2024, 5:45 a.m. UTC | #1
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
>
>
Hobin Woo Sept. 3, 2024, 5:53 a.m. UTC | #2
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
Hobin Woo Sept. 3, 2024, 6:33 a.m. UTC | #3
> 
> 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
> >
>
diff mbox series

Patch

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;