linux-kernel - Re: [PATCH 3/4] mm/hugeltb: fix potential wrong gbl_reserve value for hugetlb_acct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1311fcfe-bc5f-e878-3912-ca9a9e0eed90@huawei.com>
Date:   Wed, 7 Apr 2021 15:24:39 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>, <akpm@...ux-foundation.org>
CC:     <n-horiguchi@...jp.nec.com>, <linux-kernel@...r.kernel.org>,
        <linux-mm@...ck.org>
Subject: Re: [PATCH 3/4] mm/hugeltb: fix potential wrong gbl_reserve value for
 hugetlb_acct_memory()

Hi:
On 2021/4/7 10:49, Mike Kravetz wrote:
> On 4/2/21 2:32 AM, Miaohe Lin wrote:
>> The resv_map could be NULL since this routine can be called in the evict
>> inode path for all hugetlbfs inodes. So we could have chg = 0 and this
>> would result in a negative value when chg - freed. This is unexpected for
>> hugepage_subpool_put_pages() and hugetlb_acct_memory().
> 
> I am not sure if this is possible.
> 
> It is true that resv_map could be NULL.  However, I believe resv map
> can only be NULL for inodes that are not regular or link inodes.  This
> is the inode creation code in hugetlbfs_get_inode().
> 
>        /*
>          * Reserve maps are only needed for inodes that can have associated
>          * page allocations.
>          */
>         if (S_ISREG(mode) || S_ISLNK(mode)) {
>                 resv_map = resv_map_alloc();
>                 if (!resv_map)
>                         return NULL;
>         }
> 

Agree.

> If resv_map is NULL, then no hugetlb pages can be allocated/associated
> with the file.  As a result, remove_inode_hugepages will never find any
> huge pages associated with the inode and the passed value 'freed' will
> always be zero.
> 

But I am confused now. AFAICS, remove_inode_hugepages() searches the address_space of
the inode to remove the hugepages while does not care if inode has associated resv_map.
How does it prevent hugetlb pages from being allocated/associated with the file if
resv_map is NULL? Could you please explain this more?

Many thanks.

> Does that sound correct?
>