Re: [f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures

2018-11-26 Thread Chao Yu
On 2018/11/27 11:52, Jaegeuk Kim wrote:
> On 11/27, Chao Yu wrote:
>> On 2018/11/27 8:04, Jaegeuk Kim wrote:
>>> On 11/26, Chao Yu wrote:
 From: Chao Yu 

 As Michael reported:

 after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
 took quite a while and output ten-thousands instances of the following
 line:

> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures 
> from 0x1 to 0x00

 In old kernel, we initialized i_gc_failures as 0x01, let's skip
 resetting such unchanged initialized value to avoid unnecessary
 repairing.

 Reported-by: Michael Laß 
 Signed-off-by: Chao Yu 
 ---
  fsck/fsck.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/fsck/fsck.c b/fsck/fsck.c
 index 970d150..db0e72f 100644
 --- a/fsck/fsck.c
 +++ b/fsck/fsck.c
 @@ -941,7 +941,9 @@ skip_blkcnt_fix:
}
  
i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures);
 -  if (ftype == F2FS_FT_REG_FILE && i_gc_failures) {
 +
 +  /* old kernel initialized i_gc_failures as 0x01, skip repairing */
 +  if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) {
>>>
>>> This will break the current implementation.
>>
>> Yeah, but I doubt that after repairing i_gc_failures, old kernel can still
>> continue creating inodes in where .i_gc_failures equals to 0x01, then fsck
>> will report such info each time..., can we relief fsck in such condition?
> 
> How about adding another preen mode?
> 
> For example,
>   - 2: same as 0, but skip some checks for old kernel

Good idea, let me change to add this in v2. :)

Thanks,

> 
>>
>> Thanks,
>>
>>>
  
DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n",
le32_to_cpu(node_blk->footer.ino), en,
 -- 
 2.18.0
>>>
>>> .
>>>
> 
> .
> 



___
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: fix to skip repairing initialized i_gc_failures

2018-11-26 Thread Jaegeuk Kim
On 11/27, Chao Yu wrote:
> On 2018/11/27 8:04, Jaegeuk Kim wrote:
> > On 11/26, Chao Yu wrote:
> >> From: Chao Yu 
> >>
> >> As Michael reported:
> >>
> >> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
> >> took quite a while and output ten-thousands instances of the following
> >> line:
> >>
> >>> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures 
> >>> from 0x1 to 0x00
> >>
> >> In old kernel, we initialized i_gc_failures as 0x01, let's skip
> >> resetting such unchanged initialized value to avoid unnecessary
> >> repairing.
> >>
> >> Reported-by: Michael Laß 
> >> Signed-off-by: Chao Yu 
> >> ---
> >>  fsck/fsck.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fsck/fsck.c b/fsck/fsck.c
> >> index 970d150..db0e72f 100644
> >> --- a/fsck/fsck.c
> >> +++ b/fsck/fsck.c
> >> @@ -941,7 +941,9 @@ skip_blkcnt_fix:
> >>}
> >>  
> >>i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures);
> >> -  if (ftype == F2FS_FT_REG_FILE && i_gc_failures) {
> >> +
> >> +  /* old kernel initialized i_gc_failures as 0x01, skip repairing */
> >> +  if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) {
> > 
> > This will break the current implementation.
> 
> Yeah, but I doubt that after repairing i_gc_failures, old kernel can still
> continue creating inodes in where .i_gc_failures equals to 0x01, then fsck
> will report such info each time..., can we relief fsck in such condition?

How about adding another preen mode?

For example,
  - 2: same as 0, but skip some checks for old kernel

> 
> Thanks,
> 
> > 
> >>  
> >>DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n",
> >>le32_to_cpu(node_blk->footer.ino), en,
> >> -- 
> >> 2.18.0
> > 
> > .
> > 


___
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: read page index before freeing

2018-11-26 Thread PanBian
On Tue, Nov 27, 2018 at 11:12:40AM +0800, Chao Yu wrote:
> On 2018/11/27 8:22, PanBian wrote:
> > On Mon, Nov 26, 2018 at 07:07:08PM +0800, Chao Yu wrote:
> >> On 2018/11/26 18:28, PanBian wrote:
> >>> On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote:
>  Hi Pan,
> 
>  On 2018/11/22 18:58, Pan Bian wrote:
> > The function truncate_node frees the page with f2fs_put_page. However,
> > the page index is read after that. So, the patch reads the index before
> > freeing the page.
> 
>  I notice that you found another use-after-free bug in ext4, out of
>  curiosity, I'd like to ask how do you find those bugs? by tool or code 
>  review?
> >>>
> >>> I found such bugs by the aid of a tool I wrote recently. I designed a 
> >>> method 
> >>> to automatically find paired alloc/free functions. With such functions, I
> >>> wrote two checkers, one to check mismatched alloc/free bugs, the other to
> >>> check use-after-free and double-free bugs.
> >>
> >> Excellent! Do you have any plan to open its source or announce it w/ binary
> >> to linux kernel developers, I think w/ it we can help to improve kernel's
> >> code quality efficiently.
> > 
> > Yes. I am now writing a paper about the method. I will open the source code
> > as soon as I complete the paper and some optimizations.
> 
> Cool, if there is any progress, please let f2fs guys know, thank you in
> advance. :)

No problem. It's my honor to apply my tool to the Linux kernel.

> 
> 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] f2fs: fix m_may_create to make OPU DIO write correctly

2018-11-26 Thread Chao Yu
Ping,

On 2018/11/20 19:58, Chao Yu wrote:
> On 2018-11-20 4:29, Jia Zhu wrote:
>> Previously, we added a parameter @map.m_may_create to trigger OPU
>> allocation and call f2fs_balance_fs() correctly.
>>
>> But in get_more_blocks(), @create has been overwritten by below code.
>> So the function f2fs_map_blocks() will not allocate new block address
>> but directly go out. Meanwile,there are several functions calling
>> f2fs_map_blocks() directly and @map.m_may_create not initialized.
> 
> Oh, I missed to check all f2fs_map_blocks structure referrers, sorry.
> 
>> CODE:
>> create = dio->op == REQ_OP_WRITE;
>>  if (dio->flags & DIO_SKIP_HOLES) {
>>  if (fs_startblk <= ((i_size_read(dio->inode) - 1) >>
>>  i_blkbits))
>>  create = 0;
>>  }
>>
>> This patch fixes it.
>>
>> Signed-off-by: Jia Zhu > ---
> 
> It will be better to add simple change logs here to indicate how you modify 
> your
> patch comparing to previous one, please keep that rule for your next patch. ;)
> 
> Reviewed-by: Chao Yu 
> 
> Thanks,
> 
>>  fs/f2fs/data.c | 5 +
>>  fs/f2fs/file.c | 4 +++-
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index aa8843a..7226300 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1052,6 +1052,10 @@ int f2fs_map_blocks(struct inode *inode, struct 
>> f2fs_map_blocks *map,
>>  end = pgofs + maxblocks;
>>  
>>  if (!create && f2fs_lookup_extent_cache(inode, pgofs, )) {
>> +if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO &&
>> +map->m_may_create)
>> +goto next_dnode;
>> +
>>  map->m_pblk = ei.blk + pgofs - ei.fofs;
>>  map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs);
>>  map->m_flags = F2FS_MAP_MAPPED;
>> @@ -1261,6 +1265,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t 
>> pos, size_t len)
>>  map.m_next_pgofs = NULL;
>>  map.m_next_extent = NULL;
>>  map.m_seg_type = NO_CHECK_TYPE;
>> +map.m_may_create = false;
>>  last_lblk = F2FS_BLK_ALIGN(pos + len);
>>  
>>  while (map.m_lblk < last_lblk) {
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 3271830..ff82350 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -2201,7 +2201,8 @@ static int f2fs_defragment_range(struct f2fs_sb_info 
>> *sbi,
>>  {
>>  struct inode *inode = file_inode(filp);
>>  struct f2fs_map_blocks map = { .m_next_extent = NULL,
>> -.m_seg_type = NO_CHECK_TYPE };
>> +.m_seg_type = NO_CHECK_TYPE ,
>> +.m_may_create = false };
>>  struct extent_info ei = {0, 0, 0};
>>  pgoff_t pg_start, pg_end, next_pgofs;
>>  unsigned int blk_per_seg = sbi->blocks_per_seg;
>> @@ -2935,6 +2936,7 @@ int f2fs_precache_extents(struct inode *inode)
>>  map.m_next_pgofs = NULL;
>>  map.m_next_extent = _next_extent;
>>  map.m_seg_type = NO_CHECK_TYPE;
>> +map.m_may_create = false;
>>  end = F2FS_I_SB(inode)->max_file_blocks;
>>  
>>  while (map.m_lblk < end) {
>>
> 
> .
> 



___
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: read page index before freeing

2018-11-26 Thread Chao Yu
On 2018/11/27 8:22, PanBian wrote:
> On Mon, Nov 26, 2018 at 07:07:08PM +0800, Chao Yu wrote:
>> On 2018/11/26 18:28, PanBian wrote:
>>> On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote:
 Hi Pan,

 On 2018/11/22 18:58, Pan Bian wrote:
> The function truncate_node frees the page with f2fs_put_page. However,
> the page index is read after that. So, the patch reads the index before
> freeing the page.

 I notice that you found another use-after-free bug in ext4, out of
 curiosity, I'd like to ask how do you find those bugs? by tool or code 
 review?
>>>
>>> I found such bugs by the aid of a tool I wrote recently. I designed a 
>>> method 
>>> to automatically find paired alloc/free functions. With such functions, I
>>> wrote two checkers, one to check mismatched alloc/free bugs, the other to
>>> check use-after-free and double-free bugs.
>>
>> Excellent! Do you have any plan to open its source or announce it w/ binary
>> to linux kernel developers, I think w/ it we can help to improve kernel's
>> code quality efficiently.
> 
> Yes. I am now writing a paper about the method. I will open the source code
> as soon as I complete the paper and some optimizations.

Cool, if there is any progress, please let f2fs guys know, thank you in
advance. :)

Thanks,

> 
> Best,
> Pan
> 
> 
> .
> 



___
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: fix to skip repairing initialized i_gc_failures

2018-11-26 Thread Chao Yu
On 2018/11/27 8:04, Jaegeuk Kim wrote:
> On 11/26, Chao Yu wrote:
>> From: Chao Yu 
>>
>> As Michael reported:
>>
>> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
>> took quite a while and output ten-thousands instances of the following
>> line:
>>
>>> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures 
>>> from 0x1 to 0x00
>>
>> In old kernel, we initialized i_gc_failures as 0x01, let's skip
>> resetting such unchanged initialized value to avoid unnecessary
>> repairing.
>>
>> Reported-by: Michael Laß 
>> Signed-off-by: Chao Yu 
>> ---
>>  fsck/fsck.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 970d150..db0e72f 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -941,7 +941,9 @@ skip_blkcnt_fix:
>>  }
>>  
>>  i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures);
>> -if (ftype == F2FS_FT_REG_FILE && i_gc_failures) {
>> +
>> +/* old kernel initialized i_gc_failures as 0x01, skip repairing */
>> +if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) {
> 
> This will break the current implementation.

Yeah, but I doubt that after repairing i_gc_failures, old kernel can still
continue creating inodes in where .i_gc_failures equals to 0x01, then fsck
will report such info each time..., can we relief fsck in such condition?

Thanks,

> 
>>  
>>  DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n",
>>  le32_to_cpu(node_blk->footer.ino), en,
>> -- 
>> 2.18.0
> 
> .
> 



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 5/7] ext4: use IS_VERITY() to check inode's fsverity status

