Topic branch for fs/exportfs:
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
branch: exportfs-next
Changes since v3:
- is_dot_dotdot() now checks that the file name length > 0
Changes since v2:
- Capture the open-coded "is_dot_dotdot" implementation in
lookup_one_common()
Changes since v1:
- Fixes: was dropped from 1/2
- Added a patch to hoist is_dot_dotdot() into linux/fs.h
---
Chuck Lever (1):
fs: Create a generic is_dot_dotdot() utility
Trond Myklebust (1):
exportfs: fix the fallback implementation of the get_name export operation
fs/crypto/fname.c | 8 +-------
fs/ecryptfs/crypto.c | 10 ----------
fs/exportfs/expfs.c | 2 +-
fs/f2fs/f2fs.h | 11 -----------
fs/namei.c | 6 ++----
include/linux/fs.h | 13 +++++++++++++
6 files changed, 17 insertions(+), 33 deletions(-)
--
Chuck Lever
From: Chuck Lever <[email protected]>
De-duplicate the same functionality in several places by hoisting
the is_dot_dotdot() utility function into linux/fs.h.
Suggested-by: Amir Goldstein <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
---
fs/crypto/fname.c | 8 +-------
fs/ecryptfs/crypto.c | 10 ----------
fs/exportfs/expfs.c | 4 +---
fs/f2fs/f2fs.h | 11 -----------
fs/namei.c | 6 ++----
include/linux/fs.h | 13 +++++++++++++
6 files changed, 17 insertions(+), 35 deletions(-)
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 7b3fc189593a..0ad52fbe51c9 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -74,13 +74,7 @@ struct fscrypt_nokey_name {
static inline bool fscrypt_is_dot_dotdot(const struct qstr *str)
{
- if (str->len == 1 && str->name[0] == '.')
- return true;
-
- if (str->len == 2 && str->name[0] == '.' && str->name[1] == '.')
- return true;
-
- return false;
+ return is_dot_dotdot(str->name, str->len);
}
/**
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 03bd55069d86..2fe0f3af1a08 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -1949,16 +1949,6 @@ int ecryptfs_encrypt_and_encode_filename(
return rc;
}
-static bool is_dot_dotdot(const char *name, size_t name_size)
-{
- if (name_size == 1 && name[0] == '.')
- return true;
- else if (name_size == 2 && name[0] == '.' && name[1] == '.')
- return true;
-
- return false;
-}
-
/**
* ecryptfs_decode_and_decrypt_filename - converts the encoded cipher text name to decoded plaintext
* @plaintext_name: The plaintext name
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 84af58eaf2ca..07ea3d62b298 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -255,9 +255,7 @@ static bool filldir_one(struct dir_context *ctx, const char *name, int len,
container_of(ctx, struct getdents_callback, ctx);
buf->sequence++;
- /* Ignore the '.' and '..' entries */
- if ((len > 2 || name[0] != '.' || (len == 2 && name[1] != '.')) &&
- buf->ino == ino && len <= NAME_MAX) {
+ if (buf->ino == ino && len <= NAME_MAX && !is_dot_dotdot(name, len)) {
memcpy(buf->name, name, len);
buf->name[len] = '\0';
buf->found = 1;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9043cedfa12b..322a3b8a3533 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3368,17 +3368,6 @@ static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
}
-static inline bool is_dot_dotdot(const u8 *name, size_t len)
-{
- if (len == 1 && name[0] == '.')
- return true;
-
- if (len == 2 && name[0] == '.' && name[1] == '.')
- return true;
-
- return false;
-}
-
static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
size_t size, gfp_t flags)
{
diff --git a/fs/namei.c b/fs/namei.c
index 71c13b2990b4..2386a70667fa 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2667,10 +2667,8 @@ static int lookup_one_common(struct mnt_idmap *idmap,
if (!len)
return -EACCES;
- if (unlikely(name[0] == '.')) {
- if (len < 2 || (len == 2 && name[1] == '.'))
- return -EACCES;
- }
+ if (is_dot_dotdot(name, len))
+ return -EACCES;
while (len--) {
unsigned int c = *(const unsigned char *)name++;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 98b7a7a8c42e..53dd58a907e0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2846,6 +2846,19 @@ extern bool path_is_under(const struct path *, const struct path *);
extern char *file_path(struct file *, char *, int);
+/**
+ * is_dot_dotdot - returns true only if @name is "." or ".."
+ * @name: file name to check
+ * @len: length of file name, in bytes
+ *
+ * Coded for efficiency.
+ */
+static inline bool is_dot_dotdot(const char *name, size_t len)
+{
+ return len && unlikely(name[0] == '.') &&
+ (len < 2 || (len == 2 && name[1] == '.'));
+}
+
#include <linux/err.h>
/* needed for stackable file system support */
From: Trond Myklebust <[email protected]>
The fallback implementation for the get_name export operation uses
readdir() to try to match the inode number to a filename. That filename
is then used together with lookup_one() to produce a dentry.
A problem arises when we match the '.' or '..' entries, since that
causes lookup_one() to fail. This has sometimes been seen to occur for
filesystems that violate POSIX requirements around uniqueness of inode
numbers, something that is common for snapshot directories.
This patch just ensures that we skip '.' and '..' rather than allowing a
match.
Signed-off-by: Trond Myklebust <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Acked-by: Amir Goldstein <[email protected]>
Link: https://lore.kernel.org/linux-nfs/CAOQ4uxiOZobN76OKB-VBNXWeFKVwLW_eK5QtthGyYzWU9mjb7Q@mail.gmail.com/
Signed-off-by: Chuck Lever <[email protected]>
---
fs/exportfs/expfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 3ae0154c5680..84af58eaf2ca 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -255,7 +255,9 @@ static bool filldir_one(struct dir_context *ctx, const char *name, int len,
container_of(ctx, struct getdents_callback, ctx);
buf->sequence++;
- if (buf->ino == ino && len <= NAME_MAX) {
+ /* Ignore the '.' and '..' entries */
+ if ((len > 2 || name[0] != '.' || (len == 2 && name[1] != '.')) &&
+ buf->ino == ino && len <= NAME_MAX) {
memcpy(buf->name, name, len);
buf->name[len] = '\0';
buf->found = 1;
On Thu, Jan 4, 2024 at 10:55 PM Chuck Lever <[email protected]> wrote:
>
> From: Chuck Lever <[email protected]>
>
> De-duplicate the same functionality in several places by hoisting
> the is_dot_dotdot() utility function into linux/fs.h.
>
> Suggested-by: Amir Goldstein <[email protected]>
> Reviewed-by: Jeff Layton <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
Reviewed-by: Amir Goldstein <[email protected]>
> ---
> fs/crypto/fname.c | 8 +-------
> fs/ecryptfs/crypto.c | 10 ----------
> fs/exportfs/expfs.c | 4 +---
> fs/f2fs/f2fs.h | 11 -----------
> fs/namei.c | 6 ++----
> include/linux/fs.h | 13 +++++++++++++
> 6 files changed, 17 insertions(+), 35 deletions(-)
>
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 7b3fc189593a..0ad52fbe51c9 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -74,13 +74,7 @@ struct fscrypt_nokey_name {
>
> static inline bool fscrypt_is_dot_dotdot(const struct qstr *str)
> {
> - if (str->len == 1 && str->name[0] == '.')
> - return true;
> -
> - if (str->len == 2 && str->name[0] == '.' && str->name[1] == '.')
> - return true;
> -
> - return false;
> + return is_dot_dotdot(str->name, str->len);
> }
>
> /**
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index 03bd55069d86..2fe0f3af1a08 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -1949,16 +1949,6 @@ int ecryptfs_encrypt_and_encode_filename(
> return rc;
> }
>
> -static bool is_dot_dotdot(const char *name, size_t name_size)
> -{
> - if (name_size == 1 && name[0] == '.')
> - return true;
> - else if (name_size == 2 && name[0] == '.' && name[1] == '.')
> - return true;
> -
> - return false;
> -}
> -
> /**
> * ecryptfs_decode_and_decrypt_filename - converts the encoded cipher text name to decoded plaintext
> * @plaintext_name: The plaintext name
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 84af58eaf2ca..07ea3d62b298 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -255,9 +255,7 @@ static bool filldir_one(struct dir_context *ctx, const char *name, int len,
> container_of(ctx, struct getdents_callback, ctx);
>
> buf->sequence++;
> - /* Ignore the '.' and '..' entries */
> - if ((len > 2 || name[0] != '.' || (len == 2 && name[1] != '.')) &&
> - buf->ino == ino && len <= NAME_MAX) {
> + if (buf->ino == ino && len <= NAME_MAX && !is_dot_dotdot(name, len)) {
> memcpy(buf->name, name, len);
> buf->name[len] = '\0';
> buf->found = 1;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9043cedfa12b..322a3b8a3533 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3368,17 +3368,6 @@ static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
> return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
> }
>
> -static inline bool is_dot_dotdot(const u8 *name, size_t len)
> -{
> - if (len == 1 && name[0] == '.')
> - return true;
> -
> - if (len == 2 && name[0] == '.' && name[1] == '.')
> - return true;
> -
> - return false;
> -}
> -
> static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
> size_t size, gfp_t flags)
> {
> diff --git a/fs/namei.c b/fs/namei.c
> index 71c13b2990b4..2386a70667fa 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2667,10 +2667,8 @@ static int lookup_one_common(struct mnt_idmap *idmap,
> if (!len)
> return -EACCES;
>
> - if (unlikely(name[0] == '.')) {
> - if (len < 2 || (len == 2 && name[1] == '.'))
> - return -EACCES;
> - }
> + if (is_dot_dotdot(name, len))
> + return -EACCES;
>
> while (len--) {
> unsigned int c = *(const unsigned char *)name++;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 98b7a7a8c42e..53dd58a907e0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2846,6 +2846,19 @@ extern bool path_is_under(const struct path *, const struct path *);
>
> extern char *file_path(struct file *, char *, int);
>
> +/**
> + * is_dot_dotdot - returns true only if @name is "." or ".."
> + * @name: file name to check
> + * @len: length of file name, in bytes
> + *
> + * Coded for efficiency.
> + */
> +static inline bool is_dot_dotdot(const char *name, size_t len)
> +{
> + return len && unlikely(name[0] == '.') &&
> + (len < 2 || (len == 2 && name[1] == '.'));
> +}
> +
Looking back at the version that I suggested, (len < 2
here is silly and should be (len == 1 || ...
But let's wait for inputs from other developers on this helper,
especially Al.
Thanks,
Amir.
On Fri, Jan 05, 2024 at 09:36:51AM +0200, Amir Goldstein wrote:
> On Thu, Jan 4, 2024 at 10:55 PM Chuck Lever <[email protected]> wrote:
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 98b7a7a8c42e..53dd58a907e0 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2846,6 +2846,19 @@ extern bool path_is_under(const struct path *, const struct path *);
> >
> > extern char *file_path(struct file *, char *, int);
> >
> > +/**
> > + * is_dot_dotdot - returns true only if @name is "." or ".."
> > + * @name: file name to check
> > + * @len: length of file name, in bytes
> > + *
> > + * Coded for efficiency.
> > + */
> > +static inline bool is_dot_dotdot(const char *name, size_t len)
> > +{
> > + return len && unlikely(name[0] == '.') &&
> > + (len < 2 || (len == 2 && name[1] == '.'));
> > +}
> > +
>
> Looking back at the version that I suggested, (len < 2
> here is silly and should be (len == 1 || ...
Yeah, probably. I'm trying to stick to copying code without changing
it at the same time; that's the usual guideline.
> But let's wait for inputs from other developers on this helper,
> especially Al.
Fair, will do.
--
Chuck Lever
> On Jan 5, 2024, at 2:36 AM, Amir Goldstein <[email protected]> wrote:
>
> On Thu, Jan 4, 2024 at 10:55 PM Chuck Lever <[email protected]> wrote:
>>
>> From: Chuck Lever <[email protected]>
>>
>> De-duplicate the same functionality in several places by hoisting
>> the is_dot_dotdot() utility function into linux/fs.h.
>>
>> Suggested-by: Amir Goldstein <[email protected]>
>> Reviewed-by: Jeff Layton <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>
> Reviewed-by: Amir Goldstein <[email protected]>
>
>> ---
>> fs/crypto/fname.c | 8 +-------
>> fs/ecryptfs/crypto.c | 10 ----------
>> fs/exportfs/expfs.c | 4 +---
>> fs/f2fs/f2fs.h | 11 -----------
>> fs/namei.c | 6 ++----
>> include/linux/fs.h | 13 +++++++++++++
>> 6 files changed, 17 insertions(+), 35 deletions(-)
>>
>> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
>> index 7b3fc189593a..0ad52fbe51c9 100644
>> --- a/fs/crypto/fname.c
>> +++ b/fs/crypto/fname.c
>> @@ -74,13 +74,7 @@ struct fscrypt_nokey_name {
>>
>> static inline bool fscrypt_is_dot_dotdot(const struct qstr *str)
>> {
>> - if (str->len == 1 && str->name[0] == '.')
>> - return true;
>> -
>> - if (str->len == 2 && str->name[0] == '.' && str->name[1] == '.')
>> - return true;
>> -
>> - return false;
>> + return is_dot_dotdot(str->name, str->len);
>> }
>>
>> /**
>> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
>> index 03bd55069d86..2fe0f3af1a08 100644
>> --- a/fs/ecryptfs/crypto.c
>> +++ b/fs/ecryptfs/crypto.c
>> @@ -1949,16 +1949,6 @@ int ecryptfs_encrypt_and_encode_filename(
>> return rc;
>> }
>>
>> -static bool is_dot_dotdot(const char *name, size_t name_size)
>> -{
>> - if (name_size == 1 && name[0] == '.')
>> - return true;
>> - else if (name_size == 2 && name[0] == '.' && name[1] == '.')
>> - return true;
>> -
>> - return false;
>> -}
>> -
>> /**
>> * ecryptfs_decode_and_decrypt_filename - converts the encoded cipher text name to decoded plaintext
>> * @plaintext_name: The plaintext name
>> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
>> index 84af58eaf2ca..07ea3d62b298 100644
>> --- a/fs/exportfs/expfs.c
>> +++ b/fs/exportfs/expfs.c
>> @@ -255,9 +255,7 @@ static bool filldir_one(struct dir_context *ctx, const char *name, int len,
>> container_of(ctx, struct getdents_callback, ctx);
>>
>> buf->sequence++;
>> - /* Ignore the '.' and '..' entries */
>> - if ((len > 2 || name[0] != '.' || (len == 2 && name[1] != '.')) &&
>> - buf->ino == ino && len <= NAME_MAX) {
>> + if (buf->ino == ino && len <= NAME_MAX && !is_dot_dotdot(name, len)) {
>> memcpy(buf->name, name, len);
>> buf->name[len] = '\0';
>> buf->found = 1;
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 9043cedfa12b..322a3b8a3533 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3368,17 +3368,6 @@ static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
>> return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
>> }
>>
>> -static inline bool is_dot_dotdot(const u8 *name, size_t len)
>> -{
>> - if (len == 1 && name[0] == '.')
>> - return true;
>> -
>> - if (len == 2 && name[0] == '.' && name[1] == '.')
>> - return true;
>> -
>> - return false;
>> -}
>> -
>> static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
>> size_t size, gfp_t flags)
>> {
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 71c13b2990b4..2386a70667fa 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -2667,10 +2667,8 @@ static int lookup_one_common(struct mnt_idmap *idmap,
>> if (!len)
>> return -EACCES;
>>
>> - if (unlikely(name[0] == '.')) {
>> - if (len < 2 || (len == 2 && name[1] == '.'))
>> - return -EACCES;
>> - }
>> + if (is_dot_dotdot(name, len))
>> + return -EACCES;
>>
>> while (len--) {
>> unsigned int c = *(const unsigned char *)name++;
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 98b7a7a8c42e..53dd58a907e0 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2846,6 +2846,19 @@ extern bool path_is_under(const struct path *, const struct path *);
>>
>> extern char *file_path(struct file *, char *, int);
>>
>> +/**
>> + * is_dot_dotdot - returns true only if @name is "." or ".."
>> + * @name: file name to check
>> + * @len: length of file name, in bytes
>> + *
>> + * Coded for efficiency.
>> + */
>> +static inline bool is_dot_dotdot(const char *name, size_t len)
>> +{
>> + return len && unlikely(name[0] == '.') &&
>> + (len < 2 || (len == 2 && name[1] == '.'));
>> +}
>> +
>
> Looking back at the version that I suggested, (len < 2
> here is silly and should be (len == 1 || ...
>
> But let's wait for inputs from other developers on this helper,
> especially Al.
Al, any comments on this clean-up ?
--
Chuck Lever