Re: ubifs: read bad node type in ubifs_tnc_read_wbuf

[Hou Tao <houtao1@xxxxxxxxxx>]

Hi,

On 2020/2/27 9:18, 李傲傲 (Carson Li1/9542) wrote:
> Hi tao,
> Thanks for your patch.
>> diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
>> index e8e7b0e9532e..af35c1ff1ab4 100644
>> --- a/fs/ubifs/tnc.c
>> +++ b/fs/ubifs/tnc.c
>> @@ -1478,7 +1478,16 @@ int ubifs_tnc_locate(struct ubifs_info *c, const union ubifs_key *key,
>> mutex_unlock(&c->tnc_mutex);
> 
>> if (ubifs_get_wbuf(c, zbr.lnum)) {
>> -/* We do not GC journal heads */
>> +/*
>> + * We do not GC journal heads. However if zbr.lnum
>> + * is GC'ed, freed and then reused as GC journal head,
>> + * we also need to protect node reading by tnc_mutex.
>> + */
>> +if (maybe_leb_gced(c, zbr.lnum, gc_seq1)) {
>> +safely = 1;
>> +goto again;
>> +}
>> +
>> err = ubifs_tnc_read_node(c, &zbr, node);
>> return err;
>> }
>> --
> 
> But I have a question about your patch if it can fix the problem when it happens
> like below. The LEB might be GCed in ubifs_tnc_read_node, and there is also a
> ubifs_get_wbuf.
> 
>>>> By the way, there is another timing the LEB might be garbage collected:
>>>>>     A      |              B
>>>>> --------------------------------------------------------------------------
>>>>> ubifs_tnc_locate
>>>>>   zbr->lnum = 54 (find in TNC)
>>>>>     ubifs_get_wbuf(zbr->lnum = 54) is ture
>>>>>          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 ture again
>>>>>             ubifs_read_node_wbuf
>>>>> --------------------------------------------------------------------------

Let's check it:

>>>>>     A      |              B
>>>>> --------------------------------------------------------------------------
>>>>> ubifs_tnc_locate
>>>>>   zbr->lnum = 54 (find in TNC)
>>>>>     ubifs_get_wbuf(zbr->lnum = 54) is true
               maybe_leb_gced() return false
>>>>>          ubifs_tnc_read_node()

                              ->commit happens and 54 is removed from bud list
                              ->commit ends and 54 is GC-able
                              ->GC move 54 to 224
                              ->reuse 54 as DATAHD
>>>>>             ubifs_get_wbuf(zbr->lnum = 54 as the DATAHD) is true again
>>>>>             ubifs_read_node_wbuf

So adding an extra maybe_led_gced() doesn't help in the above case. Thanks for pointing it out.

And the quick solution comes to me is removing the following optimization for reading write buffer:

@@ -1482,12 +1489,6 @@ again:
        gc_seq1 = c->gc_seq;
        mutex_unlock(&c->tnc_mutex);

-       if (ubifs_get_wbuf(c, zbr.lnum)) {
-               /* We do not GC journal heads */
-               err = ubifs_tnc_read_node(c, &zbr, node);
-               return err;
-       }
-
        err = fallible_read_node(c, key, &zbr, node);
        if (err <= 0 || maybe_leb_gced(c, zbr.lnum, gc_seq1)) {

But let me think whether or not there is any better solution.

Regards,
Tao




> 
> 
> 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/