[PATCH v3 2/2] ubifs: read node from wbuf when it fully sits in wbuf

[Hou Tao]

Hi,

On 2020/3/17 6:21, Richard Weinberger wrote:
> Hou Tao,
> 
> On Thu, Mar 5, 2020 at 10:15 AM Hou Tao <houtao1 at huawei.com> wrote:
>>
>> Carson Li Reports the following error:
>>
>>  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 an invalid node. 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() reads
>> an invalid 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
>>
>> We can simply fix the problem by removing the optimization of reading
>> wbuf when possible. But because taking spin lock and memcpying from
>> wbuf is much less time-consuming than reading from MTD device, so we fix
>> the logic of wbuf reading instead.
> 
> I'm digging now into that issue. Did you also experiment with reading while
> tnc_mutex is locked? So, no race at all (having safely = 1 by default).
> Just to make sure we don't fix an no longer needed optimization.
>> The code is already anything but trivial and adding more code makes me
> nervous.
> 
Sorry for the late reply. I'm fine if we remove the read-wbuf optimization,
but I need to check the code firstly, running the reproducing program, and lastly
writing a xfstest for it.

Regards,
Tao