Re: [f2fs-dev] [PATCH 4/8] vfs: Fold casefolding into vfs
On Mon, Dec 02, 2019 at 09:10:45PM -0800, Daniel Rosenberg wrote:
> @@ -228,6 +229,13 @@ static inline int dentry_string_cmp(const unsigned char
> *cs, const unsigned char
>
> #endif
>
> +bool needs_casefold(const struct inode *dir)
> +{
> + return IS_CASEFOLDED(dir) &&
> + (!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir));
> +}
> +EXPORT_SYMBOL(needs_casefold);
> +
I'd suggest adding a check to make sure that dir->i_sb->s_encoding is
non-NULL before needs_casefold() returns non-NULL. Otherwise a bug
(or a fuzzed file system) which manages to set the S_CASEFOLD flag without
having s_encoding be initialized might cause a NULL dereference.
Also, maybe make needs_casefold() an inline function which returns 0
if CONFIG_UNICODE is not defined? That way the need for #ifdef
CONFIG_UNICODE could be reduced.
> @@ -247,7 +255,19 @@ static inline int dentry_cmp(const struct dentry
> *dentry, const unsigned char *c
>* be no NUL in the ct/tcount data)
>*/
> const unsigned char *cs = READ_ONCE(dentry->d_name.name);
> +#ifdef CONFIG_UNICODE
> + struct inode *parent = dentry->d_parent->d_inode;
>
> + if (unlikely(needs_casefold(parent))) {
> + const struct qstr n1 = QSTR_INIT(cs, tcount);
> + const struct qstr n2 = QSTR_INIT(ct, tcount);
> + int result = utf8_strncasecmp(dentry->d_sb->s_encoding,
> + , );
> +
> + if (result >= 0 || sb_has_enc_strict_mode(dentry->d_sb))
> + return result;
> + }
> +#endif
This is an example of how we could drop the #ifdef CONFIG_UNICODE
(moving the declaration of 'parent' into the #if statement) if
needs_casefold() always returns 0 if !defined(CONFIG_UNICODE).
> @@ -2404,7 +2424,22 @@ struct dentry *d_hash_and_lookup(struct dentry *dir,
> struct qstr *name)
>* calculate the standard hash first, as the d_op->d_hash()
>* routine may choose to leave the hash value unchanged.
>*/
> +#ifdef CONFIG_UNICODE
> + unsigned char *hname = NULL;
> + int hlen = name->len;
> +
> + if (IS_CASEFOLDED(dir->d_inode)) {
> + hname = kmalloc(PATH_MAX, GFP_ATOMIC);
> + if (!hname)
> + return ERR_PTR(-ENOMEM);
> + hlen = utf8_casefold(dir->d_sb->s_encoding,
> + name, hname, PATH_MAX);
> + }
> + name->hash = full_name_hash(dir, hname ?: name->name, hlen);
> + kfree(hname);
> +#else
> name->hash = full_name_hash(dir, name->name, name->len);
> +#endif
Perhaps this could be refactored out? It's also used in
link_path_walk() and lookup_one_len_common().
(Note, there was some strageness in lookup_one_len_common(), where
hname is freed twice, the first time using kvfree() which I don't
believe is needed. But this can be fixed as part of the refactoring.)
- Ted
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fsck.f2fs: do not access nat etnries in ckpt before initialization
On 01/03, Chao Yu wrote: > On 2019/12/11 0:47, Jaegeuk Kim wrote: > > ckpt->entries is initialized by fsck_init(), but we tried to access it > > during > > f2fs_do_mount(). > > > > The call sequence is: > > - f2fs_do_mount > > - record_fsync_data > > - traverse_dnodes > > - do_record_fsync_data > > - ADDRS_PER_PAGE > >- get_node_info > > - node_info_from_raw_nat(fsck->entries[nid]) > > - do_fsck > > - fsck_init > >- build_nat_area_bitmap > > - fsck->entries = calloc(fsck->nr_nat_entries); > > > > Signed-off-by: Jaegeuk Kim > > Reviewed-by: Chao Yu Sorry, merged in master before. > > Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] fs-verity: implement readahead for FS_IOC_ENABLE_VERITY
On Tue, Dec 10, 2019 at 10:35:31AM -0800, Eric Biggers wrote: > From: Eric Biggers > > When it builds the first level of the Merkle tree, FS_IOC_ENABLE_VERITY > sequentially reads each page of the file using read_mapping_page(). > This works fine if the file's data is already in pagecache, which should > normally be the case, since this ioctl is normally used immediately > after writing out the file. > > But in any other case this implementation performs very poorly, since > only one page is read at a time. > > Fix this by implementing readahead using the functions from > mm/readahead.c. > > This improves performance in the uncached case by about 20x, as seen in > the following benchmarks done on a 250MB file (on x86_64 with SHA-NI): > > FS_IOC_ENABLE_VERITY uncached (before) 3.299s > FS_IOC_ENABLE_VERITY uncached (after) 0.160s > FS_IOC_ENABLE_VERITY cached0.147s > sha256sum uncached 0.191s > sha256sum cached 0.145s > > Note: we could instead switch to kernel_read(). But that would mean > we'd no longer be hashing the data directly from the pagecache, which is > a nice optimization of its own. And using kernel_read() would require > allocating another temporary buffer, hashing the data and tree pages > separately, and explicitly zero-padding the last page -- so it wouldn't > really be any simpler than direct pagecache access, at least for now. > > Signed-off-by: Eric Biggers > --- > > Changed v1 => v2: > - Only do sync readahead when the page wasn't found in the pagecache at all. > - Use ->f_mapping so that the inode doesn't have to be passed. > > > fs/verity/enable.c | 45 +++-- > 1 file changed, 39 insertions(+), 6 deletions(-) > Applied to fscrypt.git#fsverity for 5.6. - Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fscrypt: don't check for ENOKEY from fscrypt_get_encryption_info()
On Mon, Dec 09, 2019 at 01:23:48PM -0800, Eric Biggers wrote:
> From: Eric Biggers
>
> fscrypt_get_encryption_info() returns 0 if the encryption key is
> unavailable; it never returns ENOKEY. So remove checks for ENOKEY.
>
> Signed-off-by: Eric Biggers
> ---
> fs/ext4/dir.c | 2 +-
> fs/f2fs/dir.c | 2 +-
> fs/ubifs/dir.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 9fdd2b269d617..4c9d3ff394a5d 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -116,7 +116,7 @@ static int ext4_readdir(struct file *file, struct
> dir_context *ctx)
>
> if (IS_ENCRYPTED(inode)) {
> err = fscrypt_get_encryption_info(inode);
> - if (err && err != -ENOKEY)
> + if (err)
> return err;
> }
>
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index c967cacf979ef..d9ad842945df5 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -987,7 +987,7 @@ static int f2fs_readdir(struct file *file, struct
> dir_context *ctx)
>
> if (IS_ENCRYPTED(inode)) {
> err = fscrypt_get_encryption_info(inode);
> - if (err && err != -ENOKEY)
> + if (err)
> goto out;
>
> err = fscrypt_fname_alloc_buffer(inode, F2FS_NAME_LEN, );
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 0b98e3c8b461d..acc4f942d25b6 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -512,7 +512,7 @@ static int ubifs_readdir(struct file *file, struct
> dir_context *ctx)
>
> if (encrypted) {
> err = fscrypt_get_encryption_info(dir);
> - if (err && err != -ENOKEY)
> + if (err)
> return err;
>
> err = fscrypt_fname_alloc_buffer(dir, UBIFS_MAX_NLEN, );
> --
> 2.24.0.393.g34dc348eaf-goog
>
Applied to fscrypt.git#master for 5.6.
- Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fscrypt: introduce fscrypt_needs_contents_encryption()
On Mon, Dec 09, 2019 at 12:50:21PM -0800, Eric Biggers wrote:
> From: Eric Biggers
>
> Add a function fscrypt_needs_contents_encryption() which takes an inode
> and returns true if it's an encrypted regular file and the kernel was
> built with fscrypt support.
>
> This will allow replacing duplicated checks of IS_ENCRYPTED() &&
> S_ISREG() on the I/O paths in ext4 and f2fs, while also optimizing out
> unneeded code when !CONFIG_FS_ENCRYPTION.
>
> Signed-off-by: Eric Biggers
> ---
> include/linux/fscrypt.h | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index cb18b5fbcef92..2a29f56b1a1cb 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -72,6 +72,21 @@ static inline bool fscrypt_has_encryption_key(const struct
> inode *inode)
> return READ_ONCE(inode->i_crypt_info) != NULL;
> }
>
> +/**
> + * fscrypt_needs_contents_encryption() - check whether an inode needs
> + *contents encryption
> + *
> + * Return: %true iff the inode is an encrypted regular file and the kernel
> was
> + * built with fscrypt support.
> + *
> + * If you need to know whether the encrypt bit is set even when the kernel
> was
> + * built without fscrypt support, you must use IS_ENCRYPTED() directly
> instead.
> + */
> +static inline bool fscrypt_needs_contents_encryption(const struct inode
> *inode)
> +{
> + return IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode);
> +}
> +
> static inline bool fscrypt_dummy_context_enabled(struct inode *inode)
> {
> return inode->i_sb->s_cop->dummy_context &&
> @@ -269,6 +284,11 @@ static inline bool fscrypt_has_encryption_key(const
> struct inode *inode)
> return false;
> }
>
> +static inline bool fscrypt_needs_contents_encryption(const struct inode
> *inode)
> +{
> + return false;
> +}
> +
> static inline bool fscrypt_dummy_context_enabled(struct inode *inode)
> {
> return false;
> --
> 2.24.0.393.g34dc348eaf-goog
>
Applied to fscrypt.git#master for 5.6.
- Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] fscrypt: support passing a keyring key to FS_IOC_ADD_ENCRYPTION_KEY
On Tue, Nov 19, 2019 at 02:24:47PM -0800, Eric Biggers wrote: > From: Eric Biggers > > Extend the FS_IOC_ADD_ENCRYPTION_KEY ioctl to allow the raw key to be > specified by a Linux keyring key, rather than specified directly. > > This is useful because fscrypt keys belong to a particular filesystem > instance, so they are destroyed when that filesystem is unmounted. > Usually this is desired. But in some cases, userspace may need to > unmount and re-mount the filesystem while keeping the keys, e.g. during > a system update. This requires keeping the keys somewhere else too. > > The keys could be kept in memory in a userspace daemon. But depending > on the security architecture and assumptions, it can be preferable to > keep them only in kernel memory, where they are unreadable by userspace. > > We also can't solve this by going back to the original fscrypt API > (where for each file, the master key was looked up in the process's > keyring hierarchy) because that caused lots of problems of its own. > > Therefore, add the ability for FS_IOC_ADD_ENCRYPTION_KEY to accept a > Linux keyring key. This solves the problem by allowing userspace to (if > needed) save the keys securely in a Linux keyring for re-provisioning, > while still using the new fscrypt key management ioctls. > > This is analogous to how dm-crypt accepts a Linux keyring key, but the > key is then stored internally in the dm-crypt data structures rather > than being looked up again each time the dm-crypt device is accessed. > > Use a custom key type "fscrypt-provisioning" rather than one of the > existing key types such as "logon". This is strongly desired because it > enforces that these keys are only usable for a particular purpose: for > fscrypt as input to a particular KDF. Otherwise, the keys could also be > passed to any kernel API that accepts a "logon" key with any service > prefix, e.g. dm-crypt, UBIFS, or (recently proposed) AF_ALG. This would > risk leaking information about the raw key despite it ostensibly being > unreadable. Of course, this mistake has already been made for multiple > kernel APIs; but since this is a new API, let's do it right. > > This patch has been tested using an xfstest which I wrote to test it. > > Signed-off-by: Eric Biggers Applied to fscrypt.git#master for 5.6. - Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [Bug 206057] 5.5.0-rc2-next: f2fs is extremely slow, with ext4 system works well
https://bugzilla.kernel.org/show_bug.cgi?id=206057 --- Comment #5 from David Heidelberg (okias) (da...@ixit.cz) --- sadly, still same output. Also not sure why, sudo -i freezes and cannot be stopped by CTRL-C. -- You are receiving this mail because: You are watching the assignee of the bug. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH TEST] f2fs: compress: fix deadlock in prepare_compress_overwrite()
100.00%13.65% fsstress [f2fs] [k]
prepare_compress_overwrite
|
|--86.35%--prepare_compress_overwrite
| |
| |--46.78%--f2fs_read_multi_pages
| | |
| | |--19.81%--f2fs_decompress_end_io
| | | |
| | | --5.77%--unlock_page
| | |
| | |--19.53%--f2fs_get_dnode_of_data
In prepare_compress_overwrite(), we need to check cluster state before
read cluster data, otherwise, read normal cluster as a compressed one
will always cause failure and retry.
Signed-off-by: Chao Yu
---
fs/f2fs/compress.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 9e8fba78db4d..f993b4ce1970 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -570,8 +570,7 @@ static void set_cluster_dirty(struct compress_ctx *cc)
}
static int prepare_compress_overwrite(struct compress_ctx *cc,
- struct page **pagep, pgoff_t index, void **fsdata,
- bool prealloc)
+ struct page **pagep, pgoff_t index, void **fsdata)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(cc->inode);
struct address_space *mapping = cc->inode->i_mapping;
@@ -582,11 +581,20 @@ static int prepare_compress_overwrite(struct compress_ctx
*cc,
unsigned int start_idx = start_idx_of_cluster(cc);
int i, idx;
int ret;
+ bool prealloc;
+
+retry:
+ ret = is_compressed_cluster(cc);
+ if (ret <= 0)
+ return ret;
+
+ /* compressed case */
+ prealloc = (ret == CLUSTER_HAS_SPACE);
ret = f2fs_init_compress_ctx(cc);
if (ret)
return ret;
-retry:
+
/* keep page reference to avoid page reclaim */
for (i = 0; i < cc->cluster_size; i++) {
page = f2fs_pagecache_get_page(mapping, start_idx + i,
@@ -632,6 +640,7 @@ static int prepare_compress_overwrite(struct compress_ctx
*cc,
(idx <= i) ? 1 : 0);
cc->rpages[idx] = NULL;
}
+ kvfree(cc->rpages);
cc->nr_rpages = 0;
goto retry;
}
@@ -687,14 +696,8 @@ int f2fs_prepare_compress_overwrite(struct inode *inode,
.rpages = NULL,
.nr_rpages = 0,
};
- int ret = is_compressed_cluster();
-
- if (ret <= 0)
- return ret;
- /* compressed case */
- return prepare_compress_overwrite(, pagep, index,
- fsdata, ret == CLUSTER_HAS_SPACE);
+ return prepare_compress_overwrite(, pagep, index, fsdata);
}
bool f2fs_compress_write_end(struct inode *inode, void *fsdata,
--
2.18.0.rc1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: compress: fix NULL pointer dereference
BUG: kernel NULL pointer dereference, address:
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0 P4D 0
Oops: [#1] SMP PTI
CPU: 11 PID: 70359 Comm: kworker/u256:4 Tainted: G OE 5.5.0-rc1
#36
Hardware name: Xen HVM domU, BIOS 4.1.2_115-908.790. 06/05/2017
Workqueue: writeback wb_workfn (flush-251:0)
RIP: 0010:f2fs_write_single_data_page+0x4f/0x700 [f2fs]
Call Trace:
? __next_timer_interrupt+0xc0/0xc0
? finish_wait+0x32/0x70
? congestion_wait+0xa5/0x120
f2fs_write_multi_pages+0xc7/0x810 [f2fs]
f2fs_write_cache_pages+0x6c0/0x790 [f2fs]
? select_task_rq_fair+0x584/0x800
? atomic_notifier_chain_unregister+0x30/0x70
? __set_page_dirty_nobuffers+0x101/0x150
f2fs_write_data_pages+0x2cd/0x320 [f2fs]
? f2fs_update_inode+0x9c/0x4f0 [f2fs]
? do_writepages+0x1a/0x60
do_writepages+0x1a/0x60
__writeback_single_inode+0x3d/0x340
writeback_sb_inodes+0x225/0x4a0
wb_writeback+0xf7/0x320
? wb_workfn+0xa8/0x450
? _raw_spin_unlock_bh+0xa/0x20
wb_workfn+0xa8/0x450
? finish_task_switch+0x75/0x2a0
process_one_work+0x15e/0x3e0
worker_thread+0x4c/0x440
? rescuer_thread+0x350/0x350
kthread+0xf8/0x130
? kthread_unpark+0x70/0x70
ret_from_fork+0x35/0x40
In scenario of truncate vs writeback, we need to check page's mapping
before access it during writeback.
Signed-off-by: Chao Yu
---
fs/f2fs/compress.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index fa67ffd9d79d..9e8fba78db4d 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -932,6 +932,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
enum iostat_type io_type,
bool compressed)
{
+ struct address_space *mapping = cc->inode->i_mapping;
int i, _submitted;
int ret, err = 0;
@@ -939,6 +940,11 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
if (!cc->rpages[i])
continue;
retry_write:
+ if (cc->rpages[i]->mapping != mapping) {
+ unlock_page(cc->rpages[i]);
+ continue;
+ }
+
BUG_ON(!PageLocked(cc->rpages[i]));
ret = f2fs_write_single_data_page(cc->rpages[i], &_submitted,
--
2.18.0.rc1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fsck.f2fs: do not access nat etnries in ckpt before initialization
On 2019/12/11 0:47, Jaegeuk Kim wrote: > ckpt->entries is initialized by fsck_init(), but we tried to access it during > f2fs_do_mount(). > > The call sequence is: > - f2fs_do_mount > - record_fsync_data > - traverse_dnodes > - do_record_fsync_data > - ADDRS_PER_PAGE >- get_node_info > - node_info_from_raw_nat(fsck->entries[nid]) > - do_fsck > - fsck_init >- build_nat_area_bitmap > - fsck->entries = calloc(fsck->nr_nat_entries); > > Signed-off-by: Jaegeuk Kim Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
