Re: ubifs: read bad node type in ubifs_tnc_read_wbuf

[Hou Tao <houtao1@xxxxxxxxxx>]

Hi,

It seems that the LEB is GCed, freed and then reused as journal head, and
leads to the failure of ubifs_read_node_wbuf(). And the problem can be
reproduced by using nandsim.

So could you please try the following patch:

From: Hou Tao <houtao1@xxxxxxxxxx>
Date: Wed, 26 Feb 2020 19:05:40 +0800
Subject: [PATCH] ubifs: check whether LEB is GCed even it is journal head
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Carson Li Reports the following oops:

 UBIFS error: ubifs_read_node_wbuf: expected node type 0
 Not a node, first 24 bytes:
 Kernel panic - not syncing
 CPU: 1 PID: 943 Comm: http-thread 4.4.83 #1
   panic+0x70/0x1e4
   ubifs_dump_node+0x6c/0x9a0
   ubifs_read_node_wbuf+0x350/0x384
   ubifs_tnc_read_node+0x54/0x214
   ubifs_tnc_locate+0x118/0x1b4
   ubifs_iget+0xb8/0x68c
   ubifs_lookup+0x1b4/0x258
   lookup_real+0x30/0x4c
   __lookup_hash+0x34/0x3c
   walk_component+0xec/0x2a0
   path_lookupat+0x80/0xfc
   filename_lookup+0x5c/0xfc
   vfs_fstatat+0x4c/0x9c
   SyS_stat64+0x14/0x30
   ret_fast_syscall+0x0/0x34

It seems the LEB used as DATA journal head is GC'ed, and ubifs_tnc_locate()
read a stale node data. But now the property of journal head LEB has
LPROPS_TAKEN flag set and GC will skip these LEBs.

The actual situation of the problem is the LEB is GCed, freed and then
reused as journal head, and finally ubifs_tnc_locate() read a stale node.
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

Fix it by checking whether the LEB is GCed or not before trying to read
it from the write buffer.

Fixes: 601c0bc46753 ("UBIFS: allow for racing between GC and TNC")
Reported-and-analyzed-by: 李傲傲 (Carson Li1/9542) <Carson.Li1@xxxxxxxxxx>
Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
---
 fs/ubifs/tnc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

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;
 	}
-- 
2.25.0.4.g0ad7144999


Regards,
Tao

On 2020/2/19 20:03, 李傲傲 (Carson Li1/9542) wrote:
> Hi Richard:
> I am sorry that I had sent an email to the linux-mtd but find out
> I did not copy to you. Now I resend it again, please kindly check it
> and reply me.
> 
>>> LPROPS_TAKEN should avoid this.
>> Is it possible that a commit finishes and removes the LPROPS_TAKEN flag?
>> and when it is reused as a journal head, LPROPS_TAKEN flag is set again.
> 
>>> If possible, can you please check the lprobs of the affected LEBs?
>> --------------------------------case 1----------------------------------------------
>> jhead[GCHD].wbuf.lnum   = 195(found in TNC)
>> jhead[DATAHD].wbuf.lnum  = 139(zbr passed to ubifs_tnc_read_node)
> 
>> struct ubifs_lprops =
>>
>>      free = 4096,
>>      dirty = 123272,
>>      flags = 16, //LPROPS_TAKEN
>>      lnum = 195,
>>      {
>>        list = {
>>          next = 0xcd1ad6d8,
>>          prev = 0xcd1ad440
>>        },
>>        hpos = -853879080
>>      }
>>    }
> 
>> struct ubifs_lprops = {
>>      free = 124944,
>>      dirty = 0,
>>      flags = 16, //LPROPS_TAKEN
>>      lnum = 139,
>>      {
>>        list = {
>>          next = 0xc8be96c0,
>>          prev = 0xc90e99c8
>>        },
>>        hpos = -927033664
>>      }
>>    }
> 
>> one more information about the ubifs_bud rbtree:
> 
>> crash_arm> tree -t rbtree -o ubifs_bud.rb -N 0xcd196dd4 -s ubifs_bud.lnum
>> cd196dc0
>>  lnum = 195//jhead[GCHD]
>> c89207c0
>>  lnum = 60
>> cd196fc0
>>  lnum = 59
>> c8bd5e40
>>  lnum = 125
>> c8935600
>>  lnum = 97
>> c2aafec0
>>  lnum = 139//jhead[DATAHD]
>> c8bb2140
>>  lnum = 279//jhead[BASEHD]
>> c8ee8d00
>>  lnum = 239
>> cd196980
>>  lnum = 348
>> --------------------------------------case 2----------------------------------------
>> jhead[GCHD].wbuf.lnum   = 224(found in TNC)
>> jhead[DATAHD].wbuf.lnum  = 54(zbr passed to ubifs_tnc_read_node)
> 
>> struct ubifs_lprops =
>> {
>>      free = 24576,
>>      dirty = 127640,
>>      flags = 16, //LPROPS_TAKEN
>>      lnum = 224,
>>      {
>>        list = {
>>          next = 0xc9014ec0,
>>          prev = 0xc91ed0c0
>>        },
>>        hpos = -922661184
>>      }
>> }
>> struct ubifs_lprops =
>> {
>>      free = 45952,
>>      dirty = 0,
>>      flags = 16, //LPROPS_TAKEN
>>      lnum = 54,
>>      {
>>        list = {
>>          next = 0xcd1d4328,
>>          prev = 0xc90ea9c8
>>        },
>>        hpos = -853720280
>>      }
>>    }
> 
>> crash_arm> tree -t rbtree -o ubifs_bud.rb -N 0xc6afae14 -s ubifs_bud.lnum
>> c6afae00
>>  lnum = 88
>> c8fa5540
>>  lnum = 53//jhead[BASEHD]
>> c916b500
>>  lnum = 42
>> c8f8b140
>>  lnum = 62
>> c2a953c0
>>  lnum = 54//jhead[DATAHD]
>> c916b480
>>  lnum = 215
>> c2ac0040
>>  lnum = 120
>> c916b400
>>  lnum = 224//jhead[GCHD]
>> c6b02f80
>>  lnum = 271
> 
>> 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
>>> --------------------------------------------------------------------------
> 
>      A      |              B
>  ---------------------------------------------------------------------------
>> ubifs_tnc_locate
>>   zbr->lnum = 54 (find in TNC)
>>                         ->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
>>          ubifs_tnc_read_node
>>             ubifs_get_wbuf(zbr->lnum = 54) is ture again
>>             ubifs_read_node_wbuf
>> ---------------------------------------------------------------------------
> 
> 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/
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/