2018-11-26 Thread Chandan Rajendra
On Monday, November 26, 2018 11:06:15 PM IST Theodore Y. Ts'o wrote:
> On Mon, Nov 19, 2018 at 10:53:22AM +0530, Chandan Rajendra wrote:
> > This commit now uses IS_VERITY() macro to check if fsverity is
> > enabled on an inode.
> > 
> > Signed-off-by: Chandan Rajendra 
> 
> This patch causes a massive number of fsverity tests.  I suspect it's
> due to a mismatch between the ext4's inode flags as opposed to the VFS
> inode's flags.  I'll take a closer look in the next day or two.
> 

I will check this and report back soon. 

-- 
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 v2] f2fs: fix sbi->extent_list corruption issue

2018-11-26 Thread Chao Yu
On 2018/11/27 8:30, Jaegeuk Kim wrote:
> On 11/26, Sahitya Tummala wrote:
>> When there is a failure in f2fs_fill_super() after/during
>> the recovery of fsync'd nodes, it frees the current sbi and
>> retries again. This time the mount is successful, but the files
>> that got recovered before retry, still holds the extent tree,
>> whose extent nodes list is corrupted since sbi and sbi->extent_list
>> is freed up. The list_del corruption issue is observed when the
>> file system is getting unmounted and when those recoverd files extent
>> node is being freed up in the below context.
>>
>> list_del corruption. prev->next should be fff1e1ef5480, but was (null)
>> <...>
>> kernel BUG at kernel/msm-4.14/lib/list_debug.c:53!
>> task: fff1f46f2280 task.stack: ff8008068000
>> lr : __list_del_entry_valid+0x94/0xb4
>> pc : __list_del_entry_valid+0x94/0xb4
>> <...>
>> Call trace:
>> __list_del_entry_valid+0x94/0xb4
>> __release_extent_node+0xb0/0x114
>> __free_extent_tree+0x58/0x7c
>> f2fs_shrink_extent_tree+0xdc/0x3b0
>> f2fs_leave_shrinker+0x28/0x7c
>> f2fs_put_super+0xfc/0x1e0
>> generic_shutdown_super+0x70/0xf4
>> kill_block_super+0x2c/0x5c
>> kill_f2fs_super+0x44/0x50
>> deactivate_locked_super+0x60/0x8c
>> deactivate_super+0x68/0x74
>> cleanup_mnt+0x40/0x78
>> __cleanup_mnt+0x1c/0x28
>> task_work_run+0x48/0xd0
>> do_notify_resume+0x678/0xe98
>> work_pending+0x8/0x14
>>
>> Fix this by cleaning up inodes, extent tree and nodes of those
>> recovered files before freeing up sbi and before next retry.
>>
>> Signed-off-by: Sahitya Tummala 
>> ---
>> v2:
>> -call evict_inodes() and f2fs_shrink_extent_tree() to cleanup inodes
>>
>>  fs/f2fs/f2fs.h |  1 +
>>  fs/f2fs/shrinker.c |  2 +-
>>  fs/f2fs/super.c| 13 -
>>  3 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 1e03197..aaee63b 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3407,6 +3407,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct 
>> rb_root_cached *root,
>>  bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi,
>>  struct rb_root_cached *root);
>>  unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int 
>> nr_shrink);
>> +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi);
>>  bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext);
>>  void f2fs_drop_extent_tree(struct inode *inode);
>>  unsigned int f2fs_destroy_extent_node(struct inode *inode);
>> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
>> index 9e13db9..7e3c13b 100644
>> --- a/fs/f2fs/shrinker.c
>> +++ b/fs/f2fs/shrinker.c
>> @@ -30,7 +30,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info 
>> *sbi)
>>  return count > 0 ? count : 0;
>>  }
>>  
>> -static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
>> +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
>>  {
>>  return atomic_read(>total_zombie_tree) +
>>  atomic_read(>total_ext_node);
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index af58b2c..769e7b1 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -3016,6 +3016,16 @@ static void f2fs_tuning_parameters(struct 
>> f2fs_sb_info *sbi)
>>  sbi->readdir_ra = 1;
>>  }
>>  
>> +static void f2fs_cleanup_inodes(struct f2fs_sb_info *sbi)
>> +{
>> +struct super_block *sb = sbi->sb;
>> +
>> +sync_filesystem(sb);
> 
> This writes another checkpoint, which would not be what this retrial intended.

Actually, checkpoint will not be triggered due to SBI_POR_DOING flag check
as below:

int f2fs_sync_fs(struct super_block *sb, int sync)
{
...
if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
return -EAGAIN;
...
}

And also all dirty data/node won't be persisted due to SBI_POR_DOING flag,
IIUC.

Thanks,

> How about adding a condition in f2fs_may_extent_tree() when adding extents?
> Likewise, if (shrinker is not registered) return false;
> 
> 
>> +shrink_dcache_sb(sb);
>> +evict_inodes(sb);
>> +f2fs_shrink_extent_tree(sbi, __count_extent_cache(sbi));
>> +}
>> +
>>  static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>  {
>>  struct f2fs_sb_info *sbi;
>> @@ -3402,6 +3412,8 @@ static int f2fs_fill_super(struct super_block *sb, 
>> void *data, int silent)
>>   * falls into an infinite loop in f2fs_sync_meta_pages().
>>   */
>>  truncate_inode_pages_final(META_MAPPING(sbi));
>> +/* cleanup recovery and quota inodes */
>> +f2fs_cleanup_inodes(sbi);
>>  f2fs_unregister_sysfs(sbi);
>>  free_root_inode:
>>  dput(sb->s_root);
>> @@ -3445,7 +3457,6 @@ static int f2fs_fill_super(struct super_block *sb, 
>> void *data, int silent)
>>  /* give only one another chance */
>>  if (retry) {
>>  retry = false;
>> -shrink_dcache_sb(sb);
>>  goto try_onemore;
>>  }
>>  return err;

Re: [f2fs-dev] [PATCH 7/7] fsverity: Remove filesystem specific build config option

2018-11-26 Thread Eric Biggers
Hi Chandan,

On Mon, Nov 19, 2018 at 10:53:24AM +0530, Chandan Rajendra wrote:
> In order to have a common code base for fsverity "post read" processing
> for all filesystems which support per-file verity, this commit removes
> filesystem specific build config option (e.g. CONFIG_EXT4_FS_VERITY) and
> replaces it with a build option (i.e. CONFIG_FS_VERITY) whose value
> affects all the filesystems making use of fsverity.
> 
> Signed-off-by: Chandan Rajendra 

Like the corresponding fscrypt patch, this is missing changing

#if IS_ENABLED(CONFIG_FS_VERITY)

in include/linux/fs.h to

#ifdef CONFIG_FS_VERITY

There are also references to the filesystem-specific config options in
Documentation/filesystems/fsverity.rst that need to be updated.

I also suggest updating the Kconfig help text for CONFIG_FS_VERITY and
CONFIG_FS_ENCRYPTION to mention the supported filesystems, similar to how
CONFIG_QUOTA lists the filesystems it supports.

Thanks!

- Eric

> ---
>  fs/ext4/Kconfig  | 20 
>  fs/ext4/ext4.h   |  2 --
>  fs/ext4/readpage.c   |  4 ++--
>  fs/ext4/super.c  |  6 +++---
>  fs/ext4/sysfs.c  |  4 ++--
>  fs/f2fs/Kconfig  | 20 
>  fs/f2fs/data.c   |  2 +-
>  fs/f2fs/f2fs.h   |  2 --
>  fs/f2fs/super.c  |  6 +++---
>  fs/f2fs/sysfs.c  |  4 ++--
>  fs/verity/Kconfig|  2 +-
>  include/linux/fsverity.h |  3 +--
>  12 files changed, 15 insertions(+), 60 deletions(-)
> 
> diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
> index e1002bbf35bf..031e5a82d556 100644
> --- a/fs/ext4/Kconfig
> +++ b/fs/ext4/Kconfig
> @@ -96,26 +96,6 @@ config EXT4_FS_SECURITY
> If you are not using a security module that requires using
> extended attributes for file security labels, say N.
>  
> -config EXT4_FS_VERITY
> - bool "Ext4 Verity"
> - depends on EXT4_FS
> - select FS_VERITY
> - help
> -   This option enables fs-verity for ext4.  fs-verity is the
> -   dm-verity mechanism implemented at the file level.  Userspace
> -   can append a Merkle tree (hash tree) to a file, then enable
> -   fs-verity on the file.  ext4 will then transparently verify
> -   any data read from the file against the Merkle tree.  The file
> -   is also made read-only.
> -
> -   This serves as an integrity check, but the availability of the
> -   Merkle tree root hash also allows efficiently supporting
> -   various use cases where normally the whole file would need to
> -   be hashed at once, such as auditing and authenticity
> -   verification (appraisal).
> -
> -   If unsure, say N.
> -
>  config EXT4_DEBUG
>   bool "EXT4 debugging support"
>   depends on EXT4_FS
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 64bf9fb7ef18..bff8d639dd0c 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -41,8 +41,6 @@
>  #endif
>  
>  #include 
> -
> -#define __FS_HAS_VERITY IS_ENABLED(CONFIG_EXT4_FS_VERITY)
>  #include 
>  
>  #include 
> diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
> index 2c037df629dd..8717ac0a5bb2 100644
> --- a/fs/ext4/readpage.c
> +++ b/fs/ext4/readpage.c
> @@ -158,7 +158,7 @@ static struct bio_post_read_ctx 
> *get_bio_post_read_ctx(struct inode *inode,
>  
>   if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
>   post_read_steps |= 1 << STEP_DECRYPT;
> -#ifdef CONFIG_EXT4_FS_VERITY
> +#ifdef CONFIG_FS_VERITY
>   if (inode->i_verity_info != NULL &&
>   (index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
>   post_read_steps |= 1 << STEP_VERITY;
> @@ -205,7 +205,7 @@ static void mpage_end_io(struct bio *bio)
>  
>  static inline loff_t ext4_readpage_limit(struct inode *inode)
>  {
> -#ifdef CONFIG_EXT4_FS_VERITY
> +#ifdef CONFIG_FS_VERITY
>   if (IS_VERITY(inode)) {
>   if (inode->i_verity_info)
>   /* limit to end of metadata region */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 16fb483a6f4a..472338c7cd03 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1316,7 +1316,7 @@ static const struct fscrypt_operations ext4_cryptops = {
>  };
>  #endif
>  
> -#ifdef CONFIG_EXT4_FS_VERITY
> +#ifdef CONFIG_FS_VERITY
>  static int ext4_set_verity(struct inode *inode, loff_t data_i_size)
>  {
>   int err;
> @@ -1401,7 +1401,7 @@ static const struct fsverity_operations ext4_verityops 
> = {
>   .set_verity = ext4_set_verity,
>   .get_metadata_end   = ext4_get_metadata_end,
>  };
> -#endif /* CONFIG_EXT4_FS_VERITY */
> +#endif /* CONFIG_FS_VERITY */
>  
>  #ifdef CONFIG_QUOTA
>  static const char * const quotatypes[] = INITQFNAMES;
> @@ -4234,7 +4234,7 @@ static int ext4_fill_super(struct super_block *sb, void 
> *data, int silent)
>  #ifdef CONFIG_FS_ENCRYPTION
>   sb->s_cop = _cryptops;
>  #endif
> -#ifdef CONFIG_EXT4_FS_VERITY
> +#ifdef CONFIG_FS_VERITY
>   sb->s_vop = _verityops;
>  

Re: [f2fs-dev] [PATCH v2] f2fs: fix to update new block address correctly for OPU

2018-11-26 Thread Jaegeuk Kim
Hi Jia,

On 11/27, Jia Zhu wrote:
> Previously, we allocated a new block address for OPU mode in direct_IO.
> 
> But the new address couldn't be assigned to @map->m_pblk correctly.
> 
> This patch fix it.
> 
> Fixes: 511f52d02f05 ('f2fs: allow out-place-update for direct IO in LFS mode')

I've marked this patch for -stable merge.

Thanks,

> 
> Signed-off-by: Jia Zhu 
> ---
> v2:
> - update commit message.
> 
>  fs/f2fs/data.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7226300..a3a567a 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1110,8 +1110,10 @@ int f2fs_map_blocks(struct inode *inode, struct 
> f2fs_map_blocks *map,
>   if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO &&
>   map->m_may_create) {
>   err = __allocate_data_block(, map->m_seg_type);
> - if (!err)
> + if (!err) {
> + blkaddr = dn.data_blkaddr;
>   set_inode_flag(inode, FI_APPEND_WRITE);
> + }
>   }
>   } else {
>   if (create) {
> -- 
> 2.10.1


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 6/7] f2fs: use IS_VERITY() to check inode's fsverity status

2018-11-26 Thread Eric Biggers
Hi Chandan,

On Mon, Nov 19, 2018 at 10:53:23AM +0530, Chandan Rajendra wrote:
> This commit now uses IS_VERITY() macro to check if fsverity is
> enabled on an inode.
> 
> Signed-off-by: Chandan Rajendra 
> ---
>  fs/f2fs/file.c  | 6 +++---
>  fs/f2fs/inode.c | 4 +++-
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6c7ad15000b9..2eb4821d95d1 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -491,7 +491,7 @@ static int f2fs_file_open(struct inode *inode, struct 
> file *filp)
>   if (err)
>   return err;
>  
> - if (f2fs_verity_file(inode)) {
> + if (IS_VERITY(inode)) {
>   err = fsverity_file_open(inode, filp);
>   if (err)
>   return err;
> @@ -701,7 +701,7 @@ int f2fs_getattr(const struct path *path, struct kstat 
> *stat,
>   struct f2fs_inode *ri;
>   unsigned int flags;
>  
> - if (f2fs_verity_file(inode)) {
> + if (IS_VERITY(inode)) {
>   /*
>* For fs-verity we need to override i_size with the original
>* data i_size.  This requires I/O to the file which with
> @@ -800,7 +800,7 @@ int f2fs_setattr(struct dentry *dentry, struct iattr 
> *attr)
>   if (err)
>   return err;
>  
> - if (f2fs_verity_file(inode)) {
> + if (IS_VERITY(inode)) {
>   err = fsverity_prepare_setattr(dentry, attr);
>   if (err)
>   return err;
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index ddef483ad689..02806feed64c 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -45,9 +45,11 @@ void f2fs_set_inode_flags(struct inode *inode)
>   new_fl |= S_DIRSYNC;
>   if (f2fs_encrypted_inode(inode))
>   new_fl |= S_ENCRYPTED;
> + if (f2fs_verity_file(inode))
> + new_fl |= S_VERITY;
>   inode_set_flags(inode, new_fl,
>   S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|
> - S_ENCRYPTED);
> + S_ENCRYPTED|S_VERITY);
>  }
>  
>  static void __get_inode_rdev(struct inode *inode, struct f2fs_inode *ri)
> -- 
> 2.19.1

Like the ext4 patch, it's missing syncing the verity flag to the VFS inode
during FS_IOC_ENABLE_VERITY:

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4f58d5840d17..841019d8bc96 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2207,6 +2207,7 @@ static int f2fs_set_verity(struct inode *inode, loff_t 
data_i_size)
return err;
 
file_set_verity(inode);
+   f2fs_set_inode_flags(inode);
f2fs_mark_inode_dirty_sync(inode, true);
return 0;
 }


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 1/7] ext4: use IS_ENCRYPTED() to check encryption status

2018-11-26 Thread Eric Biggers
Hi Chandan,

On Mon, Nov 19, 2018 at 10:53:18AM +0530, Chandan Rajendra wrote:
> @@ -4724,7 +4724,7 @@ static bool ext4_should_use_dax(struct inode *inode)
>   return false;
>   if (ext4_has_inline_data(inode))
>   return false;
> - if (ext4_encrypted_inode(inode))
> + if (IS_ENCRYPTED(inode))
>   return false;
>   if (ext4_verity_inode(inode))
>   return false;

I think this part is wrong.  See how ext4_should_use_dax() is called from
ext4_set_inode_flags(), from ext4_set_context().  In this case,
ext4_set_inode_flags() should set S_ENCRYPTED and clear S_DAX.  However, you've
changed ext4_should_use_dax() to check S_ENCRYPTED instead of EXT4_ENCRYPT_FL;
but S_ENCRYPTED isn't set until later in ext4_set_inode_flags(), so S_DAX won't
be cleared anymore.

So I think you need to use ext4_test_inode_flag() here instead.

Similarly for the verity check.

- 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] f2fs: fix sbi->extent_list corruption issue

2018-11-26 Thread Jaegeuk Kim
On 11/26, Sahitya Tummala wrote:
> When there is a failure in f2fs_fill_super() after/during
> the recovery of fsync'd nodes, it frees the current sbi and
> retries again. This time the mount is successful, but the files
> that got recovered before retry, still holds the extent tree,
> whose extent nodes list is corrupted since sbi and sbi->extent_list
> is freed up. The list_del corruption issue is observed when the
> file system is getting unmounted and when those recoverd files extent
> node is being freed up in the below context.
> 
> list_del corruption. prev->next should be fff1e1ef5480, but was (null)
> <...>
> kernel BUG at kernel/msm-4.14/lib/list_debug.c:53!
> task: fff1f46f2280 task.stack: ff8008068000
> lr : __list_del_entry_valid+0x94/0xb4
> pc : __list_del_entry_valid+0x94/0xb4
> <...>
> Call trace:
> __list_del_entry_valid+0x94/0xb4
> __release_extent_node+0xb0/0x114
> __free_extent_tree+0x58/0x7c
> f2fs_shrink_extent_tree+0xdc/0x3b0
> f2fs_leave_shrinker+0x28/0x7c
> f2fs_put_super+0xfc/0x1e0
> generic_shutdown_super+0x70/0xf4
> kill_block_super+0x2c/0x5c
> kill_f2fs_super+0x44/0x50
> deactivate_locked_super+0x60/0x8c
> deactivate_super+0x68/0x74
> cleanup_mnt+0x40/0x78
> __cleanup_mnt+0x1c/0x28
> task_work_run+0x48/0xd0
> do_notify_resume+0x678/0xe98
> work_pending+0x8/0x14
> 
> Fix this by cleaning up inodes, extent tree and nodes of those
> recovered files before freeing up sbi and before next retry.
> 
> Signed-off-by: Sahitya Tummala 
> ---
> v2:
> -call evict_inodes() and f2fs_shrink_extent_tree() to cleanup inodes
> 
>  fs/f2fs/f2fs.h |  1 +
>  fs/f2fs/shrinker.c |  2 +-
>  fs/f2fs/super.c| 13 -
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 1e03197..aaee63b 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3407,6 +3407,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct 
> rb_root_cached *root,
>  bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi,
>   struct rb_root_cached *root);
>  unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int 
> nr_shrink);
> +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi);
>  bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext);
>  void f2fs_drop_extent_tree(struct inode *inode);
>  unsigned int f2fs_destroy_extent_node(struct inode *inode);
> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> index 9e13db9..7e3c13b 100644
> --- a/fs/f2fs/shrinker.c
> +++ b/fs/f2fs/shrinker.c
> @@ -30,7 +30,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info 
> *sbi)
>   return count > 0 ? count : 0;
>  }
>  
> -static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
>  {
>   return atomic_read(>total_zombie_tree) +
>   atomic_read(>total_ext_node);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index af58b2c..769e7b1 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3016,6 +3016,16 @@ static void f2fs_tuning_parameters(struct f2fs_sb_info 
> *sbi)
>   sbi->readdir_ra = 1;
>  }
>  
> +static void f2fs_cleanup_inodes(struct f2fs_sb_info *sbi)
> +{
> + struct super_block *sb = sbi->sb;
> +
> + sync_filesystem(sb);

This writes another checkpoint, which would not be what this retrial intended.
How about adding a condition in f2fs_may_extent_tree() when adding extents?
Likewise, if (shrinker is not registered) return false;


> + shrink_dcache_sb(sb);
> + evict_inodes(sb);
> + f2fs_shrink_extent_tree(sbi, __count_extent_cache(sbi));
> +}
> +
>  static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  {
>   struct f2fs_sb_info *sbi;
> @@ -3402,6 +3412,8 @@ static int f2fs_fill_super(struct super_block *sb, void 
> *data, int silent)
>* falls into an infinite loop in f2fs_sync_meta_pages().
>*/
>   truncate_inode_pages_final(META_MAPPING(sbi));
> + /* cleanup recovery and quota inodes */
> + f2fs_cleanup_inodes(sbi);
>   f2fs_unregister_sysfs(sbi);
>  free_root_inode:
>   dput(sb->s_root);
> @@ -3445,7 +3457,6 @@ static int f2fs_fill_super(struct super_block *sb, void 
> *data, int silent)
>   /* give only one another chance */
>   if (retry) {
>   retry = false;
> - shrink_dcache_sb(sb);
>   goto try_onemore;
>   }
>   return err;
> -- 
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
> Foundation Collaborative Project.


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 5/7] ext4: use IS_VERITY() to check inode's fsverity status

2018-11-26 Thread Eric Biggers
On Mon, Nov 26, 2018 at 12:36:15PM -0500, Theodore Y. Ts'o wrote:
> On Mon, Nov 19, 2018 at 10:53:22AM +0530, Chandan Rajendra wrote:
> > This commit now uses IS_VERITY() macro to check if fsverity is
> > enabled on an inode.
> > 
> > Signed-off-by: Chandan Rajendra 
> 
> This patch causes a massive number of fsverity tests.  I suspect it's
> due to a mismatch between the ext4's inode flags as opposed to the VFS
> inode's flags.  I'll take a closer look in the next day or two.
> 
> Cheers,
> 
>   - Ted

It's missing the following to set S_VERITY during the
FS_IOC_ENABLE_VERITY ioctl:

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ed933e64e95f..82b45cceb39b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1344,6 +1344,11 @@ static int ext4_set_verity(struct inode *inode, loff_t 
data_i_size)
err = ext4_reserve_inode_write(handle, inode, );
if (err == 0) {
ext4_set_inode_flag(inode, EXT4_INODE_VERITY);
+   /*
+* Update inode->i_flags - S_VERITY will be enabled,
+* S_DAX may be disabled
+*/
+   ext4_set_inode_flags(inode);
EXT4_I(inode)->i_disksize = data_i_size;
err = ext4_mark_iloc_dirty(handle, inode, );
}


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 3/7] fscrypt: Remove filesystem specific build config option

2018-11-26 Thread Eric Biggers
Hi Chandan,

On Mon, Nov 19, 2018 at 10:53:20AM +0530, Chandan Rajendra wrote:
> In order to have a common code base for fscrypt "post read" processing
> for all filesystems which support encryption, this commit removes
> filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION)
> and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
> value affects all the filesystems making use of fscrypt.
> 
> Signed-off-by: Chandan Rajendra 

Can you grep for EXT4_ENCRYPTION?  There are still some references left, one in
Documentation/filesystems/fscrypt.rst and some in defconfig files.

Also in include/linux/fs.h, there are tests of

#if IS_ENABLED(CONFIG_FS_ENCRYPTION)

which after this change should be

#ifdef CONFIG_FS_ENCRYPTION

like is done elsewhere.

Thanks,

- Eric

> ---
>  fs/crypto/Kconfig   |   2 +-
>  fs/ext4/Kconfig |  15 --
>  fs/ext4/dir.c   |   2 -
>  fs/ext4/ext4.h  |   7 +-
>  fs/ext4/inode.c |   8 +-
>  fs/ext4/ioctl.c |   4 +-
>  fs/ext4/namei.c |  10 +-
>  fs/ext4/page-io.c   |   6 +-
>  fs/ext4/readpage.c  |   2 +-
>  fs/ext4/super.c |   6 +-
>  fs/ext4/sysfs.c |   4 +-
>  fs/f2fs/Kconfig |  11 -
>  fs/f2fs/f2fs.h  |   7 +-
>  fs/f2fs/super.c |   8 +-
>  fs/f2fs/sysfs.c |   4 +-
>  include/linux/fscrypt.h | 416 +++-
>  include/linux/fscrypt_notsupp.h | 231 --
>  include/linux/fscrypt_supp.h| 204 
>  18 files changed, 441 insertions(+), 506 deletions(-)
>  delete mode 100644 include/linux/fscrypt_notsupp.h
>  delete mode 100644 include/linux/fscrypt_supp.h
> 
> diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> index 02b7d91c9231..6e9ae566a8fc 100644
> --- a/fs/crypto/Kconfig
> +++ b/fs/crypto/Kconfig
> @@ -1,5 +1,5 @@
>  config FS_ENCRYPTION
> - tristate "FS Encryption (Per-file encryption)"
> + bool "FS Encryption (Per-file encryption)"
>   select CRYPTO
>   select CRYPTO_AES
>   select CRYPTO_CBC
> diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
> index 5a76125ac0f8..e1002bbf35bf 100644
> --- a/fs/ext4/Kconfig
> +++ b/fs/ext4/Kconfig
> @@ -96,21 +96,6 @@ config EXT4_FS_SECURITY
> If you are not using a security module that requires using
> extended attributes for file security labels, say N.
>  
> -config EXT4_ENCRYPTION
> - bool "Ext4 Encryption"
> - depends on EXT4_FS
> - select FS_ENCRYPTION
> - help
> -   Enable encryption of ext4 files and directories.  This
> -   feature is similar to ecryptfs, but it is more memory
> -   efficient since it avoids caching the encrypted and
> -   decrypted pages in the page cache.
> -
> -config EXT4_FS_ENCRYPTION
> - bool
> - default y
> - depends on EXT4_ENCRYPTION
> -
>  config EXT4_FS_VERITY
>   bool "Ext4 Verity"
>   depends on EXT4_FS
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index fb7a64ea5679..0ccd51f72048 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -283,9 +283,7 @@ static int ext4_readdir(struct file *file, struct 
> dir_context *ctx)
>  done:
>   err = 0;
>  errout:
> -#ifdef CONFIG_EXT4_FS_ENCRYPTION
>   fscrypt_fname_free_buffer();
> -#endif
>   brelse(bh);
>   return err;
>  }
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 2ae6ab88f218..db21df885186 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -40,7 +40,6 @@
>  #include 
>  #endif
>  
> -#define __FS_HAS_ENCRYPTION IS_ENABLED(CONFIG_EXT4_FS_ENCRYPTION)
>  #include 
>  
>  #define __FS_HAS_VERITY IS_ENABLED(CONFIG_EXT4_FS_VERITY)
> @@ -1341,7 +1340,7 @@ struct ext4_super_block {
>  #define EXT4_MF_FS_ABORTED   0x0002  /* Fatal error detected */
>  #define EXT4_MF_TEST_DUMMY_ENCRYPTION0x0004
>  
> -#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +#ifdef CONFIG_FS_ENCRYPTION
>  #define DUMMY_ENCRYPTION_ENABLED(sbi) (unlikely((sbi)->s_mount_flags & \
>   EXT4_MF_TEST_DUMMY_ENCRYPTION))
>  #else
> @@ -2069,7 +2068,7 @@ struct ext4_filename {
>   const struct qstr *usr_fname;
>   struct fscrypt_str disk_name;
>   struct dx_hash_info hinfo;
> -#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +#ifdef CONFIG_FS_ENCRYPTION
>   struct fscrypt_str crypto_buf;
>  #endif
>  };
> @@ -2306,7 +2305,7 @@ static inline bool ext4_verity_inode(struct inode 
> *inode)
>  #endif
>  }
>  
> -#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +#ifdef CONFIG_FS_ENCRYPTION
>  static inline int ext4_fname_setup_filename(struct inode *dir,
>   const struct qstr *iname,
>   int lookup, struct ext4_filename *fname)
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 72572f32..ae6794649817 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1150,7 +1150,7 @@ int 

Re: [f2fs-dev] fsck: reset i_gc_failures from 0x1 to 0x00

2018-11-26 Thread Jaegeuk Kim
Hi Michael,

On 11/26, Michael Laß wrote:
> Hi,
> 
> > Am 26.11.2018 um 15:09 schrieb Chao Yu :
> > On 2018-11-26 7:09, Michael Laß wrote:
> >> Hi,
> >> 
> >> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
> >> took quite a while and output ten-thousands instances of the following
> >> line:
> >> 
> >>> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures 
> >>> from 0x1 to 0x00
> > 
> > Do you use -f or -p 1 option when doing fsck on image?
> 
> One of the devices was automatically checked during boot-up. As far as I can 
> see, the following command is issued inside the initrd:
> fsck -Ta -C"$FSCK_FD" "$1”
> where $1 is the device. The other one I checked manually calling fsck.f2fs 
> without any additional arguments. From my experience, the checks are always 
> performed when the last check was done with an older kernel version (which 
> was the case here).
> 
> > We start to support reseting .i_gc_failures's value to zero in fsck since
> > 91bb7b21f740 ("f2fs-tools: fix to reset i_gc_failures offline"), this is 
> > because
> > if .i_gc_failures continues increasing and exceed threshold, it can make 
> > f2fs
> > break atomic_write semantics during GC, so I added that patch to avoid such
> > condition.
> > 
> > But the problem here is even .i_gc_failures's value is one which was 
> > initialized
> > duing inode creation by old kernel, and it never be increased by GC flow, we
> > will still trigger such fix in fsck. I think it's not necessary, anyway, 
> > let me
> > send one patch to fix it.
> 
> This is a very likely cause. The filesystems are both from early 2015, so 
> probably were used with Linux 3.18 or 3.19 at that time.

Just in case, is this Android device? If you don't use ioctl(F2FS_PIN_FILE),
this onetime fix wont' hurt any filesystem metadata.

Thanks,

> 
> Thanks for the explanation and the proposed patch!
> 
> Best regards,
> Michael
> 
> ___
> 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 4/7] Add S_VERITY and IS_VERITY()

2018-11-26 Thread Eric Biggers
Hi Chandan,

On Mon, Nov 19, 2018 at 10:53:21AM +0530, Chandan Rajendra wrote:
> Similar to S_ENCRYPTED/IS_ENCRYPTED(), this commit adds
> S_VERITY/IS_VERITY() to be able to check if a VFS inode has verity
> information associated with it.
> 
> Signed-off-by: Chandan Rajendra 
> ---
>  include/linux/fs.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index bcfc40062757..8129617c9718 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1938,6 +1938,7 @@ struct super_operations {
>  #define S_DAX0   /* Make all the DAX code disappear */
>  #endif
>  #define S_ENCRYPTED  16384   /* Encrypted file (using fs/crypto/) */
> +#define S_VERITY 32768   /* File with fsverity info (using fs/verity) */
>  

The comment for S_VERITY is misleading because IS_VERITY() is used to check
whether the verity bit is set *before* the fsverity_info is created.

Can you change it to just mirror the fscrypt comment?

#define S_VERITY32768   /* Verity file (using fs/verity/) */

>  /*
>   * Note that nosuid etc flags are inode-specific: setting some file-system
> @@ -1978,6 +1979,7 @@ static inline bool sb_rdonly(const struct super_block 
> *sb) { return sb->s_flags
>  #define IS_NOSEC(inode)  ((inode)->i_flags & S_NOSEC)
>  #define IS_DAX(inode)((inode)->i_flags & S_DAX)
>  #define IS_ENCRYPTED(inode)  ((inode)->i_flags & S_ENCRYPTED)
> +#define IS_VERITY(inode) ((inode)->i_flags & S_VERITY)
>  
>  #define IS_WHITEOUT(inode)   (S_ISCHR(inode->i_mode) && \
>(inode)->i_rdev == WHITEOUT_DEV)
> -- 
> 2.19.1
> 

Thanks,

- 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] fsck.f2fs: fix to skip repairing initialized i_gc_failures

2018-11-26 Thread Jaegeuk Kim
On 11/26, Chao Yu wrote:
> From: Chao Yu 
> 
> As Michael reported:
> 
> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
> took quite a while and output ten-thousands instances of the following
> line:
> 
> > [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures 
> > from 0x1 to 0x00
> 
> In old kernel, we initialized i_gc_failures as 0x01, let's skip
> resetting such unchanged initialized value to avoid unnecessary
> repairing.
> 
> Reported-by: Michael Laß 
> Signed-off-by: Chao Yu 
> ---
>  fsck/fsck.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 970d150..db0e72f 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -941,7 +941,9 @@ skip_blkcnt_fix:
>   }
>  
>   i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures);
> - if (ftype == F2FS_FT_REG_FILE && i_gc_failures) {
> +
> + /* old kernel initialized i_gc_failures as 0x01, skip repairing */
> + if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) {

This will break the current implementation.

>  
>   DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n",
>   le32_to_cpu(node_blk->footer.ino), en,
> -- 
> 2.18.0


___
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/7] f2fs: use IS_ENCRYPTED() to check encryption status

2018-11-26 Thread Jaegeuk Kim
Hi Ted,

On 11/26, Theodore Y. Ts'o wrote:
> On Sun, Nov 25, 2018 at 11:00:38PM -0500, Theodore Y. Ts'o wrote:
> > 
> > It might be that the simplest way to solve things is to merge the f2fs
> > dev branch up to 79c66e75720c.  This will have the net effect of
> > including the five patches listed above onto the fscrypt git tree.  So
> > long you don't plan to rebase or otherwise change these five patches,
> > it should avoid any merge conflicts.
> 
> I've set up a git branch which has the f2fs dev branch, 4.20-rc4, the
> fsverity patches, and part of Chandan's patch series here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt.git test-working
> 
> There is a minor conflict when I did a trial merge with f2fs.git's dev
> branch, but it's pretty obvious to how to resolve it.
> 
> Jaegeuk, Eric, Chandan, please take a look and let me know what you
> think.

I was about to rebase f2fs-dev branch to catch up -rc4, so could you please
update test-working with the rebased one? Then, I'll test Eric & Chandan's
patches in the test-working branch locally. If there is no issue, I'm okay
to push all of their work via fscrypt.git, if you prefer.

Afterward, merging f2fs patches till "f2fs: clean up f2fs_sb_has_##feature_name"
into the test-working branch would be fine as well.

Thanks,

> 
> - 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] f2fs: check memory boundary by insane namelen

2018-11-26 Thread Jaegeuk Kim
On 11/23, Sheng Yong wrote:
> Hi, Jaegeuk and Chao,
> 
> On 2018/11/15 15:50, Jaegeuk Kim wrote:
> > If namelen is corrupted to have very long value, fill_dentries can copy
> > wrong memory area.
> > 
> Is there any scenario that could hit this corruption? Or this is triggered
> by fuzzing injection?

Hi Sheng,

It's from a fuzzing test.

Thanks,

> 
> thanks,
> Sheng Yong
> 
> > Signed-off-by: Jaegeuk Kim 
> > ---
> >   fs/f2fs/dir.c | 12 +++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> > index bacc667950b6..c0c845da12fa 100644
> > --- a/fs/f2fs/dir.c
> > +++ b/fs/f2fs/dir.c
> > @@ -808,6 +808,17 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct 
> > f2fs_dentry_ptr *d,
> > de_name.name = d->filename[bit_pos];
> > de_name.len = le16_to_cpu(de->name_len);
> > +   /* check memory boundary before moving forward */
> > +   bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
> > +   if (unlikely(bit_pos > d->max)) {
> > +   f2fs_msg(sbi->sb, KERN_WARNING,
> > +   "%s: corrupted namelen=%d, run fsck to fix.",
> > +   __func__, le16_to_cpu(de->name_len));
> > +   set_sbi_flag(sbi, SBI_NEED_FSCK);
> > +   err = -EINVAL;
> > +   goto out;
> > +   }
> > +
> > if (f2fs_encrypted_inode(d->inode)) {
> > int save_len = fstr->len;
> > @@ -830,7 +841,6 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct 
> > f2fs_dentry_ptr *d,
> > if (readdir_ra)
> > f2fs_ra_node_page(sbi, le32_to_cpu(de->ino));
> > -   bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
> > ctx->pos = start_pos + bit_pos;
> > }
> >   out:
> > 


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 5/7] ext4: use IS_VERITY() to check inode's fsverity status

2018-11-26 Thread Theodore Y. Ts'o
On Mon, Nov 19, 2018 at 10:53:22AM +0530, Chandan Rajendra wrote:
> This commit now uses IS_VERITY() macro to check if fsverity is
> enabled on an inode.
> 
> Signed-off-by: Chandan Rajendra 

This patch causes a massive number of fsverity tests.  I suspect it's
due to a mismatch between the ext4's inode flags as opposed to the VFS
inode's flags.  I'll take a closer look in the next day or two.

Cheers,

- 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 2/7] f2fs: use IS_ENCRYPTED() to check encryption status

2018-11-26 Thread Theodore Y. Ts'o
On Sun, Nov 25, 2018 at 11:00:38PM -0500, Theodore Y. Ts'o wrote:
> 
> It might be that the simplest way to solve things is to merge the f2fs
> dev branch up to 79c66e75720c.  This will have the net effect of
> including the five patches listed above onto the fscrypt git tree.  So
> long you don't plan to rebase or otherwise change these five patches,
> it should avoid any merge conflicts.

I've set up a git branch which has the f2fs dev branch, 4.20-rc4, the
fsverity patches, and part of Chandan's patch series here:

git://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt.git test-working

There is a minor conflict when I did a trial merge with f2fs.git's dev
branch, but it's pretty obvious to how to resolve it.

Jaegeuk, Eric, Chandan, please take a look and let me know what you
think.

  - Ted


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] fsck: reset i_gc_failures from 0x1 to 0x00

2018-11-26 Thread Michael Laß
Hi,

> Am 26.11.2018 um 15:09 schrieb Chao Yu :
> On 2018-11-26 7:09, Michael Laß wrote:
>> Hi,
>> 
>> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
>> took quite a while and output ten-thousands instances of the following
>> line:
>> 
>>> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures 
>>> from 0x1 to 0x00
> 
> Do you use -f or -p 1 option when doing fsck on image?

One of the devices was automatically checked during boot-up. As far as I can 
see, the following command is issued inside the initrd:
fsck -Ta -C"$FSCK_FD" "$1”
where $1 is the device. The other one I checked manually calling fsck.f2fs 
without any additional arguments. From my experience, the checks are always 
performed when the last check was done with an older kernel version (which was 
the case here).

> We start to support reseting .i_gc_failures's value to zero in fsck since
> 91bb7b21f740 ("f2fs-tools: fix to reset i_gc_failures offline"), this is 
> because
> if .i_gc_failures continues increasing and exceed threshold, it can make f2fs
> break atomic_write semantics during GC, so I added that patch to avoid such
> condition.
> 
> But the problem here is even .i_gc_failures's value is one which was 
> initialized
> duing inode creation by old kernel, and it never be increased by GC flow, we
> will still trigger such fix in fsck. I think it's not necessary, anyway, let 
> me
> send one patch to fix it.

This is a very likely cause. The filesystems are both from early 2015, so 
probably were used with Linux 3.18 or 3.19 at that time.

Thanks for the explanation and the proposed patch!

Best regards,
Michael

___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures

2018-11-26 Thread Chao Yu
From: Chao Yu 

As Michael reported:

after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
took quite a while and output ten-thousands instances of the following
line:

> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures from 
> 0x1 to 0x00

In old kernel, we initialized i_gc_failures as 0x01, let's skip
resetting such unchanged initialized value to avoid unnecessary
repairing.

Reported-by: Michael Laß 
Signed-off-by: Chao Yu 
---
 fsck/fsck.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 970d150..db0e72f 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -941,7 +941,9 @@ skip_blkcnt_fix:
}
 
