[PATCH 0/3] fixes for ubifs xattr operations

[Hou Tao]

Hi,

On 2020/6/30 21:15, Richard Weinberger wrote:
> Tao,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Hou Tao" <houtao1 at huawei.com>
>> An: "richard" <richard at nod.at>, "linux-mtd" <linux-mtd at lists.infradead.org>
>> CC: "Hou Tao" <houtao1 at huawei.com>
>> Gesendet: Dienstag, 30. Juni 2020 15:04:35
>> Betreff: [PATCH 0/3] fixes for ubifs xattr operations
> 
>> Hi,
>>
>> The patch set tries to fix the race between ubifs xattr read
>> operations and write operations.
>>
>> Now inode_lock() is acquired during xattr write ops (set & remove),
>> however no extra lock is taken in xattr read ops (list & get), and
>> it leads to three problems:
>>
>> (1) ubifs_listxattr() may incur memory corruption and assertion failure
>>
>> (2) ubifs_xattr_get() may incur assertion failure
>>
>> (3) ubifs_xattr_get() may return a stale xattr value
>>
>> To fix it, instead of adding a xattr-related rwsem for ubifs inode,
>> we decide to fix these problems simply and still do xattr read operation
>> locklessly. The major concern is that xattr read operations (list & get)
>> may return xattr names or values which is still in creation or removal
>> process, but we think that's OK because no stale name or value is
>> returned, just either the old ones or the new ones.
>>
>> Comments are weclome.
> 
> Thanks a lot for digging into this.
> Do you have a test for this? (xfstest prefered).
> 
The first two problem can be reproduced relatively easily, and the third problem
is hard to reproduce (I do it through injecting delay in xattr ops), so I will
add xfstests for the first two problem firstly, then I will try to add an xfstests
for the last one. Maybe we can add a thin "infrastructure" layer to inject delay.

> I'm a bit surprised that this can actually happen, I was under the impression
> that vfs cares about this.
> 
Most filesystems handle the race by adding a rw-semaphore, but I think we can
simply do in a "lockless" way.

Regards,
Tao

> Thanks,
> //richard
> .
>