RE: ubifs: read bad node type in ubifs_tnc_read_wbuf

[李傲傲 (Carson Li1/9542) <Carson.Li1@xxxxxxxxxx>]

On Monday, March 2, 2020 2:35 PM, Hou Tao<houtao1@xxxxxxxxxx> wrote:

>>> The actual situation of the problem is the LEB is GCed, freed and then
>>> reused as journal head, and finally ubifs_tnc_locate() reads an invalid node.
>>
>> Actually, I think that situation might only be caused by a commit, is that right?
>> Since only commit might clear the journal head LEBs' property of LPROPS_TAKEN.
>> But it will not get the c->jheads[i].wbuf->lnum LEB's taken property cleared, so there
>> seems no need to check if a c->jheads[i].wbuf->lnum LEB might be GCed and the
>> node whether fully sits in wirte buffer.
>No. The GC'ed checking is needed here. As I have noted in the commit message:

>>And it can be reproduced by the following steps:
>>* create 128 empty files
>>* overwrite 8 files in backgroup repeatedly to trigger GC
>>* drop inode cache and stat these 128 empty files repeatedly

>In the above steps, the nodes related with these empty files are already been committed
>long before the running of stat command.

Sorry, I am a little confused by your comment saying the nodes are already been committed
long before the running of stat command.
It only happens when there is a commit between tnc_mutex released and ubifs_read_node_wbuf
executed, am I right ? How could it be so long in the gap?
--------------------------------------------------------------------------
ubifs_tnc_locate
   zbr->lnum = 54 (find in TNC)
     ubifs_get_wbuf(zbr->lnum = 54) is true
          ubifs_tnc_read_node
                         ->GC(change zt->lnum to 224(GCHD) in _TNC_)
                         ->zbr->lnum = 54 becomes DATAHD
             ubifs_get_wbuf(zbr->lnum = 54 as the DATAHD) is true again
             ubifs_read_node_wbuf
--------------------------------------------------------------------------
when commit happens, ubifs_log_start_commit will get present jheads[DATAHD].wbuf->lnum
written back to bud list and log LEB again. So What I mean is that if the zbr->lnum = 54 is just
exactly the jheads[DATAHD].wbuf->lnum, it will not be GCed.

But if there are more than one commits happen, it certainly needs GCed checking.
I am just not sure if we need to take such a less possible situation into account, since I thought
it might not take too long between tnc_mutex released and ubifs_read_node_wbuf executed.
Your patch can be thought to have taken this account in my view.

>Do you mean call may_leb_gced() after the release of wbuf->lock ? If it's the case,
>it's OK to me because it will reduce the hold time of wbuf->lock.
Yes, that is what I mean.

>It's in the first patch and I have not attached it here. It's a helper function factored out
>from ubifs_read_node_wbuf() and is used to check the validity of node in buffer:
Ok, get it.

Thanks.
Carson.

________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/