i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures);
-   if (ftype == F2FS_FT_REG_FILE && i_gc_failures) {
+
+   /* old kernel initialized i_gc_failures as 0x01, skip repairing */
+   if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) {
 
DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n",
le32_to_cpu(node_blk->footer.ino), en,
-- 
2.18.0



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] fsck: reset i_gc_failures from 0x1 to 0x00

2018-11-26 Thread Chao Yu
Hi Michael,

On 2018-11-26 7:09, Michael Laß wrote:
> Hi,
> 
> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems
> took quite a while and output ten-thousands instances of the following
> line:
> 
>> [FIX] (fsck_chk_inode_blk: 954)  --> Regular: 0xXYZ reset i_gc_failures from 
>> 0x1 to 0x00

Do you use -f or -p 1 option when doing fsck on image?

> 
> where XYZ varied from line to line. I assume this is related to
> 91bb7b21f740d61cde2bb27da3dccb0afdd5a15b but I'm not sure about the
> implications. Is this something to worry about or some deliberate
> change of the on-disk format which was now braught up to date by fsck?

We start to support reseting .i_gc_failures's value to zero in fsck since
91bb7b21f740 ("f2fs-tools: fix to reset i_gc_failures offline"), this is because
if .i_gc_failures continues increasing and exceed threshold, it can make f2fs
break atomic_write semantics during GC, so I added that patch to avoid such
condition.

