Re: [PATCH 5/8] fs/ntfs3: Use kvmalloc instead of kmalloc(... __GFP_NOWARN)

[Tetsuo Handa]

On 2023/12/23 13:46, Matthew Wilcox wrote:
> On Mon, Jul 03, 2023 at 11:26:36AM +0400, Konstantin Komarov wrote:
>>
>> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
> 
> So, um, why?  I mean, I know what kvmalloc does, but why do you want to
> do it?  Also, this is missing changes from kfree() to kvfree() so if
> you do end up falling back to vmalloc, you'll hit a bug in kfree().

Right. The first thing we should do is to revert commit fc471e39e38f("fs/ntfs3:
Use kvmalloc instead of kmalloc(... __GFP_NOWARN)"), for that commit is horrible.

> 
>> +++ b/fs/ntfs3/attrlist.c
>> @@ -52,7 +52,8 @@ int ntfs_load_attr_list(struct ntfs_inode *ni, struct
>> ATTRIB *attr)
>>
>>      if (!attr->non_res) {
>>          lsize = le32_to_cpu(attr->res.data_size);
>> -        le = kmalloc(al_aligned(lsize), GFP_NOFS | __GFP_NOWARN);
>> +        /* attr is resident: lsize < record_size (1K or 4K) */
>> +        le = kvmalloc(al_aligned(lsize), GFP_KERNEL);

syzbot is reporting that

  /* attr is resident: lsize < record_size (1K or 4K) */

was false at https://syzkaller.appspot.com/bug?extid=f987ceaddc6bcc334cde .
Maybe you can fix this report by adding more validations. Please be
sure to take into account that the filesystem image is corrupted/crafted.

But you can't replace GFP_NOFS with GFP_KERNEL anyway, for syzbot is also
reporting GFP_KERNEL allocation with filesystem lock held
at https://syzkaller.appspot.com/bug?extid=18f543fc90dd1194c616 .

By the way, you might want to add __GFP_RETRY_MAYFAIL when you can't use
GFP_KERNEL, for GFP_NOFS memory allocation request cannot trigger OOM
killer in order to allocate requested amount of memory, leading to
possibility of out-of-memory deadlock. Especially when there is possibility
that kvmalloc() needs to allocate so many pages...
See https://elixir.bootlin.com/linux/v6.7-rc6/source/mm/page_alloc.c#L3458 .