On 2018/1/29 16:58, Chao Yu wrote:
Hi Sheng Yong,

On 2018/1/29 16:39, Sheng Yong wrote:
Hi, Chao

On 2018/1/29 16:27, Chao Yu wrote:
On 2018/1/29 16:04, Sheng Yong wrote:
sb_getblk does not guarantee the buffer head is uptodate. If bh is not
uptodate, the data (may be used as boot code) in area before

Why boot code can be stored into the position f2fs superblock locates?
According to Ext4_Disk_Layout [1], the first 1024 bytes is used to install
boot sectors (may be used by some legacy bootloader, I'm not quite sure :(

OK, I can understand your concern now, anyway, no matter the area is used
by someone else or not, let's keep data in that area as it is as possible
as we can.

I checked the change again, I think we should also remove the extra
set_buffer_uptodate(bh); in __f2fs_commit_super like the following,

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8173ae688814..07d72d651c45 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1894,7 +1894,6 @@ static int __f2fs_commit_super(struct buffer_head *bh,
        lock_buffer(bh);
        if (super)
                memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
-       set_buffer_uptodate(bh);
        set_buffer_dirty(bh);
        unlock_buffer(bh);

I will resend it, thanks.


Reviewed-by: Chao Yu <yuch...@huawei.com>

Thanks,


And even we have updated the buffer, we will still commit superblock's
datas into that buffer, boot code can be messed up anyway, right?
The first 1024 bytes are all set to 0 by mkfs, while __f2fs_commit_super only
updates bh->b_data from F2FS_SUPER_OFFSET, but not the whole 4KB area. If the
buffer head is not read from super block (4KB), there is a chance it may get
messed up.

Thanks,
Sheng


[1] https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout
Thanks,

F2FS_SUPER_OFFSET may get corrupted when super block is committed.

Signed-off-by: Sheng Yong <shengyo...@huawei.com>
---
   fs/f2fs/super.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8173ae688814..7def48cb2b22 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2334,7 +2334,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool 
recover)
        }
/* write back-up superblock first */
-       bh = sb_getblk(sbi->sb, sbi->valid_super_block ? 0: 1);
+       bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
        if (!bh)
                return -EIO;
        err = __f2fs_commit_super(bh, F2FS_RAW_SUPER(sbi));
@@ -2345,7 +2345,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool 
recover)
                return err;
/* write current valid superblock */
-       bh = sb_getblk(sbi->sb, sbi->valid_super_block);
+       bh = sb_bread(sbi->sb, sbi->valid_super_block);
        if (!bh)
                return -EIO;
        err = __f2fs_commit_super(bh, F2FS_RAW_SUPER(sbi));



.



.



.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to