But the problem here is even .i_gc_failures's value is one which was initialized
 duing inode creation by old kernel, and it never be increased by GC flow, we
will still trigger such fix in fsck. I think it's not necessary, anyway, let me
send one patch to fix it.

Thanks,

> 
> Best regards,
> Michael
> 
> 
> 
> 
> 
> ___
> 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] Bug in f2fs-tools-1.12.0

2018-11-26 Thread Chao Yu
Hi Perfect Gentleman,

On 2018-11-26 21:47, Perfect Gentleman wrote:
> Hi Chao
> 
> With that patch fsck.f2fs works for  RO-partition with -f option.

Thanks for your test. :)

Thanks,

> 
>  ~ $ sudo mount -o remount,ro /dev/sda1
> 
>  ~ $ sudo fsck.f2fs -a /dev/sda1
> Info: Fix the reported corruption.
> Info: Mounted device!
> Info: Check FS only due to RO
>     Error: Failed to open the device!
> 
> ~ $ sudo fsck.f2fs -af /dev/sda1
> Info: Fix the reported corruption.
> Info: Force to fix corruption
> Info: Mounted device!
> Info: Check FS only due to RO
> Info: [/dev/sda1] Disk Model: SAMSUNG MZ7WD240
> Info: Segments per section = 1
> Info: Sections per zone = 1
> Info: sector size = 512
> Info: total sectors = 468860047 (228935 MB)
> Info: MKFS version
>   "Linux version 4.16.6-gentoo (root@kubuntu) (gcc version 7.3.0 (Gentoo
> 7.3.0-r1 p1.1)) #1 ZEN SMP PREEMPT Tue May 1 18:28:05 +07 2018"
> Info: FSCK version
>   from "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 (Gentoo
> 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018"
>     to "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 (Gentoo
> 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018"
> Info: superblock features = 0 :
> Info: superblock encrypt level = 0, salt = 
> Info: total FS sectors = 468860040 (228935 MB)
> Info: CKPT version = 40e50ae1
> Info: Checked valid nat_bits in checkpoint
> Info: checkpoint state = c5 :  nat_bits crc compacted_summary unmount
> 
> [FSCK] Unreachable nat entries    [Ok..] [0x0]
> [FSCK] SIT valid block bitmap checking    [Ok..]
> [FSCK] Hard link checking for regular file    [Ok..] [0x0]
> [FSCK] valid_block_count matching with CP [Ok..] [0x334ce10]
> [FSCK] valid_node_count matcing with CP (de lookup)   [Ok..] [0x1fe41]
> [FSCK] valid_node_count matcing with CP (nat lookup)  [Ok..] [0x1fe41]
> [FSCK] valid_inode_count matched with CP  [Ok..] [0x12fb1]
> [FSCK] free segment_count matched with CP [Ok..] [0x41f0]
> [FSCK] next block offset is free  [Ok..]
> [FSCK] fixing SIT types
> [FSCK] other corrupted bugs   [Ok..]
> 
> Done.
> 
> Regards
> 
> On 11/26/18 8:35 PM, Chao Yu wrote:
>> Hi Perfect Gentleman,
>>
>> On 2018-11-26 19:11, Perfect Gentleman wrote:
>>> Hi Chao
>>>
>>> What patch do you mean?
>> I mean
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test=a6160c3e21f43b89b49802cc4a956d1c4b65ae44
>>
>>
>> Thanks,
>>
>>> Redards
>>>
>>> P.S. It's reply to all :)
>>>
>>> On 11/26/18 5:52 PM, Chao Yu wrote:
 Hi Perfect Gentleman,

 Thanks for the report.

 On 2018/11/25 13:24, Perfect Gentleman wrote:
