linux-ext4 - Re: [PATCH 1/2] ext4, project: expand inode extra size if possible

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: <fcc6383e-a6a4-e221-e58b-b5fa2ea52e25@huawei.com>
Date:   Mon, 3 Jul 2017 10:49:29 +0800
From:   Miao Xie <miaoxie@...wei.com>
To:     Wang Shilong <wshilong@....com>,
        Wang Shilong <wangshilong1991@...il.com>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
CC:     "tytso@....edu" <tytso@....edu>, Li Xi <lixi@....com>,
        "yi.zhang@...wei.com" <yi.zhang@...wei.com>,
        "adilger@...ger.ca" <adilger@...ger.ca>,
        Shuichi Ihara <sihara@....com>
Subject: Re: [PATCH 1/2] ext4, project: expand inode extra size if possible



on 2017/7/3 at 9:16, Wang Shilong wrote:
> 
> ________________________________________
> From: Miao Xie [miaoxie@...wei.com]
> Sent: Friday, June 30, 2017 16:48
> To: Wang Shilong; linux-ext4@...r.kernel.org
> Cc: tytso@....edu; Li Xi; yi.zhang@...wei.com; adilger@...ger.ca; Wang Shilong; Shuichi Ihara
> Subject: Re: [PATCH 1/2] ext4, project: expand inode extra size if possible
> 
> on 2017/6/30 at 11:51, Wang Shilong wrote:
>> when upgrading from old format, try to set project id
>> to old file first time, it will return EOVERFLOW, but if
>> that file is dirtied(touch etc), changing project id will
> <...SNIP...>
> 
> ext4_expand_extra_isize should be invoked after ext4_reserve_inode_write.
> 
> And I think it is better to restructure ext4_expand_extra_isize by moving NO_EXPAND check,
> nojournal check and journal credits extend into it, and then we just if i_projid is in
> 
> ---->I agreed we could move NO_EXPAND check, but i don't think it good idea to move journal
> credits extend to it, jbd2_extend_journal() might fail, and we'd better avoid it.
> 
> For changing projectid, we could know how many credits before start transaction..

I found most check in set_projectid is the same as ext4_mark_inode_dirty, so I think it's better
to move those checks into ext4_expand_extra_isize to avoid the reduplicated code.

And I don't think jbd2_journal_extend's failure is a big problem, because we just invoke it once
most of the time, and even if we fail to extend the inode because of jbd2_journal_extend's failure,
the metadata is still safe, so it is unnecessary to change many codes in set_projectid, or
we will impact readability of the code.

> 
> the inode or not, if not, invoke ext4_expand_extra_isize. (don't forget to do cleanup for
> ext4_mark_inode_dirty), And then we can remove many check in the above code.
> 
> 
> ---->what do you mean cleanup for ext4_mark_inode_dirty()? I supposed you mean don't
> call ext4_mark_iloc_dirty() if extend fail? i think that is expected, even inode extend fail,
> if ext4_mark_inode_dirty() is called, it means inode is dirtied already before we call the
> function.

I means if we move the checks into ext4_expand_extra_isize, we should do cleanup for ext4_mark_inode_dirty.
(ext4_mark_iloc_dirty() shoud be invoked even if extend fail, because we need update the on-disk inode)

Thanks
Miao

> Thanks,
> Shilong
> 
> 
> .
>