Re: [f2fs-dev] Possible issues with fsck of f2fs root
On 2019/4/24 0:17, Hagbard Celine wrote: > 2019-04-23 4:55 GMT+02:00, Chao Yu : >> On 2019/4/22 18:05, Hagbard Celine wrote: >>> 2019-04-22 11:26 GMT+02:00, Chao Yu : On 2019/4/22 17:05, Hagbard Celine wrote: > 2019-04-22 9:37 GMT+02:00, Chao Yu : >> On 2019/4/22 15:11, Hagbard Celine wrote: >>> With this patch the one problem with opening the device in RO mode is >>> fixed. >> >> Oops, with default preen mode fsck should not open ro mounted image, >> that's >> the >> rule we keep line with ext4... >> >> How about changing to use -f in your scenario ( on RO mounted root >> image >> )? > > This was with -f. Without -f it still refuses to open the device. What I mean is we'd better to keep line with ext4, just refusing to open ro mounted device without -f, since triggering fsck and repair on a mounted device is dangerous, it can easily make inconsistency in between in-memory data and on-disk data of filesystem. Refusing fsck without -f is to make user being aware of such danger. >>> >>> I am sorry, I've apparently added the -f after my first report. After >>> re-testing it seems that fsck.f2fs is opening the RO partition even >>> without this patch if I use -f. So the part about fsck.f2fs not being >>> able to open RO mounted partition during boot was a user error. >> >> I've sent a patch for your second issue, could you please have a try with >> it? >> >> [PATCH] fsck.f2fs: fix to repair ro mounted device w/ -f >> >> But one concern is that, with this patch, not like the fsck.ext4, fsck.f2fs >> won't show any interaction with below reminding word to remind user to >> decide >> repair or not, it may increase the risk of damaging the device. >> >> Do you want to restore lost files into ./lost_found/? >> Do you want to fix this partition? [Y/N] >> >> Jaegeuk, Hagbard, >> >> Any suggestion on this, in current scenario, how about implement: >> 1. fsck.f2fs -f ro_mounted_device: check; show interaction words if there >> is >> corruption; >> 2. fsck.f2fs -f -a ro_moutned_device: check and repair automatically; > > I answered this all too quickly and did not think it trough properly. > As it stands today, if I run "fsck.f2fs -f /dev/some_unmounted_disk" > it will always do a full fsck. > If I on the other hand do "fsck.f2fs -f -a /dev/some_unmounted_disk" > it sometimes only reads the checkpoint state and returns with: "Info: > No errors was reported". > I do not have a ext4 partition with errors to test, but I have a fat It can easily be generated by debugfs with: debugfs: open /dev/zram0 -w debugfs: sif file extra_isize 19 > partition that comes up with "Free cluster summary wrong" on every run > of fsck.fat and there fsck asks for confirmation when run with "-f" > and autofixes without asking when running with "-f -a". For some_unmounted_disk, fsck.ext4 shows the same behavior like fsck.fat: # fsck.ext4 /dev/zram0 -f e2fsck 1.45.0 (6-Mar-2019) Pass 1: Checking inodes, blocks, and sizes Inode 12 has a extra size (19) which is invalid Fix? /dev/zram0: e2fsck canceled. # fsck.ext4 /dev/zram0 -f -a /dev/zram0: Inode 12 has a extra size (19) which is invalid FIXED. Also on a RO_mounted_disk, the behavior of fsck.ext4 is the same, so, it looks like we actually need another patch to fix fsck.f2fs' behavior for "-f" and "-f -a" on umounted/ro_mounted device. Thanks, > > Considering this I believe the proposed solution would be > counter-intuitive, unless fsck.ext4 behaves opposite of fsck.fat > already. >> >> Thanks, >> >>> Thanks, > > >> Thanks, >> >>> But as far as I can understand it will still only check the fs, not >>> fix >>> it. >>> >>> >>> 2019-04-21 12:27 GMT+02:00, Jaegeuk Kim : >>> New version of the patch is: From 3221692b060649378f1f69b898ed85a814af3dbf Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Tue, 16 Apr 2019 11:46:31 -0700 Subject: [PATCH] fsck.f2fs: open ro disk if we want to check fs only This patch fixes the "open failure" issue on ro disk, reported by Hagbard. " If I boot with kernel option "ro rootfstype=f2fs I get the following halfway trough boot: * Checking local filesystems ... Info: Use default preen mode Info: Mounted device! Info: Check FS only due to RO Error: Failed to open the device! * Filesystems couldn't be fixed " Reported-by: Hagbard Celine Signed-off-by: Jaegeuk Kim --- lib/libf2fs.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/libf2fs.c b/lib/libf2fs.c index d30047f..853e713 100644 --- a/lib/libf2fs.c +++ b/lib/l
Re: [f2fs-dev] [PATCH 1/2] f2fs: allow unfixed f2fs_checkpoint.checksum_offset
On 2019/4/24 4:56, Jaegeuk Kim wrote:
> On 04/23, Jaegeuk Kim wrote:
>> On 04/22, Chao Yu wrote:
>>> Previously, f2fs_checkpoint.checksum_offset points fixed position of
>>> f2fs_checkpoint structure:
>>>
>>> "#define CP_CHKSUM_OFFSET 4092"
>>>
>>> It is unnecessary, and it breaks the consecutiveness of nat and sit
>>> bitmap stored across checkpoint park block and payload blocks.
>>>
>>> This patch allows f2fs to handle unfixed .checksum_offset.
>>>
>>> In addition, for the case checksum value is stored in the middle of
>>> checkpoint park, calculating checksum value with superposition method
>>> like we did for inode_checksum.
>>>
>>> Signed-off-by: Chao Yu
>>> ---
>>> fs/f2fs/checkpoint.c| 27 +--
>>> include/linux/f2fs_fs.h | 4
>>> 2 files changed, 25 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index 441814607b13..a25556aef8cc 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -794,13 +794,27 @@ static void write_orphan_inodes(struct f2fs_sb_info
>>> *sbi, block_t start_blk)
>>> }
>>> }
>>>
>>> +static __u32 f2fs_checkpoint_chksum(struct f2fs_sb_info *sbi,
>>> + struct f2fs_checkpoint *ckpt)
>>> +{
>>> + unsigned int chksum_ofs = le32_to_cpu(ckpt->checksum_offset);
>>> + __u32 chksum;
>>> +
>>> + chksum = f2fs_crc32(sbi, ckpt, chksum_ofs);
>>> + if (chksum_ofs < CP_CHKSUM_OFFSET) {
>>> + chksum_ofs += sizeof(chksum);
>>> + chksum = f2fs_chksum(sbi, chksum, (__u8 *)ckpt + chksum_ofs,
>>> + F2FS_BLKSIZE - chksum_ofs);
>>
>> Do we need to cover __cp_payload(sbi) * blksize - chksum_ofs?
>
> Self answer - it'd be fine to get 4KB only, since payload will be covered
> by entire checkpoint pack.
Yup, ;)
Thanks,
>
>>
>>> + }
>>> + return chksum;
>>> +}
>>> +
>>> static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t
>>> cp_addr,
>>> struct f2fs_checkpoint **cp_block, struct page **cp_page,
>>> unsigned long long *version)
>>> {
>>> - unsigned long blk_size = sbi->blocksize;
>>> size_t crc_offset = 0;
>>> - __u32 crc = 0;
>>> + __u32 crc;
>>>
>>> *cp_page = f2fs_get_meta_page(sbi, cp_addr);
>>> if (IS_ERR(*cp_page))
>>> @@ -809,15 +823,16 @@ static int get_checkpoint_version(struct f2fs_sb_info
>>> *sbi, block_t cp_addr,
>>> *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
>>>
>>> crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
>>> - if (crc_offset > (blk_size - sizeof(__le32))) {
>>> + if (crc_offset < CP_MIN_CHKSUM_OFFSET ||
>>> + crc_offset > CP_CHKSUM_OFFSET) {
>>> f2fs_put_page(*cp_page, 1);
>>> f2fs_msg(sbi->sb, KERN_WARNING,
>>> "invalid crc_offset: %zu", crc_offset);
>>> return -EINVAL;
>>> }
>>>
>>> - crc = cur_cp_crc(*cp_block);
>>> - if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
>>> + crc = f2fs_checkpoint_chksum(sbi, *cp_block);
>>> + if (crc != cur_cp_crc(*cp_block)) {
>>> f2fs_put_page(*cp_page, 1);
>>> f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value");
>>> return -EINVAL;
>>> @@ -1425,7 +1440,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi,
>>> struct cp_control *cpc)
>>> get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP));
>>> get_nat_bitmap(sbi, __bitmap_ptr(sbi, NAT_BITMAP));
>>>
>>> - crc32 = f2fs_crc32(sbi, ckpt, le32_to_cpu(ckpt->checksum_offset));
>>> + crc32 = f2fs_checkpoint_chksum(sbi, ckpt);
>>> *((__le32 *)((unsigned char *)ckpt +
>>> le32_to_cpu(ckpt->checksum_offset)))
>>> = cpu_to_le32(crc32);
>>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
>>> index 55da9abed023..65559900d4d7 100644
>>> --- a/include/linux/f2fs_fs.h
>>> +++ b/include/linux/f2fs_fs.h
>>> @@ -164,6 +164,10 @@ struct f2fs_checkpoint {
>>> unsigned char sit_nat_version_bitmap[1];
>>> } __packed;
>>>
>>> +#define CP_CHKSUM_OFFSET 4092/* default chksum offset in checkpoint
>>> */
>>> +#define CP_MIN_CHKSUM_OFFSET
>>> \
>>> + (offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap))
>>> +
>>> /*
>>> * For orphan inode management
>>> */
>>> --
>>> 2.18.0.rc1
>>
>>
>> ___
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V1 02/14] Consolidate "post read processing" into a new file
Hi Chandan,
On 04/24, Chandan Rajendra wrote:
> The post read processing code is used by both Ext4 and F2FS. Hence to
> remove duplicity, this commit moves the code into
> include/linux/post_read_process.h and fs/post_read_process.c.
>
> The corresponding decrypt and verity "work" functions have been moved
> inside fscrypt and fsverity sources. With these in place, the post
> processing code now has to just invoke enqueue functions provided by
> fscrypt and fsverity.
>
> Signed-off-by: Chandan Rajendra
> ---
> fs/Makefile | 4 +
> fs/crypto/bio.c | 23 ++--
> fs/crypto/crypto.c| 17 +--
> fs/crypto/fscrypt_private.h | 3 +
> fs/ext4/ext4.h| 2 -
> fs/ext4/readpage.c| 175 --
> fs/ext4/super.c | 9 +-
> fs/f2fs/data.c| 146 -
> fs/f2fs/super.c | 9 +-
> fs/post_read_process.c| 136 +++
> fs/verity/verify.c| 12 ++
> include/linux/fscrypt.h | 20 +---
> include/linux/post_read_process.h | 21
> 13 files changed, 240 insertions(+), 337 deletions(-)
> create mode 100644 fs/post_read_process.c
> create mode 100644 include/linux/post_read_process.h
>
> diff --git a/fs/Makefile b/fs/Makefile
> index 9dd2186e74b5..f9abc3f71d3c 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -21,6 +21,10 @@ else
> obj-y += no-block.o
> endif
>
> +ifeq (y, $(firstword $(filter y,$(CONFIG_FS_ENCRYPTION)
> $(CONFIG_FS_VERITY
> +obj-y += post_read_process.o
> +endif
> +
> obj-$(CONFIG_PROC_FS) += proc_namespace.o
>
> obj-y+= notify/
> diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> index 5759bcd018cd..3e40d65ae6a8 100644
> --- a/fs/crypto/bio.c
> +++ b/fs/crypto/bio.c
> @@ -24,6 +24,8 @@
> #include
> #include
> #include
> +#include
> +
> #include "fscrypt_private.h"
>
> static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> @@ -54,24 +56,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
> }
> EXPORT_SYMBOL(fscrypt_decrypt_bio);
>
> -static void completion_pages(struct work_struct *work)
> +void fscrypt_decrypt_work(struct work_struct *work)
> {
> - struct fscrypt_ctx *ctx =
> - container_of(work, struct fscrypt_ctx, r.work);
> - struct bio *bio = ctx->r.bio;
> + struct bio_post_read_ctx *ctx =
> + container_of(work, struct bio_post_read_ctx, work);
>
> - __fscrypt_decrypt_bio(bio, true);
> - fscrypt_release_ctx(ctx);
> - bio_put(bio);
> -}
> + fscrypt_decrypt_bio(ctx->bio);
>
> -void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
> -{
> - INIT_WORK(&ctx->r.work, completion_pages);
> - ctx->r.bio = bio;
> - fscrypt_enqueue_decrypt_work(&ctx->r.work);
> + bio_post_read_processing(ctx);
> }
> -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
>
> void fscrypt_pullback_bio_page(struct page **page, bool restore)
> {
> @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
> restore)
> ctx = (struct fscrypt_ctx *)page_private(bounce_page);
>
> /* restore control page */
> - *page = ctx->w.control_page;
> + *page = ctx->control_page;
>
> if (restore)
> fscrypt_restore_control_page(bounce_page);
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 3fc84bf2b1e5..ffa9302a7351 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -53,6 +53,7 @@ struct kmem_cache *fscrypt_info_cachep;
>
> void fscrypt_enqueue_decrypt_work(struct work_struct *work)
> {
> + INIT_WORK(work, fscrypt_decrypt_work);
> queue_work(fscrypt_read_workqueue, work);
> }
> EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
> @@ -70,11 +71,11 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
> {
> unsigned long flags;
>
> - if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->w.bounce_page) {
> - mempool_free(ctx->w.bounce_page, fscrypt_bounce_page_pool);
> - ctx->w.bounce_page = NULL;
> + if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->bounce_page) {
> + mempool_free(ctx->bounce_page, fscrypt_bounce_page_pool);
> + ctx->bounce_page = NULL;
> }
> - ctx->w.control_page = NULL;
> + ctx->control_page = NULL;
> if (ctx->flags & FS_CTX_REQUIRES_FREE_ENCRYPT_FL) {
> kmem_cache_free(fscrypt_ctx_cachep, ctx);
> } else {
> @@ -194,11 +195,11 @@ int fscrypt_do_page_crypto(const struct inode *inode,
> fscrypt_direction_t rw,
> struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
> gfp_t gfp_flags)
> {
> - ctx->w.bounce_page = mempool_alloc(fscrypt_bounce_page_pool, gfp_flags);
> - if (ctx->w.bounce_page == NULL)
> + ctx->bounce_page = mempool_alloc(fscrypt_bounce_p
Re: [f2fs-dev] [PATCH v2 4/5] f2fs: add tracepoint for f2fs_filemap_fault()
On 04/15, Chao Yu wrote:
> This patch adds tracepoint for f2fs_filemap_fault().
>
> Signed-off-by: Chao Yu
> ---
> v2:
> - fix wrong type of @ret parameter
> fs/f2fs/file.c | 2 ++
> include/trace/events/f2fs.h | 26 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 30d49467578e..578486e03427 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -39,6 +39,8 @@ static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
> ret = filemap_fault(vmf);
> up_read(&F2FS_I(inode)->i_mmap_sem);
>
> + trace_f2fs_filemap_fault(inode, vmf->pgoff, ret);
In order to avoid wrong casting warning, how about this?
---
include/trace/events/f2fs.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index abe763cc1d0b..c29dea7ac0fe 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -1286,7 +1286,7 @@ DEFINE_EVENT(f2fs__page, f2fs_commit_inmem_page,
TRACE_EVENT(f2fs_filemap_fault,
- TP_PROTO(struct inode *inode, pgoff_t index, enum vm_fault_reason ret),
+ TP_PROTO(struct inode *inode, pgoff_t index, vm_fault_t ret),
TP_ARGS(inode, index, ret),
@@ -1294,7 +1294,7 @@ TRACE_EVENT(f2fs_filemap_fault,
__field(dev_t, dev)
__field(ino_t, ino)
__field(pgoff_t, index)
- __field(enum vm_fault_reason, ret)
+ __field(vm_fault_t, ret)
),
TP_fast_assign(
@@ -1304,10 +1304,10 @@ TRACE_EVENT(f2fs_filemap_fault,
__entry->ret= ret;
),
- TP_printk("dev = (%d,%d), ino = %lu, index = %lu, ret = %u",
+ TP_printk("dev = (%d,%d), ino = %lu, index = %lu, ret = %lx",
show_dev_ino(__entry),
(unsigned long)__entry->index,
- ret)
+ (unsigned long)__entry->ret)
);
TRACE_EVENT(f2fs_writepages,
--
2.19.0.605.g01d371f741-goog
___
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 2/2] f2fs: introduce DATA_GENERIC_ENHANCE
On 04/15, Chao Yu wrote:
> Previously, f2fs_is_valid_blkaddr(, blkaddr, DATA_GENERIC) will check
> whether @blkaddr locates in main area or not.
>
> That check is weak, since the block address in range of main area can
> point to the address which is not valid in segment info table, and we
> can not detect such condition, we may suffer worse corruption as system
> continues running.
>
> So this patch introduce DATA_GENERIC_ENHANCE to enhance the sanity check
> which trigger SIT bitmap check rather than only range check.
>
> This patch did below changes as wel:
> - set SBI_NEED_FSCK in f2fs_is_valid_blkaddr().
> - get rid of is_valid_data_blkaddr() to avoid panic if blkaddr is invalid.
> - introduce verify_fio_blkaddr() to wrap fio {new,old}_blkaddr validation
> check.
> - spread blkaddr check in:
> * f2fs_get_node_info()
> * __read_out_blkaddrs()
> * f2fs_submit_page_read()
> * ra_data_block()
> * do_recover_data()
>
> This patch can fix bug reported from bugzilla below:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=203215
> https://bugzilla.kernel.org/show_bug.cgi?id=203223
> https://bugzilla.kernel.org/show_bug.cgi?id=203231
> https://bugzilla.kernel.org/show_bug.cgi?id=203235
> https://bugzilla.kernel.org/show_bug.cgi?id=203241
Hi Chao,
This introduces failures on xfstests/generic/446, and I'm testing the below
patch on top of this. Could you check this patch, so that I could combine
both of them?
>From 8c1808c1743ad75d1ad8d1dc5a53910edaf7afd7 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim
Date: Wed, 24 Apr 2019 00:21:07 +0100
Subject: [PATCH] f2fs: consider data race on read and truncation on
DATA_GENERIC_ENHANCE
DATA_GENERIC_ENHANCE enhanced to validate block addresses on read/write paths.
But, xfstest/generic/446 compalins some generated kernel messages saying invalid
bitmap was detected when reading a block. The reaons is, when we get the
block addresses from extent_cache, there is no lock to synchronize it from
truncating the blocks in parallel.
This patch tries to return EFAULT without warning and setting SBI_NEED_FSCK
in this case.
Fixes: ("f2fs: introduce DATA_GENERIC_ENHANCE")
Signed-off-by: Jaegeuk Kim
---
fs/f2fs/checkpoint.c | 35 ++-
fs/f2fs/data.c | 25 ++---
fs/f2fs/f2fs.h | 6 ++
fs/f2fs/gc.c | 9 ++---
4 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index e37fbbf843a5..805a33088e82 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -130,26 +130,28 @@ struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi,
pgoff_t index)
return __get_meta_page(sbi, index, false);
}
-static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr)
+static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr,
+ int type)
{
struct seg_entry *se;
unsigned int segno, offset;
bool exist;
+ if (type != DATA_GENERIC_ENHANCE && type != DATA_GENERIC_ENHANCE_READ)
+ return true;
+
segno = GET_SEGNO(sbi, blkaddr);
offset = GET_BLKOFF_FROM_SEG0(sbi, blkaddr);
se = get_seg_entry(sbi, segno);
exist = f2fs_test_bit(offset, se->cur_valid_map);
-
- if (!exist) {
+ if (!exist && type == DATA_GENERIC_ENHANCE) {
f2fs_msg(sbi->sb, KERN_ERR, "Inconsistent error "
"blkaddr:%u, sit bitmap:%d", blkaddr, exist);
set_sbi_flag(sbi, SBI_NEED_FSCK);
WARN_ON(1);
- return false;
}
- return true;
+ return exist;
}
bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
@@ -173,23 +175,22 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
return false;
break;
case META_POR:
+ if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
+ blkaddr < MAIN_BLKADDR(sbi)))
+ return false;
+ break;
case DATA_GENERIC:
case DATA_GENERIC_ENHANCE:
+ case DATA_GENERIC_ENHANCE_READ:
if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
- blkaddr < MAIN_BLKADDR(sbi))) {
- if (type == DATA_GENERIC ||
- type == DATA_GENERIC_ENHANCE) {
- f2fs_msg(sbi->sb, KERN_WARNING,
- "access invalid blkaddr:%u", blkaddr);
- set_sbi_flag(sbi, SBI_NEED_FSCK);
- WARN_ON(1);
- }
+ blkaddr < MAIN_BLKADDR(sbi))) {
+ f2fs_msg(sbi->sb, KERN_WARNING,
+ "access invalid blkaddr:%u", blkaddr);
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ WARN_ON(1);
[f2fs-dev] [PATCH] f2fs-tools: get rid of unneeded fields in on-disk inode
As Jaegeuk reminded:
Once user updates f2fs-tools which support new fields in inode layout,
but do keep the kernel which can not support those fields, it will cause
old f2fs fail to mount new image due to root_inode's i_extra_isize value
sanity check.
So if f2fs-tools doesn't enable feature which will use new fields of
inode, we don't need to expand i_extra_isize to include them, let's just
let i_extra_isize point to the end of last valid extra field's position.
Signed-off-by: Chao Yu
---
fsck/dir.c | 3 +--
fsck/fsck.c| 6 +++---
fsck/segment.c | 2 +-
include/f2fs_fs.h | 1 +
lib/libf2fs.c | 16
mkfs/f2fs_format.c | 9 +++--
6 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/fsck/dir.c b/fsck/dir.c
index f1665c4..592d9db 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -461,8 +461,7 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
node_blk->i.i_inline |= F2FS_EXTRA_ATTR;
- node_blk->i.i_extra_isize =
- cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE);
+ node_blk->i.i_extra_isize = cpu_to_le16(calc_extra_isize());
}
node_blk->footer.ino = cpu_to_le32(de->ino);
diff --git a/fsck/fsck.c b/fsck/fsck.c
index a1791d4..baec041 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -733,12 +733,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
if (node_blk->i.i_extra_isize >
cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE)) {
FIX_MSG("ino[0x%x] recover i_extra_isize "
- "from %u to %lu",
+ "from %u to %u",
nid,
le16_to_cpu(node_blk->i.i_extra_isize),
- F2FS_TOTAL_EXTRA_ATTR_SIZE);
+ calc_extra_isize());
node_blk->i.i_extra_isize =
- cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE);
+ cpu_to_le16(calc_extra_isize());
need_fix = 1;
}
} else {
diff --git a/fsck/segment.c b/fsck/segment.c
index ba57a82..d1c569f 100644
--- a/fsck/segment.c
+++ b/fsck/segment.c
@@ -342,7 +342,7 @@ int f2fs_build_file(struct f2fs_sb_info *sbi, struct dentry
*de)
if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
node_blk->i.i_inline |= F2FS_EXTRA_ATTR;
node_blk->i.i_extra_isize =
- cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE);
+ cpu_to_le16(calc_extra_isize());
}
n = read(fd, buffer, BLOCK_SZ);
ASSERT((unsigned long)n == de->size);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 6def302..b766f7c 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1158,6 +1158,7 @@ extern int f2fs_devs_are_umounted(void);
extern int f2fs_dev_is_writable(void);
extern int f2fs_dev_is_umounted(char *);
extern int f2fs_get_device_info(void);
+extern unsigned int calc_extra_isize(void);
extern int get_device_info(int);
extern int f2fs_init_sparse_file(void);
extern int f2fs_finalize_device(void);
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index fd8bd5f..9a6a779 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -1191,3 +1191,19 @@ int f2fs_get_device_info(void)
(c.sector_size >> 9)) >> 11);
return 0;
}
+
+unsigned int calc_extra_isize(void)
+{
+ unsigned int size = offsetof(struct f2fs_inode, i_projid);
+
+ if (c.feature & cpu_to_le32(F2FS_FEATURE_FLEXIBLE_INLINE_XATTR))
+ size = offsetof(struct f2fs_inode, i_projid);
+ if (c.feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA));
+ size = offsetof(struct f2fs_inode, i_inode_checksum);
+ if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM));
+ size = offsetof(struct f2fs_inode, i_crtime);
+ if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CRTIME));
+ size = offsetof(struct f2fs_inode, i_extra_end);
+
+ return size;
+}
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 2bf46ae..a0fb68f 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -1132,8 +1132,7 @@ static int f2fs_write_root_inode(void)
if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
raw_node->i.i_inline = F2FS_EXTRA_ATTR;
- raw_node->i.i_extra_isize =
- cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE);
+ raw_node->i.i_extra_isize = cpu_to_le16(calc_extra_isize());
}
if (c.feature & cpu_to_le32(F2FS_FEATURE_PRJ
[f2fs-dev] [PATCH] f2fs: fix to do sanity with enabled features in image
This patch fixes to do sanity with enabled features in image, if
there are features kernel can not recognize, just fail the mount.
Signed-off-by: Chao Yu
---
fs/f2fs/f2fs.h | 13 +
fs/f2fs/super.c | 9 +
2 files changed, 22 insertions(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index f5ffc09705eb..15b640967e12 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -151,6 +151,19 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_VERITY0x0400 /* reserved */
#define F2FS_FEATURE_SB_CHKSUM 0x0800
+#define F2FS_ALL_FEATURES (F2FS_FEATURE_ENCRYPT | \
+ F2FS_FEATURE_BLKZONED | \
+ F2FS_FEATURE_ATOMIC_WRITE | \
+ F2FS_FEATURE_EXTRA_ATTR | \
+ F2FS_FEATURE_PRJQUOTA | \
+ F2FS_FEATURE_INODE_CHKSUM | \
+ F2FS_FEATURE_FLEXIBLE_INLINE_XATTR |\
+ F2FS_FEATURE_QUOTA_INO |\
+ F2FS_FEATURE_INODE_CRTIME | \
+ F2FS_FEATURE_LOST_FOUND | \
+ F2FS_FEATURE_VERITY | \
+ F2FS_FEATURE_SB_CHKSUM)
+
#define __F2FS_HAS_FEATURE(raw_super, mask)\
((raw_super->feature & cpu_to_le32(mask)) != 0)
#define F2FS_HAS_FEATURE(sbi, mask)__F2FS_HAS_FEATURE(sbi->raw_super, mask)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4f8e9ab48b26..57f2fc6d14ba 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2573,6 +2573,15 @@ static int sanity_check_raw_super(struct f2fs_sb_info
*sbi,
return 1;
}
+ /* check whether kernel supports all features */
+ if (le32_to_cpu(raw_super->feature) & (~F2FS_ALL_FEATURES)) {
+ f2fs_msg(sb, KERN_INFO,
+ "Unsupported feature:%u: supported:%u",
+ le32_to_cpu(raw_super->feature),
+ F2FS_ALL_FEATURES);
+ return 1;
+ }
+
/* check CP/SIT/NAT/SSA/MAIN_AREA area boundary */
if (sanity_check_area_boundary(sbi, bh))
return 1;
--
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 V1 02/14] Consolidate "post read processing" into a new file
On Wednesday, April 24, 2019 2:01:26 PM IST Jaegeuk Kim wrote:
> Hi Chandan,
>
> On 04/24, Chandan Rajendra wrote:
> > The post read processing code is used by both Ext4 and F2FS. Hence to
> > remove duplicity, this commit moves the code into
> > include/linux/post_read_process.h and fs/post_read_process.c.
> >
> > The corresponding decrypt and verity "work" functions have been moved
> > inside fscrypt and fsverity sources. With these in place, the post
> > processing code now has to just invoke enqueue functions provided by
> > fscrypt and fsverity.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/Makefile | 4 +
> > fs/crypto/bio.c | 23 ++--
> > fs/crypto/crypto.c| 17 +--
> > fs/crypto/fscrypt_private.h | 3 +
> > fs/ext4/ext4.h| 2 -
> > fs/ext4/readpage.c| 175 --
> > fs/ext4/super.c | 9 +-
> > fs/f2fs/data.c| 146 -
> > fs/f2fs/super.c | 9 +-
> > fs/post_read_process.c| 136 +++
> > fs/verity/verify.c| 12 ++
> > include/linux/fscrypt.h | 20 +---
> > include/linux/post_read_process.h | 21
> > 13 files changed, 240 insertions(+), 337 deletions(-)
> > create mode 100644 fs/post_read_process.c
> > create mode 100644 include/linux/post_read_process.h
> >
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 9dd2186e74b5..f9abc3f71d3c 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,10 @@ else
> > obj-y += no-block.o
> > endif
> >
> > +ifeq (y, $(firstword $(filter y,$(CONFIG_FS_ENCRYPTION)
> > $(CONFIG_FS_VERITY
> > +obj-y += post_read_process.o
> > +endif
> > +
> > obj-$(CONFIG_PROC_FS) += proc_namespace.o
> >
> > obj-y += notify/
> > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> > index 5759bcd018cd..3e40d65ae6a8 100644
> > --- a/fs/crypto/bio.c
> > +++ b/fs/crypto/bio.c
> > @@ -24,6 +24,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +
> > #include "fscrypt_private.h"
> >
> > static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> > @@ -54,24 +56,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
> > }
> > EXPORT_SYMBOL(fscrypt_decrypt_bio);
> >
> > -static void completion_pages(struct work_struct *work)
> > +void fscrypt_decrypt_work(struct work_struct *work)
> > {
> > - struct fscrypt_ctx *ctx =
> > - container_of(work, struct fscrypt_ctx, r.work);
> > - struct bio *bio = ctx->r.bio;
> > + struct bio_post_read_ctx *ctx =
> > + container_of(work, struct bio_post_read_ctx, work);
> >
> > - __fscrypt_decrypt_bio(bio, true);
> > - fscrypt_release_ctx(ctx);
> > - bio_put(bio);
> > -}
> > + fscrypt_decrypt_bio(ctx->bio);
> >
> > -void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
> > -{
> > - INIT_WORK(&ctx->r.work, completion_pages);
> > - ctx->r.bio = bio;
> > - fscrypt_enqueue_decrypt_work(&ctx->r.work);
> > + bio_post_read_processing(ctx);
> > }
> > -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
> >
> > void fscrypt_pullback_bio_page(struct page **page, bool restore)
> > {
> > @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
> > restore)
> > ctx = (struct fscrypt_ctx *)page_private(bounce_page);
> >
> > /* restore control page */
> > - *page = ctx->w.control_page;
> > + *page = ctx->control_page;
> >
> > if (restore)
> > fscrypt_restore_control_page(bounce_page);
> > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > index 3fc84bf2b1e5..ffa9302a7351 100644
> > --- a/fs/crypto/crypto.c
> > +++ b/fs/crypto/crypto.c
> > @@ -53,6 +53,7 @@ struct kmem_cache *fscrypt_info_cachep;
> >
> > void fscrypt_enqueue_decrypt_work(struct work_struct *work)
> > {
> > + INIT_WORK(work, fscrypt_decrypt_work);
> > queue_work(fscrypt_read_workqueue, work);
> > }
> > EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
> > @@ -70,11 +71,11 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
> > {
> > unsigned long flags;
> >
> > - if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->w.bounce_page) {
> > - mempool_free(ctx->w.bounce_page, fscrypt_bounce_page_pool);
> > - ctx->w.bounce_page = NULL;
> > + if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->bounce_page) {
> > + mempool_free(ctx->bounce_page, fscrypt_bounce_page_pool);
> > + ctx->bounce_page = NULL;
> > }
> > - ctx->w.control_page = NULL;
> > + ctx->control_page = NULL;
> > if (ctx->flags & FS_CTX_REQUIRES_FREE_ENCRYPT_FL) {
> > kmem_cache_free(fscrypt_ctx_cachep, ctx);
> > } else {
> > @@ -194,11 +195,11 @@ int fscrypt_do_page_crypto(const struct inode *inode,
> > fscrypt_direction_t rw,
> > struct page *fscrypt_alloc_bounce_page(struct fscryp
Re: [f2fs-dev] [PATCH V1 02/14] Consolidate "post read processing" into a new file
On Wednesday, April 24, 2019 11:05:44 AM IST Christoph Hellwig wrote: > On Wed, Apr 24, 2019 at 10:07:18AM +0530, Chandan Rajendra wrote: > > +ifeq (y, $(firstword $(filter y,$(CONFIG_FS_ENCRYPTION) > > $(CONFIG_FS_VERITY > > +obj-y += post_read_process.o > > +endif > > Please just add a new config option selected by the users. > Hi Christoph, To clarify, Are you suggesting that a new kconfig option (say CONFIG_FS_READ_CALLBACKS) be provided to the user so that the following could occur, 1. User selects CONFIG_FS_ENCRYPTION and/or CONFIG_FS_VERITY and this causes CONFIG_FS_READ_CALLBACKS to be set to 'y'. 2. User selects CONFIG_FS_READ_CALLBACKS explicitly. > Also I find the file name rather cumbersome. Maybe just > read-callbacks.[co] ? > I will do this. -- chandan ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/2] f2fs: relocate chksum_offset for large_nat_bitmap feature
Hi Chao,
On Mon, Apr 22, 2019 at 6:34 PM Chao Yu wrote:
> + if (__is_set_ckpt_flags(*cp_block, CP_LARGE_NAT_BITMAP_FLAG)) {
> + if (crc_offset != CP_MIN_CHKSUM_OFFSET) {
> + f2fs_put_page(*cp_page, 1);
> + f2fs_msg(sbi->sb, KERN_WARNING,
> + "layout of large_nat_bitmap is deprecated, "
> + "run fsck to repair, chksum_offset: %zu",
> + crc_offset);
> + return -EINVAL;
> + }
> + }
> +
I try not to be a Grammar Nazi and a jerk on every patches, but since
this is a message a user would directly encounter, I'd like to see a
bit less ambiguous message.
How about "using deprecated layout of large_nat_bitmap, please run
fsck v1.13.0 or higher to repair, chksum_offset: %zu"?
The original message seems to suggest that large_nat_bitmap is
deprecated outright.
Also, I'd like to suggest to write down an actual version of
f2fs-tools here as we've seen older versions of fsck doing even more
damage and the users might not have the latest f2fs-tools installed.
Btw, what happens if we use the latest fsck to fix the corrupted image
and use the older kernel to mount it?
Would it even mount?
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 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature
Hi Chao and Jaegeuk,
I'm currently unavailable to recompile the kernel and use the new
layout scheme but I ran the new fsck and attempted to mount it on the
older kernel.
The used fsck-tools is at f50234c248532528b217efff5861b29fe6ec1b36
("f2fs-tools: Fix device zoned model detection").
fsck still behaves weird, I believe.
The new logs are at
http://arter97.com/f2fs/f50234c248532528b217efff5861b29fe6ec1b36
The first run spits out 600MB worth of log
and the second run spits out 60MB worth of log
and the third run fails to fix "other corrupted bugs".
Runs after the third prints out the same log.
When I try to mount the image after third run on the older kernel, it fails:
[ 1710.824598] F2FS-fs (loop1): invalid crc value
[ 1710.855240] F2FS-fs (loop1): SIT is corrupted node# 2174236 vs 2517451
[ 1710.855243] F2FS-fs (loop1): Failed to initialize F2FS segment manager
I'll report back after I compile and run the kernel with the new
layout scheme, but I don't feel good about this after fsck printing
that much logs, and the fact that the 2nd run still spits out a lot of
logs.
Thanks.
On Mon, Apr 22, 2019 at 6:35 PM Chao Yu wrote:
>
> For large_nat_bitmap feature, there is a design flaw:
>
> Previous:
>
> struct f2fs_checkpoint layout:
> +--+ 0x
> | checkpoint_ver |
> | .. |
> | checksum_offset |--+
> | .. | |
> | sit_nat_version_bitmap[] |<-|---+
> | .. | | |
> | checksum_value |<-+ |
> +--+ 0x1000 |
> | | nat_bitmap + sit_bitmap
> | payload blocks | |
> | | |
> +--|<-+
>
> Obviously, if nat_bitmap size + sit_bitmap size is larger than
> MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
> checkpoint checksum's position, once checkpoint() is triggered
> from kernel, nat or sit bitmap will be damaged by checksum field.
>
> In order to fix this, let's relocate checksum_value's position
> to the head of sit_nat_version_bitmap as below, then nat/sit
> bitmap and chksum value update will become safe.
>
> After:
>
> struct f2fs_checkpoint layout:
> +--+ 0x
> | checkpoint_ver |
> | .. |
> | checksum_offset |--+
> | .. | |
> | sit_nat_version_bitmap[] |<-+
> | .. |<-+
> | | |
> +--+ 0x1000 |
> | | nat_bitmap + sit_bitmap
> | payload blocks | |
> | | |
> +--|<-+
>
> Related report and discussion:
>
> https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
>
> In addition, during writing checkpoint, if large_nat_bitmap feature is
> enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.
>
> Reported-by: Park Ju Hyung
> Signed-off-by: Chao Yu
> ---
> fsck/f2fs.h| 6 +-
> fsck/fsck.c| 16
> fsck/fsck.h| 1 +
> fsck/main.c| 2 ++
> fsck/mount.c | 42 +++---
> include/f2fs_fs.h | 9 +++--
> mkfs/f2fs_format.c | 5 -
> 7 files changed, 62 insertions(+), 19 deletions(-)
>
> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
> index 93f01e5..6a6c4a5 100644
> --- a/fsck/f2fs.h
> +++ b/fsck/f2fs.h
> @@ -272,7 +272,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info
> *sbi, int flag)
> if (is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
> offset = (flag == SIT_BITMAP) ?
> le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
> - return &ckpt->sit_nat_version_bitmap + offset;
> + /*
> +* if large_nat_bitmap feature is enabled, leave checksum
> +* protection for all nat/sit bitmaps.
> +*/
> + return &ckpt->sit_nat_version_bitmap + offset +
> sizeof(__le32);
> }
>
> if (le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload) > 0) {
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 7ecdee1..8d7deb5 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -1917,6 +1917,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
> return 0;
> }
>
> +void fsck_chk_checkpoint(struct f2fs_sb_info *sbi)
> +{
> + struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> +
> + if (get_cp(ckpt_flags) & CP_LARGE_NAT_BITMAP_FLAG) {
> + if (get_cp(checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
> + ASSERT_MSG("Deprecated layout of large_nat_bitmap, "
> + "chksum_offset:%u", get_cp(checksum_offset));
> + }
> + }
> +}
> +
> void fsck_init(struct f2
Re: [f2fs-dev] [PATCH V1 02/14] Consolidate "post read processing" into a new file
On 04/24, Chandan Rajendra wrote:
> On Wednesday, April 24, 2019 2:01:26 PM IST Jaegeuk Kim wrote:
> > Hi Chandan,
> >
> > On 04/24, Chandan Rajendra wrote:
> > > The post read processing code is used by both Ext4 and F2FS. Hence to
> > > remove duplicity, this commit moves the code into
> > > include/linux/post_read_process.h and fs/post_read_process.c.
> > >
> > > The corresponding decrypt and verity "work" functions have been moved
> > > inside fscrypt and fsverity sources. With these in place, the post
> > > processing code now has to just invoke enqueue functions provided by
> > > fscrypt and fsverity.
> > >
> > > Signed-off-by: Chandan Rajendra
> > > ---
> > > fs/Makefile | 4 +
> > > fs/crypto/bio.c | 23 ++--
> > > fs/crypto/crypto.c| 17 +--
> > > fs/crypto/fscrypt_private.h | 3 +
> > > fs/ext4/ext4.h| 2 -
> > > fs/ext4/readpage.c| 175 --
> > > fs/ext4/super.c | 9 +-
> > > fs/f2fs/data.c| 146 -
> > > fs/f2fs/super.c | 9 +-
> > > fs/post_read_process.c| 136 +++
> > > fs/verity/verify.c| 12 ++
> > > include/linux/fscrypt.h | 20 +---
> > > include/linux/post_read_process.h | 21
> > > 13 files changed, 240 insertions(+), 337 deletions(-)
> > > create mode 100644 fs/post_read_process.c
> > > create mode 100644 include/linux/post_read_process.h
> > >
> > > diff --git a/fs/Makefile b/fs/Makefile
> > > index 9dd2186e74b5..f9abc3f71d3c 100644
> > > --- a/fs/Makefile
> > > +++ b/fs/Makefile
> > > @@ -21,6 +21,10 @@ else
> > > obj-y += no-block.o
> > > endif
> > >
> > > +ifeq (y, $(firstword $(filter y,$(CONFIG_FS_ENCRYPTION)
> > > $(CONFIG_FS_VERITY
> > > +obj-y += post_read_process.o
> > > +endif
> > > +
> > > obj-$(CONFIG_PROC_FS) += proc_namespace.o
> > >
> > > obj-y+= notify/
> > > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> > > index 5759bcd018cd..3e40d65ae6a8 100644
> > > --- a/fs/crypto/bio.c
> > > +++ b/fs/crypto/bio.c
> > > @@ -24,6 +24,8 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > +
> > > #include "fscrypt_private.h"
> > >
> > > static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> > > @@ -54,24 +56,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
> > > }
> > > EXPORT_SYMBOL(fscrypt_decrypt_bio);
> > >
> > > -static void completion_pages(struct work_struct *work)
> > > +void fscrypt_decrypt_work(struct work_struct *work)
> > > {
> > > - struct fscrypt_ctx *ctx =
> > > - container_of(work, struct fscrypt_ctx, r.work);
> > > - struct bio *bio = ctx->r.bio;
> > > + struct bio_post_read_ctx *ctx =
> > > + container_of(work, struct bio_post_read_ctx, work);
> > >
> > > - __fscrypt_decrypt_bio(bio, true);
> > > - fscrypt_release_ctx(ctx);
> > > - bio_put(bio);
> > > -}
> > > + fscrypt_decrypt_bio(ctx->bio);
> > >
> > > -void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio
> > > *bio)
> > > -{
> > > - INIT_WORK(&ctx->r.work, completion_pages);
> > > - ctx->r.bio = bio;
> > > - fscrypt_enqueue_decrypt_work(&ctx->r.work);
> > > + bio_post_read_processing(ctx);
> > > }
> > > -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
> > >
> > > void fscrypt_pullback_bio_page(struct page **page, bool restore)
> > > {
> > > @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
> > > restore)
> > > ctx = (struct fscrypt_ctx *)page_private(bounce_page);
> > >
> > > /* restore control page */
> > > - *page = ctx->w.control_page;
> > > + *page = ctx->control_page;
> > >
> > > if (restore)
> > > fscrypt_restore_control_page(bounce_page);
> > > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > > index 3fc84bf2b1e5..ffa9302a7351 100644
> > > --- a/fs/crypto/crypto.c
> > > +++ b/fs/crypto/crypto.c
> > > @@ -53,6 +53,7 @@ struct kmem_cache *fscrypt_info_cachep;
> > >
> > > void fscrypt_enqueue_decrypt_work(struct work_struct *work)
> > > {
> > > + INIT_WORK(work, fscrypt_decrypt_work);
> > > queue_work(fscrypt_read_workqueue, work);
> > > }
> > > EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
> > > @@ -70,11 +71,11 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
> > > {
> > > unsigned long flags;
> > >
> > > - if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->w.bounce_page) {
> > > - mempool_free(ctx->w.bounce_page, fscrypt_bounce_page_pool);
> > > - ctx->w.bounce_page = NULL;
> > > + if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->bounce_page) {
> > > + mempool_free(ctx->bounce_page, fscrypt_bounce_page_pool);
> > > + ctx->bounce_page = NULL;
> > > }
> > > - ctx->w.control_page = NULL;
> > > + ctx->control_page = NULL;
> > > if (ctx->flags & FS_CTX_REQUIRES_FREE_ENCRYPT_FL) {
> > >
Re: [f2fs-dev] [PATCH v2 4/5] f2fs: add tracepoint for f2fs_filemap_fault()
On 2019-4-24 17:14, Jaegeuk Kim wrote:
> On 04/15, Chao Yu wrote:
>> This patch adds tracepoint for f2fs_filemap_fault().
>>
>> Signed-off-by: Chao Yu
>> ---
>> v2:
>> - fix wrong type of @ret parameter
>> fs/f2fs/file.c | 2 ++
>> include/trace/events/f2fs.h | 26 ++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 30d49467578e..578486e03427 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -39,6 +39,8 @@ static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
>> ret = filemap_fault(vmf);
>> up_read(&F2FS_I(inode)->i_mmap_sem);
>>
>> +trace_f2fs_filemap_fault(inode, vmf->pgoff, ret);
>
> In order to avoid wrong casting warning, how about this?
Confirmed, I used wrong type in v2, thanks for fixing this.
Thanks,
>
> ---
> include/trace/events/f2fs.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index abe763cc1d0b..c29dea7ac0fe 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -1286,7 +1286,7 @@ DEFINE_EVENT(f2fs__page, f2fs_commit_inmem_page,
>
> TRACE_EVENT(f2fs_filemap_fault,
>
> - TP_PROTO(struct inode *inode, pgoff_t index, enum vm_fault_reason ret),
> + TP_PROTO(struct inode *inode, pgoff_t index, vm_fault_t ret),
>
> TP_ARGS(inode, index, ret),
>
> @@ -1294,7 +1294,7 @@ TRACE_EVENT(f2fs_filemap_fault,
> __field(dev_t, dev)
> __field(ino_t, ino)
> __field(pgoff_t, index)
> - __field(enum vm_fault_reason, ret)
> + __field(vm_fault_t, ret)
> ),
>
> TP_fast_assign(
> @@ -1304,10 +1304,10 @@ TRACE_EVENT(f2fs_filemap_fault,
> __entry->ret= ret;
> ),
>
> - TP_printk("dev = (%d,%d), ino = %lu, index = %lu, ret = %u",
> + TP_printk("dev = (%d,%d), ino = %lu, index = %lu, ret = %lx",
> show_dev_ino(__entry),
> (unsigned long)__entry->index,
> - ret)
> + (unsigned long)__entry->ret)
> );
>
> TRACE_EVENT(f2fs_writepages,
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V1 02/14] Consolidate "post read processing" into a new file
On Wed, Apr 24, 2019 at 03:34:17PM +0530, Chandan Rajendra wrote: > To clarify, Are you suggesting that a new kconfig option (say > CONFIG_FS_READ_CALLBACKS) be provided to the user so that the following could > occur, > > 1. User selects CONFIG_FS_ENCRYPTION and/or CONFIG_FS_VERITY and this causes > CONFIG_FS_READ_CALLBACKS to be set to 'y'. > 2. User selects CONFIG_FS_READ_CALLBACKS explicitly. If you don't add a user description to a Kconfig entry it won't be visible to users, but only selectable by other options. That is what I suggest. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V1 02/14] Consolidate "post read processing" into a new file
On Wednesday, April 24, 2019 5:36:46 PM IST Jaegeuk Kim wrote:
> On 04/24, Chandan Rajendra wrote:
> > On Wednesday, April 24, 2019 2:01:26 PM IST Jaegeuk Kim wrote:
> > > Hi Chandan,
> > >
> > > On 04/24, Chandan Rajendra wrote:
> > > > The post read processing code is used by both Ext4 and F2FS. Hence to
> > > > remove duplicity, this commit moves the code into
> > > > include/linux/post_read_process.h and fs/post_read_process.c.
> > > >
> > > > The corresponding decrypt and verity "work" functions have been moved
> > > > inside fscrypt and fsverity sources. With these in place, the post
> > > > processing code now has to just invoke enqueue functions provided by
> > > > fscrypt and fsverity.
> > > >
> > > > Signed-off-by: Chandan Rajendra
> > > > ---
> > > > fs/Makefile | 4 +
> > > > fs/crypto/bio.c | 23 ++--
> > > > fs/crypto/crypto.c| 17 +--
> > > > fs/crypto/fscrypt_private.h | 3 +
> > > > fs/ext4/ext4.h| 2 -
> > > > fs/ext4/readpage.c| 175 --
> > > > fs/ext4/super.c | 9 +-
> > > > fs/f2fs/data.c| 146 -
> > > > fs/f2fs/super.c | 9 +-
> > > > fs/post_read_process.c| 136 +++
> > > > fs/verity/verify.c| 12 ++
> > > > include/linux/fscrypt.h | 20 +---
> > > > include/linux/post_read_process.h | 21
> > > > 13 files changed, 240 insertions(+), 337 deletions(-)
> > > > create mode 100644 fs/post_read_process.c
> > > > create mode 100644 include/linux/post_read_process.h
> > > >
> > > > diff --git a/fs/Makefile b/fs/Makefile
> > > > index 9dd2186e74b5..f9abc3f71d3c 100644
> > > > --- a/fs/Makefile
> > > > +++ b/fs/Makefile
> > > > @@ -21,6 +21,10 @@ else
> > > > obj-y += no-block.o
> > > > endif
> > > >
> > > > +ifeq (y, $(firstword $(filter y,$(CONFIG_FS_ENCRYPTION)
> > > > $(CONFIG_FS_VERITY
> > > > +obj-y += post_read_process.o
> > > > +endif
> > > > +
> > > > obj-$(CONFIG_PROC_FS) += proc_namespace.o
> > > >
> > > > obj-y += notify/
> > > > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> > > > index 5759bcd018cd..3e40d65ae6a8 100644
> > > > --- a/fs/crypto/bio.c
> > > > +++ b/fs/crypto/bio.c
> > > > @@ -24,6 +24,8 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > +
> > > > #include "fscrypt_private.h"
> > > >
> > > > static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> > > > @@ -54,24 +56,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
> > > > }
> > > > EXPORT_SYMBOL(fscrypt_decrypt_bio);
> > > >
> > > > -static void completion_pages(struct work_struct *work)
> > > > +void fscrypt_decrypt_work(struct work_struct *work)
> > > > {
> > > > - struct fscrypt_ctx *ctx =
> > > > - container_of(work, struct fscrypt_ctx, r.work);
> > > > - struct bio *bio = ctx->r.bio;
> > > > + struct bio_post_read_ctx *ctx =
> > > > + container_of(work, struct bio_post_read_ctx, work);
> > > >
> > > > - __fscrypt_decrypt_bio(bio, true);
> > > > - fscrypt_release_ctx(ctx);
> > > > - bio_put(bio);
> > > > -}
> > > > + fscrypt_decrypt_bio(ctx->bio);
> > > >
> > > > -void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio
> > > > *bio)
> > > > -{
> > > > - INIT_WORK(&ctx->r.work, completion_pages);
> > > > - ctx->r.bio = bio;
> > > > - fscrypt_enqueue_decrypt_work(&ctx->r.work);
> > > > + bio_post_read_processing(ctx);
> > > > }
> > > > -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
> > > >
> > > > void fscrypt_pullback_bio_page(struct page **page, bool restore)
> > > > {
> > > > @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page,
> > > > bool restore)
> > > > ctx = (struct fscrypt_ctx *)page_private(bounce_page);
> > > >
> > > > /* restore control page */
> > > > - *page = ctx->w.control_page;
> > > > + *page = ctx->control_page;
> > > >
> > > > if (restore)
> > > > fscrypt_restore_control_page(bounce_page);
> > > > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > > > index 3fc84bf2b1e5..ffa9302a7351 100644
> > > > --- a/fs/crypto/crypto.c
> > > > +++ b/fs/crypto/crypto.c
> > > > @@ -53,6 +53,7 @@ struct kmem_cache *fscrypt_info_cachep;
> > > >
> > > > void fscrypt_enqueue_decrypt_work(struct work_struct *work)
> > > > {
> > > > + INIT_WORK(work, fscrypt_decrypt_work);
> > > > queue_work(fscrypt_read_workqueue, work);
> > > > }
> > > > EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
> > > > @@ -70,11 +71,11 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
> > > > {
> > > > unsigned long flags;
> > > >
> > > > - if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL &&
> > > > ctx->w.bounce_page) {
> > >
Re: [f2fs-dev] [PATCH V1 02/14] Consolidate "post read processing" into a new file
On Wednesday, April 24, 2019 7:54:23 PM IST Christoph Hellwig wrote: > On Wed, Apr 24, 2019 at 03:34:17PM +0530, Chandan Rajendra wrote: > > To clarify, Are you suggesting that a new kconfig option (say > > CONFIG_FS_READ_CALLBACKS) be provided to the user so that the following > > could > > occur, > > > > 1. User selects CONFIG_FS_ENCRYPTION and/or CONFIG_FS_VERITY and this causes > > CONFIG_FS_READ_CALLBACKS to be set to 'y'. > > 2. User selects CONFIG_FS_READ_CALLBACKS explicitly. > > If you don't add a user description to a Kconfig entry it won't be > visible to users, but only selectable by other options. That is what > I suggest. > > Thanks for the clarification. I will make the necessary changes. -- chandan ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs-tools:fix memory leak in write dquot
On 2019-4-24 14:47, Xiaojun Wang wrote: > this patch free ddquot in qtree_write_dquot to avoid memory leak > > Signed-off-by: Xiaojun Wang 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
Re: [f2fs-dev] [PATCH v2 2/2] f2fs: introduce DATA_GENERIC_ENHANCE
On 2019-4-24 17:36, Jaegeuk Kim wrote:
> On 04/15, Chao Yu wrote:
>> Previously, f2fs_is_valid_blkaddr(, blkaddr, DATA_GENERIC) will check
>> whether @blkaddr locates in main area or not.
>>
>> That check is weak, since the block address in range of main area can
>> point to the address which is not valid in segment info table, and we
>> can not detect such condition, we may suffer worse corruption as system
>> continues running.
>>
>> So this patch introduce DATA_GENERIC_ENHANCE to enhance the sanity check
>> which trigger SIT bitmap check rather than only range check.
>>
>> This patch did below changes as wel:
>> - set SBI_NEED_FSCK in f2fs_is_valid_blkaddr().
>> - get rid of is_valid_data_blkaddr() to avoid panic if blkaddr is invalid.
>> - introduce verify_fio_blkaddr() to wrap fio {new,old}_blkaddr validation
>> check.
>> - spread blkaddr check in:
>> * f2fs_get_node_info()
>> * __read_out_blkaddrs()
>> * f2fs_submit_page_read()
>> * ra_data_block()
>> * do_recover_data()
>>
>> This patch can fix bug reported from bugzilla below:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=203215
>> https://bugzilla.kernel.org/show_bug.cgi?id=203223
>> https://bugzilla.kernel.org/show_bug.cgi?id=203231
>> https://bugzilla.kernel.org/show_bug.cgi?id=203235
>> https://bugzilla.kernel.org/show_bug.cgi?id=203241
>
> Hi Chao,
>
> This introduces failures on xfstests/generic/446, and I'm testing the below
> patch on top of this. Could you check this patch, so that I could combine
> both of them?
>
> From 8c1808c1743ad75d1ad8d1dc5a53910edaf7afd7 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim
> Date: Wed, 24 Apr 2019 00:21:07 +0100
> Subject: [PATCH] f2fs: consider data race on read and truncation on
> DATA_GENERIC_ENHANCE
>
> DATA_GENERIC_ENHANCE enhanced to validate block addresses on read/write paths.
> But, xfstest/generic/446 compalins some generated kernel messages saying
> invalid
> bitmap was detected when reading a block. The reaons is, when we get the
> block addresses from extent_cache, there is no lock to synchronize it from
> truncating the blocks in parallel.
>
> This patch tries to return EFAULT without warning and setting SBI_NEED_FSCK
> in this case.
>
> Fixes: ("f2fs: introduce DATA_GENERIC_ENHANCE")
> Signed-off-by: Jaegeuk Kim
> ---
> fs/f2fs/checkpoint.c | 35 ++-
> fs/f2fs/data.c | 25 ++---
> fs/f2fs/f2fs.h | 6 ++
> fs/f2fs/gc.c | 9 ++---
> 4 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index e37fbbf843a5..805a33088e82 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -130,26 +130,28 @@ struct page *f2fs_get_tmp_page(struct f2fs_sb_info
> *sbi, pgoff_t index)
> return __get_meta_page(sbi, index, false);
> }
>
> -static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr)
> +static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr,
> + int type)
> {
> struct seg_entry *se;
> unsigned int segno, offset;
> bool exist;
>
> + if (type != DATA_GENERIC_ENHANCE && type != DATA_GENERIC_ENHANCE_READ)
> + return true;
> +
> segno = GET_SEGNO(sbi, blkaddr);
> offset = GET_BLKOFF_FROM_SEG0(sbi, blkaddr);
> se = get_seg_entry(sbi, segno);
>
> exist = f2fs_test_bit(offset, se->cur_valid_map);
> -
> - if (!exist) {
> + if (!exist && type == DATA_GENERIC_ENHANCE) {
> f2fs_msg(sbi->sb, KERN_ERR, "Inconsistent error "
> "blkaddr:%u, sit bitmap:%d", blkaddr, exist);
> set_sbi_flag(sbi, SBI_NEED_FSCK);
> WARN_ON(1);
> - return false;
> }
> - return true;
> + return exist;
> }
>
> bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> @@ -173,23 +175,22 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> return false;
> break;
> case META_POR:
> + if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> + blkaddr < MAIN_BLKADDR(sbi)))
> + return false;
> + break;
> case DATA_GENERIC:
> case DATA_GENERIC_ENHANCE:
> + case DATA_GENERIC_ENHANCE_READ:
> if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> - blkaddr < MAIN_BLKADDR(sbi))) {
> - if (type == DATA_GENERIC ||
> - type == DATA_GENERIC_ENHANCE) {
> - f2fs_msg(sbi->sb, KERN_WARNING,
> - "access invalid blkaddr:%u", blkaddr);
> - set_sbi_flag(sbi, SBI_NEED_FSCK);
> - WARN_ON(1);
> - }
> + blkaddr < MAIN_BLKADDR(sbi))) {
> + f2fs_msg(sbi->sb, KERN_WARNING,
> +
[f2fs-dev] [PATCH] f2fs-tools: improve filename printing
From: Eric Biggers
- Make buffers for pretty-printed filenames 341 bytes long, long enough
for 255 (NAME_MAX) base64-encoded bytes. Then print encrypted
filenames in full, base64-encoded. This will be useful for tests I'm
writing which verify the correct ciphertext is stored on-disk.
- Rename convert_encrypted_name() to pretty_print_filename(), to make it
clear that it handles unencrypted names too. Also make the output
'char' rather than 'unsigned char', as it's for printing; and remove
the unnecessary return value.
Signed-off-by: Eric Biggers
---
fsck/dump.c | 12 --
fsck/fsck.c | 58 ---
fsck/fsck.h | 3 ++-
fsck/mount.c | 10
include/f2fs_fs.h | 4
5 files changed, 40 insertions(+), 47 deletions(-)
diff --git a/fsck/dump.c b/fsck/dump.c
index 07dc2fc..390361d 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -627,8 +627,8 @@ static void dump_dirent(u32 blk_addr, int is_inline, int
enc_name)
while (i < d.max) {
struct f2fs_dir_entry *de;
- unsigned char en[F2FS_NAME_LEN + 1];
- u16 en_len, name_len;
+ char en[F2FS_PRINT_NAMELEN];
+ u16 name_len;
int enc;
if (!test_bit_le(i, d.bitmap)) {
@@ -654,18 +654,16 @@ static void dump_dirent(u32 blk_addr, int is_inline, int
enc_name)
}
}
- en_len = convert_encrypted_name(d.filename[i],
- le16_to_cpu(de->name_len), en, enc);
- en[en_len] = '\0';
+ pretty_print_filename(d.filename[i], name_len, en, enc);
DBG(1, "bitmap pos[0x%x] name[%s] len[0x%x] hash[0x%x]
ino[0x%x] type[0x%x]\n",
i, en,
- le16_to_cpu(de->name_len),
+ name_len,
le32_to_cpu(de->hash_code),
le32_to_cpu(de->ino),
de->file_type);
- i += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
+ i += GET_DENTRY_SLOTS(name_len);
}
free(blk);
diff --git a/fsck/fsck.c b/fsck/fsck.c
index a1791d4..e7b3fba 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -660,7 +660,7 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
u64 i_size = le64_to_cpu(node_blk->i.i_size);
u64 i_blocks = le64_to_cpu(node_blk->i.i_blocks);
int ofs;
- unsigned char *en;
+ char *en;
u32 namelen;
unsigned int idx = 0;
unsigned short i_gc_failures;
@@ -907,7 +907,7 @@ check:
}
}
skip_blkcnt_fix:
- en = malloc(F2FS_NAME_LEN + 1);
+ en = malloc(F2FS_PRINT_NAMELEN);
ASSERT(en);
namelen = le32_to_cpu(node_blk->i.i_namelen);
@@ -926,9 +926,8 @@ skip_blkcnt_fix:
} else
namelen = F2FS_NAME_LEN;
}
- namelen = convert_encrypted_name(node_blk->i.i_name, namelen,
- en, file_enc_name(&node_blk->i));
- en[namelen] = '\0';
+ pretty_print_filename(node_blk->i.i_name, namelen, en,
+ file_enc_name(&node_blk->i));
if (ftype == F2FS_FT_ORPHAN)
DBG(1, "Orphan Inode: 0x%x [%s] i_blocks: %u\n\n",
le32_to_cpu(node_blk->footer.ino),
@@ -1168,45 +1167,40 @@ static const char *lookup_table =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
/**
- * digest_encode() -
+ * base64_encode() -
*
- * Encodes the input digest using characters from the set [a-zA-Z0-9_+].
+ * Encodes the input string using characters from the set [A-Za-z0-9+,].
* The encoded string is roughly 4/3 times the size of the input string.
*/
-static int digest_encode(const char *src, int len, char *dst)
+static int base64_encode(const u8 *src, int len, char *dst)
{
- int i = 0, bits = 0, ac = 0;
+ int i, bits = 0, ac = 0;
char *cp = dst;
- while (i < len && i < 24) {
- ac += (((unsigned char) src[i]) << bits);
+ for (i = 0; i < len; i++) {
+ ac += src[i] << bits;
bits += 8;
do {
*cp++ = lookup_table[ac & 0x3f];
ac >>= 6;
bits -= 6;
} while (bits >= 6);
- i++;
}
if (bits)
*cp++ = lookup_table[ac & 0x3f];
- *cp = 0;
return cp - dst;
}
-int convert_encrypted_name(unsigned char *name, u32 len,
- unsigned char *new, int enc_name)
+void pretty_print_filename(const u8 *raw_name, u32 len,
+ char out[F2FS_PRINT_NAMELEN], int enc_name)
{
- if (!enc_name) {
- if (len > F2FS_NAME
[f2fs-dev] [PATCH] f2fs-tools: improve xattr value printing
From: Eric Biggers
- Print the values of xattrs that have an unknown ->e_name_index, rather
than ignoring them.
- Replace char with u8. Otherwise xattr values containing bytes >= 0x80
are printed incorrectly on platforms where char is signed.
- Only parse the encryption xattr if it has a known format number and
size. Otherwise print it as hex.
- Remove incorrect le16_to_cpu() on ->e_name_len which has type u8.
- Consolidate the code that prints the xattr value as hex.
- Constify pointers to the xattr entry.
Signed-off-by: Eric Biggers
---
fsck/mount.c | 56 ++--
fsck/xattr.h | 1 +
2 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/fsck/mount.c b/fsck/mount.c
index 843742e..c69226d 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -55,11 +55,11 @@ void update_free_segments(struct f2fs_sb_info *sbi)
}
#if defined(HAVE_LINUX_POSIX_ACL_H) || defined(HAVE_SYS_ACL_H)
-void print_acl(char *value, int size)
+static void print_acl(const u8 *value, int size)
{
- struct f2fs_acl_header *hdr = (struct f2fs_acl_header *)value;
- struct f2fs_acl_entry *entry = (struct f2fs_acl_entry *)(hdr + 1);
- const char *end = value + size;
+ const struct f2fs_acl_header *hdr = (struct f2fs_acl_header *)value;
+ const struct f2fs_acl_entry *entry = (struct f2fs_acl_entry *)(hdr + 1);
+ const u8 *end = value + size;
int i, count;
if (hdr->a_version != cpu_to_le32(F2FS_ACL_VERSION)) {
@@ -75,7 +75,7 @@ void print_acl(char *value, int size)
}
for (i = 0; i < count; i++) {
- if ((char *)entry > end) {
+ if ((u8 *)entry > end) {
MSG(0, "Invalid ACL entries count %d\n", count);
return;
}
@@ -114,42 +114,33 @@ void print_acl(char *value, int size)
}
}
}
-#else
-#define print_acl(value, size) do {\
- int i; \
- for (i = 0; i < size; i++) \
- MSG(0, "%02X", value[i]); \
- MSG(0, "\n"); \
-} while (0)
-#endif
+#endif /* HAVE_LINUX_POSIX_ACL_H || HAVE_SYS_ACL_H */
-void print_xattr_entry(struct f2fs_xattr_entry *ent)
+static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
{
- char *value = (char *)(ent->e_name + le16_to_cpu(ent->e_name_len));
- struct fscrypt_context *ctx;
+ const u8 *value = (const u8 *)&ent->e_name[ent->e_name_len];
+ const int size = le16_to_cpu(ent->e_value_size);
+ const struct fscrypt_context *ctx;
int i;
MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
- for (i = 0; i < le16_to_cpu(ent->e_name_len); i++)
+ for (i = 0; i < ent->e_name_len; i++)
MSG(0, "%c", ent->e_name[i]);
MSG(0, " e_name_len:%d e_value_size:%d e_value:\n",
- ent->e_name_len, le16_to_cpu(ent->e_value_size));
+ ent->e_name_len, size);
switch (ent->e_name_index) {
+#if defined(HAVE_LINUX_POSIX_ACL_H) || defined(HAVE_SYS_ACL_H)
case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
case F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT:
- print_acl(value, le16_to_cpu(ent->e_value_size));
- break;
- case F2FS_XATTR_INDEX_USER:
- case F2FS_XATTR_INDEX_SECURITY:
- case F2FS_XATTR_INDEX_TRUSTED:
- case F2FS_XATTR_INDEX_LUSTRE:
- for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
- MSG(0, "%02X", value[i]);
- MSG(0, "\n");
- break;
+ print_acl(value, size);
+ return;
+#endif
case F2FS_XATTR_INDEX_ENCRYPTION:
- ctx = (struct fscrypt_context *)value;
+ ctx = (const struct fscrypt_context *)value;
+ if (size != sizeof(*ctx) ||
+ ctx->format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+ break;
MSG(0, "format: %d\n", ctx->format);
MSG(0, "contents_encryption_mode: 0x%x\n",
ctx->contents_encryption_mode);
MSG(0, "filenames_encryption_mode: 0x%x\n",
ctx->filenames_encryption_mode);
@@ -161,10 +152,11 @@ void print_xattr_entry(struct f2fs_xattr_entry *ent)
for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
MSG(0, "%02X", ctx->nonce[i]);
MSG(0, "\n");
- break;
- default:
- break;
+ return;
}
+ for (i = 0; i < size; i++)
+ MSG(0, "%02X", value[i]);
+ MSG(0, "\n");
}
void print_inode_info(struct f2fs_sb_info *sbi,
diff --git a/fsck/xattr.h b/fsck/xattr.h
index dc80faf..579ab6c 100644
--- a/fsck/xattr.h
+++ b/fsck/xattr.h
@@ -34,6 +34,7 @@ struct f2fs_xattr_entry {
char e_name[0]; /* attribute name */
};
Re: [f2fs-dev] [PATCH 2/2] f2fs: relocate chksum_offset for large_nat_bitmap feature
Hi Ju Hyung,
On 2019/4/24 19:43, Ju Hyung Park wrote:
> Hi Chao,
>
> On Mon, Apr 22, 2019 at 6:34 PM Chao Yu wrote:
>> + if (__is_set_ckpt_flags(*cp_block, CP_LARGE_NAT_BITMAP_FLAG)) {
>> + if (crc_offset != CP_MIN_CHKSUM_OFFSET) {
>> + f2fs_put_page(*cp_page, 1);
>> + f2fs_msg(sbi->sb, KERN_WARNING,
>> + "layout of large_nat_bitmap is deprecated, "
>> + "run fsck to repair, chksum_offset: %zu",
>> + crc_offset);
>> + return -EINVAL;
>> + }
>> + }
>> +
>
> I try not to be a Grammar Nazi and a jerk on every patches, but since
> this is a message a user would directly encounter, I'd like to see a
> bit less ambiguous message.
Please feel free to give us your opinion or suggestion. :)
>
> How about "using deprecated layout of large_nat_bitmap, please run
> fsck v1.13.0 or higher to repair, chksum_offset: %zu"?
> The original message seems to suggest that large_nat_bitmap is
> deprecated outright.
>
> Also, I'd like to suggest to write down an actual version of
> f2fs-tools here as we've seen older versions of fsck doing even more
> damage and the users might not have the latest f2fs-tools installed.
Agreed, user should be told which version of fsck can repair that problem, will
update the message in next version.
>
> Btw, what happens if we use the latest fsck to fix the corrupted image
> and use the older kernel to mount it?
> Would it even mount?
No, since fixed image is using a new layout, how about giving the detailed
information about which version of kernel user should update to, once we detect
such issue and trigger the repairing?
Thanks,
>
> Thanks.
> .
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH UNTESTED] f2fs-tools: write fill chunk in sparse file for zeroed block
As zhaowuyun reported:
we met one problem of f2fs, and found one issue of make_f2fs, so I write
this email to search for your help to confirm this issue.
The issue was found on one of Android projects. We use f2fs as the filesystem
of userdata, and make sparse userdata.img using following command, which
invoked in script mkf2fsuserimg.sh
make_f2fs -S $SIZE -f -O encrypt -O quota -O verity $MKFS_OPTS $OUTPUT_FILE
use fastboot to flash this userdata.img to device, and it encountered f2fs
problem and leading to the mount fail of data partition.
we can make this issue 100% persent reproduced by making the data partition
dirty before flashing userdata.img.
suspect that issue is caused by the dirty data in the data partition.
so we checked that source code of make_f2fs in f2fs-tool, found that when
making f2fs, it use dev_fill to do some process:
...
we change code to the following, and the issue is gone.
if (c.sparse_mode)
return dev_write(buf, offset, len);
Chao Yu:
>
> After checking the codes, IIUC, I guess the problem here is, unlike
> img2simg, mkfs.f2fs won't record zeroed block in sparse image, so
> during transforming to normal image, some critical region like
> NAT/SIT/CP.payload area weren't be zeroed correctly, later kernel may
> load obsoleting data from those region.
>
> Also, The way you provide will obviously increase the size of sparse
> file, since with it we need to write all zeroed blocks of
> NAT/SIT/CP.payload to sparse file, it's not needed.
>
> Not sure, maybe we should use sparse_file_add_fill() to record zeroed
> blocks, so that this will make formatted image more like img2simged one.
Jaegeuk:
> We have to call sparse_file_add_fill() for dev_fill().
This patch fixes to support writing fill chunk sparse file for those
zeroed blocks.
Reported-by: zhaowuyun
Signed-off-by: Chao Yu
---
lib/libf2fs_io.c | 78 +++-
1 file changed, 64 insertions(+), 14 deletions(-)
diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
index f848510..d284752 100644
--- a/lib/libf2fs_io.c
+++ b/lib/libf2fs_io.c
@@ -36,6 +36,7 @@ struct f2fs_configuration c;
struct sparse_file *f2fs_sparse_file;
static char **blocks;
u_int64_t blocks_count;
+static char *zeroed_block;
#endif
static int __get_device_fd(__u64 *offset)
@@ -103,6 +104,8 @@ static int sparse_write_blk(__u64 block, int count, const
void *buf)
for (i = 0; i < count; ++i) {
cur_block = block + i;
+ if (blocks[cur_block] == zeroed_block)
+ blocks[cur_block] = NULL;
if (!blocks[cur_block]) {
blocks[cur_block] = calloc(1, F2FS_BLKSIZE);
if (!blocks[cur_block])
@@ -114,6 +117,22 @@ static int sparse_write_blk(__u64 block, int count, const
void *buf)
return 0;
}
+static int sparse_write_zeroed_blk(__u64 block, int count)
+{
+ int i;
+ __u64 cur_block;
+
+ for (i = 0; i < count; ++i) {
+ cur_block = block + i;
+ if (blocks[cur_block] == zeroed_block)
+ continue;
+ else if (blocks[cur_block])
+ return -EEXIST;
+ blocks[cur_block] = zeroed_block;
+ }
+ return 0;
+}
+
#ifdef SPARSE_CALLBACK_USES_SIZE_T
static int sparse_import_segment(void *UNUSED(priv), const void *data,
size_t len, unsigned int block, unsigned int nr_blocks)
@@ -129,11 +148,17 @@ static int sparse_import_segment(void *UNUSED(priv),
const void *data, int len,
return sparse_write_blk(block, nr_blocks, data);
}
-static int sparse_merge_blocks(uint64_t start, uint64_t num)
+static int sparse_merge_blocks(uint64_t start, uint64_t num, int zero)
{
char *buf;
uint64_t i;
+ if (zero) {
+ blocks[start] = NULL;
+ return sparse_file_add_fill(f2fs_sparse_file, 0x0,
+ F2FS_BLKSIZE * num, start);
+ }
+
buf = calloc(num, F2FS_BLKSIZE);
if (!buf) {
fprintf(stderr, "failed to alloc %llu\n",
@@ -156,6 +181,7 @@ static int sparse_merge_blocks(uint64_t start, uint64_t num)
#else
static int sparse_read_blk(__u64 block, int count, void *buf) { return 0; }
static int sparse_write_blk(__u64 block, int count, const void *buf) { return
0; }
+static int sparse_write_zeroed_blk(__u64 block, int count) { return 0; }
#endif
int dev_read(void *buf, __u64 offset, size_t len)
@@ -235,7 +261,8 @@ int dev_fill(void *buf, __u64 offset, size_t len)
int fd;
if (c.sparse_mode)
- return 0;
+ return sparse_write_zeroed_blk(offset / F2FS_BLKSIZE,
+ len / F2FS_BLKSIZE);
fd = __get_device_fd(&offset);
if (fd < 0)
@@ -307,6 +334,12 @@ int f2fs_init_sparse_file(void)
return -1;
}
+ zeroed_block = calloc(1,