> Hi f2fs-team,
>
> there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0
> can do that.
>
> fsck  |Info: Fix the reported corruption.
> fsck  |Info: Mounted device!
> fsck  |Info: Check FS only due to RO
> fsck  |Error: Failed to open the device!
 I guess this is due to below commit, which stop opening device in
 f2fs-tools if the device has already been opened by other one, like mount.

 https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64



 I tried e2fsprogs, w/o -f option, the result is the same:

 fsck 1.44.4 (18-Aug-2018)
 e2fsck 1.44.4 (18-Aug-2018)
 /dev/zram0 is mounted.
 e2fsck: Cannot continue, aborting.

 But still we can trigger forced fsck.ext4 by using -f option.

 Could you have a try with following patch?

 [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option

 Thanks,

> fsck  | * Filesystems couldn't be fixed
>     [ !! ]
> fsck  | * ERROR: fsck failed to start
>
> Regards,
> Alex
>
>
>
> ___
> 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


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] Bug in f2fs-tools-1.12.0

2018-11-26 Thread Perfect Gentleman

Hi Chao

With that patch fsck.f2fs works for  RO-partition with -f option.

 ~ $ sudo mount -o remount,ro /dev/sda1

 ~ $ sudo fsck.f2fs -a /dev/sda1
Info: Fix the reported corruption.
Info: Mounted device!
Info: Check FS only due to RO
    Error: Failed to open the device!

