| 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
| ||
|
Message-ID: <8e773a7c-e1dd-4e0e-8349-8aa52f39d85a@os.amperecomputing.com> Date: Fri, 10 Jan 2025 10:04:42 -0800 From: Yang Shi <yang@...amperecomputing.com> To: Matthew Wilcox <willy@...radead.org> Cc: Liu Shixin <liushixin2@...wei.com>, Andrew Morton <akpm@...ux-foundation.org>, Chengming Zhou <chengming.zhou@...ux.dev>, Kefeng Wang <wangkefeng.wang@...wei.com>, Nanyong Sun <sunnanyong@...wei.com>, Muchun Song <muchun.song@...ux.dev>, Qi Zheng <zhengqi.arch@...edance.com>, Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma On 1/9/25 8:31 PM, Matthew Wilcox wrote: > On Thu, Jan 09, 2025 at 09:00:24AM -0800, Yang Shi wrote: >> Thanks for catching this. It sounds a little bit weird to have vm_file for >> an anonymous VMA. I'm not sure why we should keep such special case. It >> seems shared mapping is treated as shmem file mapping. So can we set vm_file >> to NULL when mmap'ing /dev/zero for private mapping? Something like: >> >> diff --git a/drivers/char/mem.c b/drivers/char/mem.c >> index 169eed162a7f..fc332efc5c11 100644 >> --- a/drivers/char/mem.c >> +++ b/drivers/char/mem.c >> @@ -527,6 +527,7 @@ static int mmap_zero(struct file *file, struct >> vm_area_struct *vma) >> if (vma->vm_flags & VM_SHARED) >> return shmem_zero_setup(vma); >> vma_set_anonymous(vma); >> + vma->vm_file = NULL; >> return 0; >> } > I'm wary this might cause other bugs somewhere. rc6 is a bit late to be > introducing such a subtle change. Thanks for the extra caution. Applying the proposed fix in khugepaged code is fine to me either. We can try to kill the special case later. Looking at the code further, I think we should do more to make private /dev/zero mapping an anonymous mapping: diff --git a/drivers/char/mem.c b/drivers/char/mem.c index 169eed162a7f..98cfac2bb01f 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -527,6 +527,10 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma) if (vma->vm_flags & VM_SHARED) return shmem_zero_setup(vma); vma_set_anonymous(vma); + fput(vma->vm_file); + vma->vm_file = NULL; + vma->vm_pgoff = vma->vm_start >> PAGE_SHIFT; + return 0; } AFAICT, the user visible effect is we will have different entry in smaps/maps. Before the change: ffffb7190000-ffffb7590000 rw-p 00001000 00:06 8 /dev/zero Size: 4096 kB KernelPageSize: 4 kB MMUPageSize: 4 kB Rss: 4 kB Pss: 4 kB Pss_Dirty: 4 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 4 kB Referenced: 4 kB Anonymous: 4 kB KSM: 0 kB LazyFree: 0 kB AnonHugePages: 0 kB ShmemPmdMapped: 0 kB FilePmdMapped: 0 kB Shared_Hugetlb: 0 kB Private_Hugetlb: 0 kB Swap: 0 kB SwapPss: 0 kB Locked: 0 kB THPeligible: 0 VmFlags: rd wr mr mw me ac After the change: ffffb6130000-ffffb6530000 rw-p 00000000 00:00 0 Size: 4096 kB KernelPageSize: 4 kB MMUPageSize: 4 kB Rss: 4 kB Pss: 4 kB Pss_Dirty: 4 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 4 kB Referenced: 4 kB Anonymous: 4 kB KSM: 0 kB LazyFree: 0 kB AnonHugePages: 0 kB ShmemPmdMapped: 0 kB FilePmdMapped: 0 kB Shared_Hugetlb: 0 kB Private_Hugetlb: 0 kB Swap: 0 kB SwapPss: 0 kB Locked: 0 kB THPeligible: 0 VmFlags: rd wr mr mw me ac I'm not sure who really cares about the difference.
Powered by blists - more mailing lists