Hi,
André Almeida <andrealmeid@xxxxxxxxxx> writes:
> @@ -3427,6 +3431,10 @@ shmem_mknod(struct mnt_idmap *idmap, struct inode *dir,
> if (IS_ERR(inode))
> return PTR_ERR(inode);
>
> + if (IS_ENABLED(CONFIG_UNICODE))
> + if (!generic_ci_validate_strict_name(dir, &dentry->d_name))
> + return -EINVAL;
> +
if (IS_ENABLED(CONFIG_UNICODE) &&
generic_ci_validate_strict_name(dir, &dentry->d_name))
> static const struct constant_table shmem_param_enums_huge[] = {
> @@ -4081,9 +4111,62 @@ const struct fs_parameter_spec shmem_fs_parameters[] = {
> fsparam_string("grpquota_block_hardlimit", Opt_grpquota_block_hardlimit),
> fsparam_string("grpquota_inode_hardlimit", Opt_grpquota_inode_hardlimit),
> #endif
> + fsparam_string("casefold", Opt_casefold_version),
> + fsparam_flag ("casefold", Opt_casefold),
> + fsparam_flag ("strict_encoding", Opt_strict_encoding),
I don't know if it is possible, but can we do it with a single parameter?
> +static int shmem_parse_opt_casefold(struct fs_context *fc, struct fs_parameter *param,
> + bool latest_version)
Instead of the boolean, can't you check if param->string != NULL? (real
question, I never used fs_parameter.
> +{
> + struct shmem_options *ctx = fc->fs_private;
> + unsigned int maj = 0, min = 0, rev = 0, version = 0;
> + struct unicode_map *encoding;
> + char *version_str = param->string + 5;
> + int ret;
unsigned int version = UTF8_LATEST;
and kill the if/else below:
> +
> + if (latest_version) {
> + version = UTF8_LATEST;
> + } else {
> + if (strncmp(param->string, "utf8-", 5))
> + return invalfc(fc, "Only UTF-8 encodings are supported "
> + "in the format: utf8-<version number>");
> +
> + ret = utf8_parse_version(version_str, &maj, &min, &rev);
utf8_parse_version interface could return UNICODE_AGE() already, so we hide the details
from the caller. wdyt?
> + if (ret)
> + return invalfc(fc, "Invalid UTF-8 version: %s", version_str);
> +
> + version = UNICODE_AGE(maj, min, rev);
> + }
> +
> + encoding = utf8_load(version);
> +
> + if (IS_ERR(encoding)) {
> + if (latest_version)
> + return invalfc(fc, "Failed loading latest UTF-8 version");
> + else
> + return invalfc(fc, "Failed loading UTF-8 version: %s", version_str);
The following covers both legs (untested):
if (IS_ERR(encoding))
return invalfc(fc, "Failed loading UTF-8 version: utf8-%u.%u.%u\n"",
unicode_maj(version), unicode_min(version), unicode_rev(version));
> + if (latest_version)
> + pr_info("tmpfs: Using the latest UTF-8 version available");
> + else
> + pr_info("tmpfs: Using encoding provided by mount
> options: %s\n", param->string);
The following covers both legs (untested):
pr_info (fc, "tmpfs: Using encoding : utf8-%u.%u.%u\n"
unicode_maj(version), unicode_min(version), unicode_rev(version));
> +
> + ctx->encoding = encoding;
> +
> + return 0;
> +}
> +#else
> +static int shmem_parse_opt_casefold(struct fs_context *fc, struct fs_parameter *param,
> + bool latest_version)
> +{
> + return invalfc(fc, "tmpfs: No kernel support for casefold filesystems\n");
> +}
A message like "Kernel not built with CONFIG_UNICODE" immediately tells
you how to fix it.
> @@ -4515,6 +4610,16 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
> }
> sb->s_export_op = &shmem_export_ops;
> sb->s_flags |= SB_NOSEC | SB_I_VERSION;
> +
> +#if IS_ENABLED(CONFIG_UNICODE)
> + if (ctx->encoding) {
> + sb->s_encoding = ctx->encoding;
> + generic_set_sb_d_ops(sb);
This is the right place for setting d_ops (see the next comment), but you
should be loading generic_ci_always_del_dentry_ops, right?
Also, since generic_ci_always_del_dentry_ops is only used by this one,
can you move it to this file?
> +static struct dentry *shmem_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
> +{
> + const struct dentry_operations *d_ops = &simple_dentry_operations;
> +
> +#if IS_ENABLED(CONFIG_UNICODE)
> + if (dentry->d_sb->s_encoding)
> + d_ops = &generic_ci_always_del_dentry_ops;
> +#endif
This needs to be done at mount time through sb->s_d_op. See
https://lore.kernel.org/all/20240221171412.10710-1-krisman@xxxxxxx/
I suppose we can do it at mount-time for
generic_ci_always_del_dentry_ops and simple_dentry_operations.
> +
> + if (dentry->d_name.len > NAME_MAX)
> + return ERR_PTR(-ENAMETOOLONG);
> +
> + if (!dentry->d_sb->s_d_op)
> + d_set_d_op(dentry, d_ops);
> +
> + /*
> + * For now, VFS can't deal with case-insensitive negative dentries, so
> + * we prevent them from being created
> + */
> + if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir))
> + return NULL;
Thinking out loud:
I misunderstood always_delete_dentry before. It removes negative
dentries right after the lookup, since ->d_delete is called on dput.
But you still need this check here, IMO, to prevent the negative dentry
from ever being hashed. Otherwise it can be found by a concurrent
lookup. And you cannot drop ->d_delete from the case-insensitive
operations too, because we still wants it for !IS_CASEFOLDED(dir).
The window is that, without this code, the negative dentry dentry would
be hashed in d_add() and a concurrent lookup might find it between that
time and the d_put, where it is removed at the end of the concurrent
lookup.
All of this would hopefully go away with the negative dentry for
casefolded directories.
> +
> + d_add(dentry, NULL);
> +
> + return NULL;
> +}
The sole reason you are doing this custom function is to exclude negative
dentries from casefolded directories. I doubt we care about the extra
check being done. Can we just do it in simple_lookup?
> +
> static const struct inode_operations shmem_dir_inode_operations = {
> #ifdef CONFIG_TMPFS
> .getattr = shmem_getattr,
> .create = shmem_create,
> - .lookup = simple_lookup,
> + .lookup = shmem_lookup,
> .link = shmem_link,
> .unlink = shmem_unlink,
> .symlink = shmem_symlink,
> @@ -4791,6 +4923,8 @@ int shmem_init_fs_context(struct fs_context *fc)
> ctx->uid = current_fsuid();
> ctx->gid = current_fsgid();
>
> + ctx->encoding = NULL;
> +
> fc->fs_private = ctx;
> fc->ops = &shmem_fs_context_ops;
> return 0;
--
Gabriel Krisman Bertazi