~ $ sudo fsck.f2fs -af /dev/sda1
Info: Fix the reported corruption.
Info: Force to fix corruption
Info: Mounted device!
Info: Check FS only due to RO
Info: [/dev/sda1] Disk Model: SAMSUNG MZ7WD240
Info: Segments per section = 1
Info: Sections per zone = 1
Info: sector size = 512
Info: total sectors = 468860047 (228935 MB)
Info: MKFS version
  "Linux version 4.16.6-gentoo (root@kubuntu) (gcc version 7.3.0 
(Gentoo 7.3.0-r1 p1.1)) #1 ZEN SMP PREEMPT Tue May 1 18:28:05 +07 2018"

Info: FSCK version
  from "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 
(Gentoo 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018"
    to "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 
(Gentoo 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018"

Info: superblock features = 0 :
Info: superblock encrypt level = 0, salt = 
Info: total FS sectors = 468860040 (228935 MB)
Info: CKPT version = 40e50ae1
Info: Checked valid nat_bits in checkpoint
Info: checkpoint state = c5 :  nat_bits crc compacted_summary unmount

[FSCK] Unreachable nat entries    [Ok..] [0x0]
[FSCK] SIT valid block bitmap checking    [Ok..]
[FSCK] Hard link checking for regular file    [Ok..] [0x0]
[FSCK] valid_block_count matching with CP [Ok..] [0x334ce10]
[FSCK] valid_node_count matcing with CP (de lookup)   [Ok..] [0x1fe41]
[FSCK] valid_node_count matcing with CP (nat lookup)  [Ok..] [0x1fe41]
[FSCK] valid_inode_count matched with CP  [Ok..] [0x12fb1]
[FSCK] free segment_count matched with CP [Ok..] [0x41f0]
[FSCK] next block offset is free  [Ok..]
[FSCK] fixing SIT types
[FSCK] other corrupted bugs   [Ok..]

Done.

Regards

On 11/26/18 8:35 PM, Chao Yu wrote:

Hi Perfect Gentleman,

On 2018-11-26 19:11, Perfect Gentleman wrote:

Hi Chao

What patch do you mean?

I mean

https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test=a6160c3e21f43b89b49802cc4a956d1c4b65ae44

Thanks,


Redards

P.S. It's reply to all :)

On 11/26/18 5:52 PM, Chao Yu wrote:

Hi Perfect Gentleman,

Thanks for the report.

On 2018/11/25 13:24, Perfect Gentleman wrote:

Hi f2fs-team,

there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0
can do that.

fsck  |Info: Fix the reported corruption.
fsck  |Info: Mounted device!
fsck  |Info: Check FS only due to RO
fsck  |Error: Failed to open the device!

I guess this is due to below commit, which stop opening device in
f2fs-tools if the device has already been opened by other one, like mount.

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64


I tried e2fsprogs, w/o -f option, the result is the same:

fsck 1.44.4 (18-Aug-2018)
e2fsck 1.44.4 (18-Aug-2018)
/dev/zram0 is mounted.
e2fsck: Cannot continue, aborting.

But still we can trigger forced fsck.ext4 by using -f option.

Could you have a try with following patch?

[PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option

Thanks,


fsck  | * Filesystems couldn't be fixed
    [ !! ]
fsck  | * ERROR: fsck failed to start

Regards,
Alex



___
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



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] f2fs-tools: fix to check return value of {c, m}alloc()

2018-11-26 Thread Chao Yu
From: Chao Yu 

It needs to fix to handle error case of {c,m}alloc().

Signed-off-by: Chao Yu 
---
 fsck/dump.c  |  4 
 fsck/fsck.c  |  2 ++
 fsck/mount.c | 13 +
 lib/libf2fs.c|  4 
 lib/libf2fs_io.c |  4 
 5 files changed, 27 insertions(+)

diff --git a/fsck/dump.c b/fsck/dump.c
index 8cf431c..d0e3355 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -277,6 +277,8 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int 
ntype,
get_node_info(sbi, nid, );
 
node_blk = calloc(BLOCK_SZ, 1);
+   ASSERT(node_blk);
+
dev_read_block(node_blk, ni.blk_addr);
 
for (i = 0; i < idx; i++, (*ofs)++) {
@@ -475,6 +477,8 @@ void dump_node(struct f2fs_sb_info *sbi, nid_t nid, int 
force)
get_node_info(sbi, nid, );
 
node_blk = calloc(BLOCK_SZ, 1);
+   ASSERT(node_blk);
+
dev_read_block(node_blk, ni.blk_addr);
 
DBG(1, "Node ID   [0x%x]\n", nid);
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 366ba13..970d150 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -1420,6 +1420,8 @@ static int __chk_dentries(struct f2fs_sb_info *sbi, 
struct child_info *child,
continue;
}
name = calloc(name_len + 1, 1);
+   ASSERT(name);
+
memcpy(name, filenames[i], name_len);
slots = (name_len + F2FS_SLOT_LEN - 1) / F2FS_SLOT_LEN;
 
diff --git a/fsck/mount.c b/fsck/mount.c
index 861e5fb..d853fcf 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -664,6 +664,8 @@ int validate_super_block(struct f2fs_sb_info *sbi, enum 
SB_ADDR sb_addr)
char buf[F2FS_BLKSIZE];
 
sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
+   if (!sbi->raw_super)
+   return -ENOMEM;
 
if (dev_read_block(buf, sb_addr))
return -1;
@@ -779,6 +781,8 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t 
cp_addr,
 
/* Read the 1st cp block in this CP pack */
cp_page_1 = malloc(PAGE_SIZE);
+   ASSERT(cp_page_1);
+
if (dev_read_block(cp_page_1, cp_addr) < 0)
goto invalid_cp1;
 
@@ -798,6 +802,8 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t 
cp_addr,
 
/* Read the 2nd cp block in this CP pack */
cp_page_2 = malloc(PAGE_SIZE);
+   ASSERT(cp_page_2);
+
cp_addr += get_cp(cp_pack_total_block_count) - 1;
 
if (dev_read_block(cp_page_2, cp_addr) < 0)
@@ -1320,6 +1326,9 @@ int build_sit_info(struct f2fs_sb_info *sbi)
src_bitmap = __bitmap_ptr(sbi, SIT_BITMAP);
 
dst_bitmap = malloc(bitmap_size);
+   if (!dst_bitmap)
+   return -ENOMEM;
+
memcpy(dst_bitmap, src_bitmap, bitmap_size);
 
sit_i->sit_base_addr = get_sb(sit_blkaddr);
@@ -1361,6 +1370,8 @@ static void read_compacted_summaries(struct f2fs_sb_info 
*sbi)
start = start_sum_block(sbi);
 
kaddr = (char *)malloc(PAGE_SIZE);
+   ASSERT(kaddr);
+
ret = dev_read_block(kaddr, start++);
ASSERT(ret >= 0);
 
@@ -1452,6 +1463,8 @@ static void read_normal_summaries(struct f2fs_sb_info 
*sbi, int type)
}
 
sum_blk = (struct f2fs_summary_block *)malloc(PAGE_SIZE);
+   ASSERT(sum_blk);
+
ret = dev_read_block(sum_blk, blk_addr);
ASSERT(ret >= 0);
 
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 498f6c0..c692bf2 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -566,6 +566,8 @@ char *get_rootdev()
}
 
uevent = malloc(ret + 1);
+   ASSERT(uevent);
+
uevent[ret] = '\0';
 
ret = read(fd, uevent, ret);
@@ -709,6 +711,8 @@ int f2fs_dev_is_umounted(char *path)
 * the file system. In this case, we should not format.
 */
st_buf = malloc(sizeof(struct stat));
+   ASSERT(st_buf);
+
if (stat(path, st_buf) == 0 && S_ISBLK(st_buf->st_mode)) {
int fd = open(path, O_RDONLY | O_EXCL);
 
diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
index 47917ab..b40c3d2 100644
--- a/lib/libf2fs_io.c
+++ b/lib/libf2fs_io.c
@@ -302,6 +302,10 @@ int f2fs_init_sparse_file(void)
}
blocks_count = c.device_size / F2FS_BLKSIZE;
blocks = calloc(blocks_count, sizeof(char *));
+   if (!blocks) {
+   MSG(0, "\tError: Calloc Failed for blocks!!!\n");
+   return -1;
+   }
 
return sparse_file_foreach_chunk(f2fs_sparse_file, true, false,
sparse_import_segment, NULL);
-- 
2.18.0



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] Bug in f2fs-tools-1.12.0

