Observed memory leak in hugetlb_reserve_pages

Observed memory leak in hugetlb_reserve_pages

[Lukas Bulwahn]

Dear Mike,

The reproducer for the 'memory leak in hugetlb_reserve_pages' bug (see
https://elisa-builder-00.iol.unh.edu/syzkaller-next/report?id=3469603f4a0da86b581cc979bd6c6663b46ceb1b)
is reproducible, it is triggering the memory leak on the current
mainline (commit 60bb8154d1d7), and it was not triggering on v6.0. My
build config is a x86_64 defconfig with some syzkaller-recommended
debug options.

I build the kernel with:

make O=$BUILD defconfig && make O=$BUILD kvm_guest.config && \
./scripts/kconfig/merge_config.sh -O $BUILD -r $BUILD/.config
kernel/configs/syzkaller-recommended.config && \
make O=$BUILD -j`nproc`

My syzkaller-recommended.config is:

CONFIG_KCOV=y
CONFIG_KCOV_INSTRUMENT_ALL=y
CONFIG_KCOV_ENABLE_COMPARISONS=y
CONFIG_DEBUG_FS=y
CONFIG_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_INFO=y
CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
CONFIG_KALLSYMS=y
CONFIG_KALLSYMS_ALL=y
CONFIG_CONFIGFS_FS=y
CONFIG_SECURITYFS=y
# CONFIG_RANDOMIZE_BASE is not set
CONFIG_KASAN=y
CONFIG_KASAN_INLINE=y
CONFIG_FAULT_INJECTION=y
CONFIG_FAULT_INJECTION_DEBUG_FS=y
CONFIG_FAULT_INJECTION_USERCOPY=y
CONFIG_FAILSLAB=y
CONFIG_FAIL_PAGE_ALLOC=y
CONFIG_FAIL_MAKE_REQUEST=y
CONFIG_FAIL_IO_TIMEOUT=y
CONFIG_FAIL_FUTEX=y
CONFIG_LOCKDEP=y
CONFIG_PROVE_LOCKING=y
CONFIG_DEBUG_ATOMIC_SLEEP=y
CONFIG_PROVE_RCU=y
CONFIG_DEBUG_VM=y
CONFIG_FORTIFY_SOURCE=y
CONFIG_HARDENED_USERCOPY=y
CONFIG_LOCKUP_DETECTOR=y
CONFIG_SOFTLOCKUP_DETECTOR=y
CONFIG_HARDLOCKUP_DETECTOR=y
CONFIG_BOOTPARAM_HARDLOCKUP_PANIC=y
CONFIG_DETECT_HUNG_TASK=y
CONFIG_WQ_WATCHDOG=y
CONFIG_DEFAULT_HUNG_TASK_TIMEOUT=140
CONFIG_RCU_CPU_STALL_TIMEOUT=100


My git bisection showed that the first bad commit is
[bbff39cc6cbcb86ccfacb2dcafc79912a9f9df69] hugetlb: allocate vma lock
for all sharable vmas.


Full git bisect report:

git bisect start
# bad: [60bb8154d1d77042a5d43d335a68fdb202302cbe] Merge tag
'xfs-6.1-for-linus' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
git bisect bad 60bb8154d1d77042a5d43d335a68fdb202302cbe
# good: [4fe89d07dcc2804c8b562f6c7896a45643d34b2f] Linux 6.0
git bisect good 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
# good: [ff6862c23d2e83d12d1759bf4337d41248fb4dc8] Merge tag
'arm-drivers-6.1' of
git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
git bisect good ff6862c23d2e83d12d1759bf4337d41248fb4dc8
# good: [e8bc52cb8df80c31c73c726ab58ea9746e9ff734] Merge tag
'driver-core-6.1-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
git bisect good e8bc52cb8df80c31c73c726ab58ea9746e9ff734
# good: [4899a36f91a9f9b06878471096bd143e7253006d] Merge tag
'powerpc-6.1-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect good 4899a36f91a9f9b06878471096bd143e7253006d
# good: [0e0073eb1b60f4ec6faecea034a6772fe1409a88] Merge tag
'hyperv-next-signed-20221009' of
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux
git bisect good 0e0073eb1b60f4ec6faecea034a6772fe1409a88
# good: [2f3568017268fc34eb0b6b4b3163c0f2e619fde6] kasan: move
kasan_get_*_meta to generic.c
git bisect good 2f3568017268fc34eb0b6b4b3163c0f2e619fde6
# good: [a57ae9ef9e1a20b68ae841a8cab7aff3f000ed9d] mm/page_alloc:
update comments for rmqueue()
git bisect good a57ae9ef9e1a20b68ae841a8cab7aff3f000ed9d
# bad: [dc914858561c424978307561492fcf3145b8b525] Merge tag
'nfsd-6.1-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux
git bisect bad dc914858561c424978307561492fcf3145b8b525
# good: [70442fc54e6889a2a77f0e9554e8188a1557f00e] Merge tag
'x86_mm_for_v6.1_rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 70442fc54e6889a2a77f0e9554e8188a1557f00e
# good: [c91bdc9358992856721ff77887202a7e80b7ab22] mm: memcontrol:
don't allocate cgroup swap arrays when memcg is disabled
git bisect good c91bdc9358992856721ff77887202a7e80b7ab22
# good: [7d37539037c2fca70346fbedc219f655253d5cff] fuse: implement ->tmpfile()
git bisect good 7d37539037c2fca70346fbedc219f655253d5cff
# good: [131a79b474e973f023c5c75e2323a940332103be] hugetlb: fix vma
lock handling during split vma and range unmapping
git bisect good 131a79b474e973f023c5c75e2323a940332103be
# bad: [27bc50fc90647bbf7b734c3fc306a5e61350da53] Merge tag
'mm-stable-2022-10-08' of
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
git bisect bad 27bc50fc90647bbf7b734c3fc306a5e61350da53
# bad: [bbff39cc6cbcb86ccfacb2dcafc79912a9f9df69] hugetlb: allocate
vma lock for all sharable vmas
git bisect bad bbff39cc6cbcb86ccfacb2dcafc79912a9f9df69
# good: [ecfbd733878da48ed03a5b8a9c301366a03e3cca] hugetlb: take
hugetlb vma_lock when clearing vma_lock->vma pointer
git bisect good ecfbd733878da48ed03a5b8a9c301366a03e3cca
# first bad commit: [bbff39cc6cbcb86ccfacb2dcafc79912a9f9df69]
hugetlb: allocate vma lock for all sharable vmas


If there is more information needed, please let me know.

Best regards,

Lukas

Re: Observed memory leak in hugetlb_reserve_pages

[Mike Kravetz]

On 10/17/22 13:11, Lukas Bulwahn wrote:
> Dear Mike,
> 
> The reproducer for the 'memory leak in hugetlb_reserve_pages' bug (see
> https://elisa-builder-00.iol.unh.edu/syzkaller-next/report?id=3469603f4a0da86b581cc979bd6c6663b46ceb1b)
> is reproducible, it is triggering the memory leak on the current
> mainline (commit 60bb8154d1d7), and it was not triggering on v6.0. My
> build config is a x86_64 defconfig with some syzkaller-recommended
> debug options.

Thank you Lukas!

The leak is embarrassingly obvious.  Here is a bit of code at the beginning of
hugetlb_reserve_pages:

	/*
	 * vma specific semaphore used for pmd sharing synchronization
	 */
	hugetlb_vma_lock_alloc(vma);

	/*
	 * Only apply hugepage reservation if asked. At fault time, an
	 * attempt will be made for VM_NORESERVE to allocate a page
	 * without using reserves
	 */
	if (vm_flags & VM_NORESERVE)
		return true;

There needs to be a hugetlb_vma_lock_free(vma) call before that return.

I will do some testing and send a patch.  However, I will use commit
8d9bfb2608145 in the Fixes: tag.  This is because that commit added the
call to hugetlb_vma_lock_alloc in hugetlb_reserve_pages without the
hugetlb_vma_lock_free in the VM_NORESERVE return.

> My git bisection showed that the first bad commit is
> [bbff39cc6cbcb86ccfacb2dcafc79912a9f9df69] hugetlb: allocate vma lock
> for all sharable vmas.

The reason your bisect pointed to bbff39cc6cbc is because the mmap/vma size in
the test case is not sufficient for pmd sharing.  Therefore, the test did not
experience a leak until the patch which allocates the vma lock for ALL vmas
was added.
-- 
Mike Kravetz

Re: Observed memory leak in hugetlb_reserve_pages

[Mike Kravetz]

On 10/17/22 10:38, Mike Kravetz wrote:
> On 10/17/22 13:11, Lukas Bulwahn wrote:
> > Dear Mike,
> > 
> > The reproducer for the 'memory leak in hugetlb_reserve_pages' bug (see
> > https://elisa-builder-00.iol.unh.edu/syzkaller-next/report?id=3469603f4a0da86b581cc979bd6c6663b46ceb1b)
> > is reproducible, it is triggering the memory leak on the current
> > mainline (commit 60bb8154d1d7), and it was not triggering on v6.0. My
> > build config is a x86_64 defconfig with some syzkaller-recommended
> > debug options.
> 
> Thank you Lukas!
> 
> The leak is embarrassingly obvious.  Here is a bit of code at the beginning of
> hugetlb_reserve_pages:
> 
> 	/*
> 	 * vma specific semaphore used for pmd sharing synchronization
> 	 */
> 	hugetlb_vma_lock_alloc(vma);
> 
> 	/*
> 	 * Only apply hugepage reservation if asked. At fault time, an
> 	 * attempt will be made for VM_NORESERVE to allocate a page
> 	 * without using reserves
> 	 */
> 	if (vm_flags & VM_NORESERVE)
> 		return true;
> 
> There needs to be a hugetlb_vma_lock_free(vma) call before that return.
> 
> I will do some testing and send a patch.  However, I will use commit
> 8d9bfb2608145 in the Fixes: tag.  This is because that commit added the
> call to hugetlb_vma_lock_alloc in hugetlb_reserve_pages without the
> hugetlb_vma_lock_free in the VM_NORESERVE return.
> 
> > My git bisection showed that the first bad commit is
> > [bbff39cc6cbcb86ccfacb2dcafc79912a9f9df69] hugetlb: allocate vma lock
> > for all sharable vmas.
> 
> The reason your bisect pointed to bbff39cc6cbc is because the mmap/vma size in
> the test case is not sufficient for pmd sharing.  Therefore, the test did not
> experience a leak until the patch which allocates the vma lock for ALL vmas
> was added.

Please ignore this email.  I sent it before looking (or thinking) closely.

-- 
Mike Kravetz