linux-kernel - Re: [PATCH RFC 00/17] ubifs: Add filesystem repair support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <460eb02e-8937-282c-62c5-6ea606324b0e@huawei.com>
Date: Wed, 3 Jan 2024 11:18:34 +0800
From: Zhihao Cheng <chengzhihao1@...wei.com>
To: Richard Weinberger <richard@....at>
CC: david oberhollenzer <david.oberhollenzer@...ma-star.at>, Miquel Raynal
	<miquel.raynal@...tlin.com>, Sascha Hauer <s.hauer@...gutronix.de>, Tudor
 Ambarus <Tudor.Ambarus@...aro.org>, linux-kernel
	<linux-kernel@...r.kernel.org>, linux-mtd <linux-mtd@...ts.infradead.org>
Subject: Re: [PATCH RFC 00/17] ubifs: Add filesystem repair support

在 2024/1/3 4:45, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <chengzhihao1@...wei.com>
>> I come up with another way to implement fsck.ubifs:
>>
>> There are three modes:
>>
>> 1. common mode(no options): Ask user whether to fix as long as a problem
>> is detected.
> 
> Makes sense.
> 
>> 2. safe mode(-a option): Auto repair as long as no data/files lost(eg.
>> nlink, isize, xattr_cnt, which can be corrected without dropping nodes),
>> otherwise returns error code.
> 
> Makes sense.
>   
>> 3. dangerous mode(-y option): Fix is always successful, unless
>> superblock is corrupted. There are 2 situations:
> 
> Please not use "-y". Usually "-y" stands for "answer yes to all questions".
> But selecting names for command line flags is currently my least concern.
>   
>>    a) TNC is valid: fsck will print which file is dropped and which
>> file's data is dropped
> 
> Sounds also good.
>   
>>    b) TNC is invalid: fsck will scan all nodes without referencing TNC,
>> same as this patchset does
> 
> I'd make this a distinct mode.
> It can be used to rebuild index and LEB property trees.
> This is basically a "drop trees and rebuild" mode.
> 

OK, then fsck will have four modes.

>>
>> Q1: How do you think of this method?
> 
> Sounds good so far.
>   
>> Q2: Mode 1, 2 and 3(a) depend on journal replaying, I found
>> xfs_repair[1] should be used after mounting/unmounting xfs (Let kernel
>> replay journal), if UBIFS does so, there is no need to copy recovery
>> subsystem into userspace, but user has to mount/unmount before doing
>> fsck. I found e2fsck has copied recovery code into userspace, so it can
>> do fsck without mounting/unmounting. If UBIFS does so, journal replaying
>> will update TNC and LPT, please reference Q3(1). Which method do you
>> suggest?
> 
> UBIFS is a little special regarding the journal.
> 
> 1. The journal is not an add-on like it is on many other file systems,
> you have to replay it to get a consistent file system.
> 2. Journal replay is also needed after a clean umount. The reason is that
> UBIFS does no commit at umount time.

I agree, there exists one situation that UBIFS replays journal even 
after clean umount.
     P1      ubifs_bgt      umount
   mkdir
          run_bg_commit
           c->cmt_state = COMMIT_RUNNING_BACKGROUND
           do_commit
            ubifs_log_start_commit(c, &new_ltail_lnum) // log start
            up_write(&c->commit_sem)
   touch
    ubifs_jnl_update // new buds added
                          cleanup_mnt
                           deactivate_super
                            fs->kill_sb
                             generic_shutdown_super
                              sync_filesystem
                               ubifs_sync_fs
                                ubifs_run_commit
                                 wait_for_commit // wait bg commit, 
'touch' won't be commited, it will be replayed in next mount

> 
> So IMHO you need to have journal replay code in your tool in any case.
> You can copy it from the kernel implementation and add more checks.
> While the kernel code also tries to be fast, fsck should be more paranoid.
> Ideally the outcome is a libubifs or such which can be derived from the
> kernel source while building mtd-utils.

All right, sounds like a huge copy work.

> 
>> Q3: If fsck drops or updates a node(eg. dentry lost inode, corrected
>> inode) in mode 1,2 and 3(a), TNC/LPT should be updated. There are two
>> ways updating TNC and LPT:
>>
>>    1) Like kernel does, which means that mark dirty TNC/LPT for
>> corresponding branches and do_commit(). It will copy much code into
>> userspace, eg. tnc.c, lpt.c, tnc_commit.c,
>> lpt_commit.c. I fear there exists risks. For example, there is no space
>> left for new index nodes, gc should be performed? If so, gc/lpt gc code
>> should be copied too.
>>
>>    2) Rebuild new TNC/LPT based on valid nodes. This way is simple, but
>> old good TNC could be corrupted, it means that powercut during fsck may
>> let UBIFS image must be repaired in mode 3(b) but it could be repaired
>> in mode 2\3(a) before invoking fsck.
>>
>> Which way is better?
> 
> Since you need to do a full journal replay anyway and power-cut awareness
> is one of your requirements, I fear the only option is 1). >
> Thanks,
> //richard
> .
>