2018-11-26 Thread Chao Yu
Hi Perfect Gentleman,

On 2018-11-26 19:11, Perfect Gentleman wrote:
> Hi Chao
> 
> What patch do you mean?

I mean

https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test=a6160c3e21f43b89b49802cc4a956d1c4b65ae44

Thanks,

> 
> Redards
> 
> P.S. It's reply to all :)
> 
> On 11/26/18 5:52 PM, Chao Yu wrote:
>> Hi Perfect Gentleman,
>>
>> Thanks for the report.
>>
>> On 2018/11/25 13:24, Perfect Gentleman wrote:
>>> Hi f2fs-team,
>>>
>>> there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0
>>> can do that.
>>>
>>> fsck  |Info: Fix the reported corruption.
>>> fsck  |Info: Mounted device!
>>> fsck  |Info: Check FS only due to RO
>>> fsck  |Error: Failed to open the device!
>> I guess this is due to below commit, which stop opening device in
>> f2fs-tools if the device has already been opened by other one, like mount.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64
>>
>>
>> I tried e2fsprogs, w/o -f option, the result is the same:
>>
>> fsck 1.44.4 (18-Aug-2018)
>> e2fsck 1.44.4 (18-Aug-2018)
>> /dev/zram0 is mounted.
>> e2fsck: Cannot continue, aborting.
>>
>> But still we can trigger forced fsck.ext4 by using -f option.
>>
>> Could you have a try with following patch?
>>
>> [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option
>>
>> Thanks,
>>
>>> fsck  | * Filesystems couldn't be fixed
>>>    [ !! ]
>>> fsck  | * ERROR: fsck failed to start
>>>
>>> Regards,
>>> Alex
>>>
>>>
>>>
>>> ___
>>> 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


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] Bug in f2fs-tools-1.12.0

