linux-kernel - Re: [PATCH v3 1/1] kasan: Avoid sleepable page allocation from atomic context

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] [day] [month] [year] [list]

Message-ID: <aBomoDkNgiEAJjgX@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com>
Date: Tue, 6 May 2025 17:11:28 +0200
From: Alexander Gordeev <agordeev@...ux.ibm.com>
To: Andrey Ryabinin <ryabinin.a.a@...il.com>
Cc: Harry Yoo <harry.yoo@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Daniel Axtens <dja@...ens.net>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, kasan-dev@...glegroups.com,
        linux-s390@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v3 1/1] kasan: Avoid sleepable page allocation from
 atomic context

On Tue, May 06, 2025 at 04:55:20PM +0200, Andrey Ryabinin wrote:
> >>> -	if (likely(pte_none(ptep_get(ptep)))) {
> >>> +	if (likely(pte_none(ptep_get(ptep))))
> >>>  		set_pte_at(&init_mm, addr, ptep, pte);
> >>> -		page = 0;
> >>
> >> With this patch, now if the pte is already set, the page is leaked?
> > 
> > Yes. But currently it is leaked for previously allocated pages anyway,
> > so no change in behaviour (unless I misread the code).
> 
> Current code doesn't even allocate page if pte set, and if set pte discovered only after
> taking spinlock, the page will be freed, not leaked.

Oh, right. I rather meant pages that are leaked in case of a failure. My bad.

> Whereas, this patch leaks page for every single !pte_none case. This will build up over time
> as long as vmalloc called.
> 
> > 
> >> Should we set data->pages[PFN_DOWN(addr - data->start)] = NULL 
> >> and free non-null elements later in __kasan_populate_vmalloc()?
> > 
> > Should the allocation fail on boot, the kernel would not fly anyway.
> 
> This is not boot code, it's called from vmalloc() code path.

FWIW, it is called from rest_init() too.

> > If for whatever reason we want to free, that should be a follow-up
> > change, as far as I am concerned.
> > 
> We want to free it, because we don't want unbound memory leak.

Will send v5.

Thanks!