linux-kernel - Re: [PATCH] ipc/sem: do not sleep with a spin lock held

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]

Date:   Wed, 22 Dec 2021 18:31:00 +0300
From:   Vasily Averin <vvs@...tuozzo.com>
To:     Manfred Spraul <manfred@...orfullife.com>, cgel.zte@...il.com,
        akpm@...ux-foundation.org
Cc:     shakeelb@...gle.com, rdunlap@...radead.org, dbueso@...e.de,
        unixbhaskar@...il.com, chi.minghao@....com.cn, arnd@...db.de,
        linux-kernel@...r.kernel.org, Zeal Robot <zealci@....com.cn>
Subject: Re: [PATCH] ipc/sem: do not sleep with a spin lock held

On 22.12.2021 14:45, Manfred Spraul wrote:
> Hi Minghao,
> 
> On 12/22/21 09:10, cgel.zte@...il.com wrote:
>> From: Minghao Chi <chi.minghao@....com.cn>
>>
>> We can't call kvfree() with a spin lock held, so defer it.

I'm sorry, but I do not understand why exactly we cannot use kvfree?
Could you explain it in more details?

>> Reported-by: Zeal Robot <zealci@....com.cn>
>> Signed-off-by: Minghao Chi <chi.minghao@....com.cn>
> 
> Could you add
> 
> Fixes: fc37a3b8b438 ("[PATCH] ipc sem: use kvmalloc for sem_undo allocation")
> 
> Cc: stable@...r.kernel.org
> 
> I will review/test the change in the next few days.
> 
> Especially, I would like to check if there are further instances where the same mistake was made.
> 
>> /**
>> * kvfree() - Free memory.
>> * @addr: Pointer to allocated memory.
>> *
>> * kvfree frees memory allocated by any of vmalloc(), kmalloc() or kvmalloc().
>> * It is slightly more efficient to use kfree() or vfree() if you are certain
>> * that you know which one to use.
>> *
>> * Context: Either preemptible task context or not-NMI interrupt.
>> */
>>
> As an independent change: Should we add a
> 
> 
>       might_sleep_if(!in_interrupt());
> 
> into kvfree(), to trigger bugs more easily?

I think it is good idea in general, 
however please do not use "in_interrupt()", it is obsoleted 
and in fact means "We're in NMI,IRQ,SoftIRQ context or have BH disabled"

Please use something like in_task()

Thank you,	Vasily Averin