utf8data.h_shipped has a large database table which is an auto-generated
decodification trie for the unicode normalization functions and it is not
necessary to carry this large table in the kernel.
Goal is to make UTF-8 encoding loadable by converting it into a module
and adding a layer between the filesystems and the utf8 module which will
load the module whenever any filesystem that needs unicode is mounted.
1st patch in the series resolves the warning reported by kernel test robot
and 2nd patch fixes the incorrect use of utf8_unload() in ext4 and
f2fs filesystems.
Unicode is the subsystem and utf8 is a charachter encoding for the
subsystem, hence 3rd and 4th patches in the series are renaming functions
and file name to unicode for better understanding the difference between
UTF-8 module and unicode layer.
Last patch in the series adds the layer and utf8 module and also uses
static_call() function introducted for preventing speculative execution
attacks.
---
Changes in v3
- Add a patch which checks if utf8 is loaded before calling utf8_unload()
in ext4 and f2fs filesystems
- Return error if strscpy() returns value < 0
- Correct the conditions to prevent NULL pointer dereference while
accessing functions via utf8_ops variable.
- Add spinlock to avoid race conditions.
- Use static_call() for preventing speculative execution attacks.
Changes in v2
- Remove the duplicate file from the last patch.
- Make the wrapper functions inline.
- Remove msleep and use try_module_get() and module_put()
for ensuring that module is loaded correctly and also
doesn't get unloaded while in use.
- Resolve the warning reported by kernel test robot.
- Resolve all the checkpatch.pl warnings.
Shreeya Patel (4):
fs: unicode: Use strscpy() instead of strncpy()
fs: Check if utf8 encoding is loaded before calling utf8_unload()
fs: unicode: Rename function names from utf8 to unicode
fs: unicode: Rename utf8-core file to unicode-core
fs/ext4/hash.c | 2 +-
fs/ext4/namei.c | 12 ++---
fs/ext4/super.c | 8 +--
fs/f2fs/dir.c | 12 ++---
fs/f2fs/super.c | 11 ++--
fs/libfs.c | 6 +--
fs/unicode/Makefile | 2 +-
fs/unicode/{utf8-core.c => unicode-core.c} | 62 +++++++++++-----------
fs/unicode/utf8-selftest.c | 8 +--
include/linux/unicode.h | 32 +++++------
10 files changed, 81 insertions(+), 74 deletions(-)
rename fs/unicode/{utf8-core.c => unicode-core.c} (72%)
--
2.30.1
Following warning was reported by Kernel Test Robot.
In function 'utf8_parse_version',
inlined from 'utf8_load' at fs/unicode/utf8mod.c:195:7:
>> fs/unicode/utf8mod.c:175:2: warning: 'strncpy' specified bound 12 equals
destination size [-Wstringop-truncation]
175 | strncpy(version_string, version, sizeof(version_string));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The -Wstringop-truncation warning highlights the unintended
uses of the strncpy function that truncate the terminating NULL
character from the source string.
Unlike strncpy(), strscpy() always null-terminates the destination string,
hence use strscpy() instead of strncpy().
Fixes: 9d53690f0d4e5 (unicode: implement higher level API for string handling)
Signed-off-by: Shreeya Patel <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
Changes in v3
- Return error if strscpy() returns value < 0
Changes in v2
- Resolve warning of -Wstringop-truncation reported by
kernel test robot.
fs/unicode/utf8-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/unicode/utf8-core.c b/fs/unicode/utf8-core.c
index dc25823bf..706f086bb 100644
--- a/fs/unicode/utf8-core.c
+++ b/fs/unicode/utf8-core.c
@@ -180,7 +180,10 @@ static int utf8_parse_version(const char *version, unsigned int *maj,
{0, NULL}
};
- strncpy(version_string, version, sizeof(version_string));
+ int ret = strscpy(version_string, version, sizeof(version_string));
+
+ if (ret < 0)
+ return ret;
if (match_token(version_string, token, args) != 1)
return -EINVAL;
--
2.24.3 (Apple Git-128)
utf8_unload is being called if CONFIG_UNICODE is enabled.
The ifdef block doesn't check if utf8 encoding has been loaded
or not before calling the utf8_unload() function.
This is not the expected behavior since it would sometimes lead
to unloading utf8 even before loading it.
Hence, add a condition which will check if sb->encoding is NOT NULL
before calling the utf8_unload().
Signed-off-by: Shreeya Patel <[email protected]>
---
Changes in v3
- Add this patch to the series which checks if utf8 encoding
was loaded before calling uft8_unload().
fs/ext4/super.c | 6 ++++--
fs/f2fs/super.c | 9 ++++++---
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ad34a3727..e438d14f9 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1259,7 +1259,8 @@ static void ext4_put_super(struct super_block *sb)
fs_put_dax(sbi->s_daxdev);
fscrypt_free_dummy_policy(&sbi->s_dummy_enc_policy);
#ifdef CONFIG_UNICODE
- utf8_unload(sb->s_encoding);
+ if (sb->s_encoding)
+ utf8_unload(sb->s_encoding);
#endif
kfree(sbi);
}
@@ -5165,7 +5166,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
crypto_free_shash(sbi->s_chksum_driver);
#ifdef CONFIG_UNICODE
- utf8_unload(sb->s_encoding);
+ if (sb->s_encoding)
+ utf8_unload(sb->s_encoding);
#endif
#ifdef CONFIG_QUOTA
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 706979375..0a04983c2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1430,7 +1430,8 @@ static void f2fs_put_super(struct super_block *sb)
for (i = 0; i < NR_PAGE_TYPE; i++)
kvfree(sbi->write_io[i]);
#ifdef CONFIG_UNICODE
- utf8_unload(sb->s_encoding);
+ if (sb->s_encoding)
+ utf8_unload(sb->s_encoding);
#endif
kfree(sbi);
}
@@ -4073,8 +4074,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
kvfree(sbi->write_io[i]);
#ifdef CONFIG_UNICODE
- utf8_unload(sb->s_encoding);
- sb->s_encoding = NULL;
+ if (sb->s_encoding) {
+ utf8_unload(sb->s_encoding);
+ sb->s_encoding = NULL;
+ }
#endif
free_options:
#ifdef CONFIG_QUOTA
--
2.24.3 (Apple Git-128)
Rename the file name from utf8-core to unicode-core for transformation of
utf8-core file into the unicode subsystem layer file and also for better
understanding.
Signed-off-by: Shreeya Patel <[email protected]>
---
fs/unicode/Makefile | 2 +-
fs/unicode/{utf8-core.c => unicode-core.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename fs/unicode/{utf8-core.c => unicode-core.c} (100%)
diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
index b88aecc86..fbf9a629e 100644
--- a/fs/unicode/Makefile
+++ b/fs/unicode/Makefile
@@ -3,7 +3,7 @@
obj-$(CONFIG_UNICODE) += unicode.o
obj-$(CONFIG_UNICODE_NORMALIZATION_SELFTEST) += utf8-selftest.o
-unicode-y := utf8-norm.o utf8-core.o
+unicode-y := utf8-norm.o unicode-core.o
$(obj)/utf8-norm.o: $(obj)/utf8data.h
diff --git a/fs/unicode/utf8-core.c b/fs/unicode/unicode-core.c
similarity index 100%
rename from fs/unicode/utf8-core.c
rename to fs/unicode/unicode-core.c
--
2.24.3 (Apple Git-128)
Shreeya Patel <[email protected]> writes:
> Following warning was reported by Kernel Test Robot.
>
> In function 'utf8_parse_version',
> inlined from 'utf8_load' at fs/unicode/utf8mod.c:195:7:
>>> fs/unicode/utf8mod.c:175:2: warning: 'strncpy' specified bound 12 equals
> destination size [-Wstringop-truncation]
> 175 | strncpy(version_string, version, sizeof(version_string));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The -Wstringop-truncation warning highlights the unintended
> uses of the strncpy function that truncate the terminating NULL
> character from the source string.
> Unlike strncpy(), strscpy() always null-terminates the destination string,
> hence use strscpy() instead of strncpy().
>
> Fixes: 9d53690f0d4e5 (unicode: implement higher level API for string handling)
> Signed-off-by: Shreeya Patel <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> ---
>
> Changes in v3
> - Return error if strscpy() returns value < 0
>
> Changes in v2
> - Resolve warning of -Wstringop-truncation reported by
> kernel test robot.
>
> fs/unicode/utf8-core.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
Hi Shreeya,
Thanks for fixing this.
> diff --git a/fs/unicode/utf8-core.c b/fs/unicode/utf8-core.c
> index dc25823bf..706f086bb 100644
> --- a/fs/unicode/utf8-core.c
> +++ b/fs/unicode/utf8-core.c
> @@ -180,7 +180,10 @@ static int utf8_parse_version(const char *version, unsigned int *maj,
> {0, NULL}
> };
>
> - strncpy(version_string, version, sizeof(version_string));
> + int ret = strscpy(version_string, version, sizeof(version_string));
Usually, no spaces between variable declarations
Other than that,
Acked-by: Gabriel Krisman Bertazi <[email protected]>
> +
> + if (ret < 0)
> + return ret;
> if (match_token(version_string, token, args) != 1)
> return -EINVAL;
--
Gabriel Krisman Bertazi
Shreeya Patel <[email protected]> writes:
> utf8_unload is being called if CONFIG_UNICODE is enabled.
> The ifdef block doesn't check if utf8 encoding has been loaded
> or not before calling the utf8_unload() function.
> This is not the expected behavior since it would sometimes lead
> to unloading utf8 even before loading it.
> Hence, add a condition which will check if sb->encoding is NOT NULL
> before calling the utf8_unload().
Just to mention this used to be safe, since it was just doing a
kfree(NULL), but won't be anymore after the rest of this series.
Reviewed-by: Gabriel Krisman Bertazi <[email protected]>
>
> Signed-off-by: Shreeya Patel <[email protected]>
> ---
>
> Changes in v3
> - Add this patch to the series which checks if utf8 encoding
> was loaded before calling uft8_unload().
>
> fs/ext4/super.c | 6 ++++--
> fs/f2fs/super.c | 9 ++++++---
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index ad34a3727..e438d14f9 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1259,7 +1259,8 @@ static void ext4_put_super(struct super_block *sb)
> fs_put_dax(sbi->s_daxdev);
> fscrypt_free_dummy_policy(&sbi->s_dummy_enc_policy);
> #ifdef CONFIG_UNICODE
> - utf8_unload(sb->s_encoding);
> + if (sb->s_encoding)
> + utf8_unload(sb->s_encoding);
> #endif
> kfree(sbi);
> }
> @@ -5165,7 +5166,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> crypto_free_shash(sbi->s_chksum_driver);
>
> #ifdef CONFIG_UNICODE
> - utf8_unload(sb->s_encoding);
> + if (sb->s_encoding)
> + utf8_unload(sb->s_encoding);
> #endif
>
> #ifdef CONFIG_QUOTA
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 706979375..0a04983c2 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1430,7 +1430,8 @@ static void f2fs_put_super(struct super_block *sb)
> for (i = 0; i < NR_PAGE_TYPE; i++)
> kvfree(sbi->write_io[i]);
> #ifdef CONFIG_UNICODE
> - utf8_unload(sb->s_encoding);
> + if (sb->s_encoding)
> + utf8_unload(sb->s_encoding);
> #endif
> kfree(sbi);
> }
> @@ -4073,8 +4074,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> kvfree(sbi->write_io[i]);
>
> #ifdef CONFIG_UNICODE
> - utf8_unload(sb->s_encoding);
> - sb->s_encoding = NULL;
> + if (sb->s_encoding) {
> + utf8_unload(sb->s_encoding);
> + sb->s_encoding = NULL;
> + }
> #endif
> free_options:
> #ifdef CONFIG_QUOTA
--
Gabriel Krisman Bertazi
Shreeya Patel <[email protected]> writes:
> Rename the file name from utf8-core to unicode-core for transformation of
> utf8-core file into the unicode subsystem layer file and also for better
> understanding.
>
> Signed-off-by: Shreeya Patel <[email protected]>
Acked-by: Gabriel Krisman Bertazi <[email protected]>
Thanks,
> ---
> fs/unicode/Makefile | 2 +-
> fs/unicode/{utf8-core.c => unicode-core.c} | 0
> 2 files changed, 1 insertion(+), 1 deletion(-)
> rename fs/unicode/{utf8-core.c => unicode-core.c} (100%)
>
> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> index b88aecc86..fbf9a629e 100644
> --- a/fs/unicode/Makefile
> +++ b/fs/unicode/Makefile
> @@ -3,7 +3,7 @@
> obj-$(CONFIG_UNICODE) += unicode.o
> obj-$(CONFIG_UNICODE_NORMALIZATION_SELFTEST) += utf8-selftest.o
>
> -unicode-y := utf8-norm.o utf8-core.o
> +unicode-y := utf8-norm.o unicode-core.o
>
> $(obj)/utf8-norm.o: $(obj)/utf8data.h
>
> diff --git a/fs/unicode/utf8-core.c b/fs/unicode/unicode-core.c
> similarity index 100%
> rename from fs/unicode/utf8-core.c
> rename to fs/unicode/unicode-core.c
--
Gabriel Krisman Bertazi