2018-11-26 Thread Perfect Gentleman

Hi Chao

What patch do you mean?

Redards

P.S. It's reply to all :)

On 11/26/18 5:52 PM, Chao Yu wrote:

Hi Perfect Gentleman,

Thanks for the report.

On 2018/11/25 13:24, Perfect Gentleman wrote:

Hi f2fs-team,

there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0
can do that.

fsck  |Info: Fix the reported corruption.
fsck  |Info: Mounted device!
fsck  |Info: Check FS only due to RO
fsck  |Error: Failed to open the device!

I guess this is due to below commit, which stop opening device in
f2fs-tools if the device has already been opened by other one, like mount.

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64

I tried e2fsprogs, w/o -f option, the result is the same:

fsck 1.44.4 (18-Aug-2018)
e2fsck 1.44.4 (18-Aug-2018)
/dev/zram0 is mounted.
e2fsck: Cannot continue, aborting.

But still we can trigger forced fsck.ext4 by using -f option.

Could you have a try with following patch?

[PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option

Thanks,


fsck  | * Filesystems couldn't be fixed
   [ !! ]
fsck  | * ERROR: fsck failed to start

Regards,
Alex



___
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] f2fs: read page index before freeing

2018-11-26 Thread Chao Yu
On 2018/11/26 18:28, PanBian wrote:
> On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote:
>> Hi Pan,
>>
>> On 2018/11/22 18:58, Pan Bian wrote:
>>> The function truncate_node frees the page with f2fs_put_page. However,
>>> the page index is read after that. So, the patch reads the index before
>>> freeing the page.
>>
>> I notice that you found another use-after-free bug in ext4, out of
>> curiosity, I'd like to ask how do you find those bugs? by tool or code 
>> review?
> 
> I found such bugs by the aid of a tool I wrote recently. I designed a method 
> to automatically find paired alloc/free functions. With such functions, I
> wrote two checkers, one to check mismatched alloc/free bugs, the other to
> check use-after-free and double-free bugs.

Excellent! Do you have any plan to open its source or announce it w/ binary
to linux kernel developers, I think w/ it we can help to improve kernel's
code quality efficiently.

Thanks,

> 
> Best regards,
> Pan Bian
> 
> 
> .
> 



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option

2018-11-26 Thread Chao Yu
To keep line with e2fsprogs, let's allow to fsck mounted image as
readonly w/ -f option.

Reported-by: Perfect Gentleman 
Signed-off-by: Chao Yu 
---
 fsck/main.c   | 1 +
 include/f2fs_fs.h | 1 +
 lib/libf2fs.c | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fsck/main.c b/fsck/main.c
index 675c603b1a76..bb79f6e9fc89 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -249,6 +249,7 @@ void f2fs_parse_options(int argc, char *argv[])
case 'f':
case 'y':
c.fix_on = 1;
+   c.force = 1;
MSG(0, "Info: Force to fix corruption\n");
break;
case 'q':
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 65cc8fdb3c22..6eebb3ac44e1 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -369,6 +369,7 @@ struct f2fs_configuration {
void *private;
int dry_run;
int fix_on;
+   int force;
int defset;
int bug_on;
int alloc_failed;
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index cc335dbb48ee..498f6c0968d8 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -821,7 +821,7 @@ int get_device_info(int i)
return -1;
}
 
-   if (S_ISBLK(stat_buf->st_mode))
+   if (S_ISBLK(stat_buf->st_mode) && !c.force)
fd = open(dev->path, O_RDWR | O_EXCL);
else
fd = open(dev->path, O_RDWR);
-- 
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] Bug in f2fs-tools-1.12.0

2018-11-26 Thread Chao Yu
Hi Perfect Gentleman,

Thanks for the report.

On 2018/11/25 13:24, Perfect Gentleman wrote:
> Hi f2fs-team,
> 
> there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0 
> can do that.
> 
> fsck  |Info: Fix the reported corruption.
> fsck  |Info: Mounted device!
> fsck  |Info: Check FS only due to RO
> fsck  |Error: Failed to open the device!

I guess this is due to below commit, which stop opening device in
f2fs-tools if the device has already been opened by other one, like mount.

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64

I tried e2fsprogs, w/o -f option, the result is the same:

fsck 1.44.4 (18-Aug-2018)
e2fsck 1.44.4 (18-Aug-2018)
/dev/zram0 is mounted.
e2fsck: Cannot continue, aborting.

But still we can trigger forced fsck.ext4 by using -f option.

Could you have a try with following patch?

[PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option

Thanks,

> fsck  | * Filesystems couldn't be fixed
>   [ !! ]
> fsck  | * ERROR: fsck failed to start
> 
> Regards,
> Alex
> 
> 
> 
> ___
> 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 v2] f2fs: fix to update new block address correctly for OPU

2018-11-26 Thread Greg KH
On Tue, Nov 27, 2018 at 02:32:32AM +0800, Jia Zhu wrote:
> Previously, we allocated a new block address for OPU mode in direct_IO.
> 
> But the new address couldn't be assigned to @map->m_pblk correctly.
> 
> This patch fix it.
> 
> Fixes: 511f52d02f05 ('f2fs: allow out-place-update for direct IO in LFS mode')
> 
> Signed-off-by: Jia Zhu 
> ---
> v2:
> - update commit message.
> 
>  fs/f2fs/data.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.




___
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: read page index before freeing

2018-11-26 Thread PanBian
On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote:
> Hi Pan,
> 
> On 2018/11/22 18:58, Pan Bian wrote:
> > The function truncate_node frees the page with f2fs_put_page. However,
> > the page index is read after that. So, the patch reads the index before
> > freeing the page.
> 
> I notice that you found another use-after-free bug in ext4, out of
> curiosity, I'd like to ask how do you find those bugs? by tool or code review?

I found such bugs by the aid of a tool I wrote recently. I designed a method 
to automatically find paired alloc/free functions. With such functions, I
wrote two checkers, one to check mismatched alloc/free bugs, the other to
check use-after-free and double-free bugs.

Best regards,
Pan Bian



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v2] f2fs: fix to update new block address correctly for OPU

2018-11-26 Thread Jia Zhu
Previously, we allocated a new block address for OPU mode in direct_IO.

But the new address couldn't be assigned to @map->m_pblk correctly.

This patch fix it.

Fixes: 511f52d02f05 ('f2fs: allow out-place-update for direct IO in LFS mode')

Signed-off-by: Jia Zhu 
---
v2:
- update commit message.

 fs/f2fs/data.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7226300..a3a567a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1110,8 +1110,10 @@ int f2fs_map_blocks(struct inode *inode, struct 
f2fs_map_blocks *map,
if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO &&
map->m_may_create) {
err = __allocate_data_block(, map->m_seg_type);
-   if (!err)
+   if (!err) {
+   blkaddr = dn.data_blkaddr;
set_inode_flag(inode, FI_APPEND_WRITE);
+   }
}
} else {
if (create) {
-- 
2.10.1



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v3 2/2] f2fs: adjust trace print in f2fs_get_victim() to cover all paths

2018-11-26 Thread Chao Yu
On 2018/11/26 16:01, Sahitya Tummala wrote:
> Adjust the trace print in f2fs_get_victim() to cover GC done by
> F2FS_IOC_GARBAGE_COLLECT_RANGE.
> 
> Signed-off-by: Sahitya Tummala 

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 v3 1/2] f2fs: fix to allow node segment for GC by ioctl path

2018-11-26 Thread Chao Yu
On 2018/11/26 16:01, Sahitya Tummala wrote:
> Allow node type segments also to be GC'd via f2fs ioctl
> F2FS_IOC_GARBAGE_COLLECT_RANGE.
> 
> Signed-off-by: Sahitya Tummala 

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] f2fs: read page index before freeing

2018-11-26 Thread Chao Yu
Hi Pan,

On 2018/11/22 18:58, Pan Bian wrote:
> The function truncate_node frees the page with f2fs_put_page. However,
> the page index is read after that. So, the patch reads the index before
> freeing the page.

I notice that you found another use-after-free bug in ext4, out of
curiosity, I'd like to ask how do you find those bugs? by tool or code review?

Thanks,



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v3 2/2] f2fs: adjust trace print in f2fs_get_victim() to cover all paths

2018-11-26 Thread Sahitya Tummala
Adjust the trace print in f2fs_get_victim() to cover GC done by
F2FS_IOC_GARBAGE_COLLECT_RANGE.

Signed-off-by: Sahitya Tummala 
---
 fs/f2fs/gc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index d720551..e4689c6 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -403,11 +403,12 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
}
*result = (p.min_segno / p.ofs_unit) * p.ofs_unit;
 
+   }
+out:
+   if (p.min_segno != NULL_SEGNO)
trace_f2fs_get_victim(sbi->sb, type, gc_type, ,
sbi->cur_victim_sec,
prefree_segments(sbi), free_segments(sbi));
-   }
-out:
mutex_unlock(_i->seglist_lock);
 
return (p.min_segno == NULL_SEGNO) ? 0 : 1;
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v3 1/2] f2fs: fix to allow node segment for GC by ioctl path

2018-11-26 Thread Sahitya Tummala
Allow node type segments also to be GC'd via f2fs ioctl
F2FS_IOC_GARBAGE_COLLECT_RANGE.

Signed-off-by: Sahitya Tummala 
---
v3:
seperate the trace print change from this patch

 fs/f2fs/gc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index a07241f..d720551 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -323,8 +323,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
p.min_cost = get_max_cost(sbi, );
 
if (*result != NULL_SEGNO) {
-   if (IS_DATASEG(get_seg_entry(sbi, *result)->type) &&
-   get_valid_blocks(sbi, *result, false) &&
+   if (get_valid_blocks(sbi, *result, false) &&
!sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result)))
p.min_segno = *result;
goto out;
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel