linux-kernel - Re: [mm] 408579cd62: WARNING:suspicious_RCU

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:   Mon, 3 Jul 2023 19:29:48 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Oliver Sang <oliver.sang@...el.com>
Cc:     "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        LKML <linux-kernel@...r.kernel.org>, lkp@...ts.01.org,
        lkp@...el.com
Subject: Re: [mm] 408579cd62: WARNING:suspicious_RCU_usage

On Mon, 3 Jul 2023 at 18:48, Oliver Sang <oliver.sang@...el.com> wrote:
>
> by patch [1], we found the warning is not fixed.

Hmm. I already committed that "fix" as obvious, since the main
difference in commit 408579cd627a ("mm: Update do_vmi_align_munmap()
return semantics") around that validate_mm() call was how it did that
mmap_read_unlock().

> we also found there are some changes in stack backtrace. now it's as below:
> (detail dmesg is attached)
>
> [   26.412372][    T1] stack backtrace:
> [   26.412846][    T1] CPU: 0 PID: 1 Comm: systemd Not tainted 6.4.0-09908-gcb226fb1fb7a #1
> [   26.413506][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [   26.414326][    T1] Call Trace:
> [   26.414605][    T1]  <TASK>
> [   26.414847][    T1]  dump_stack_lvl+0x73/0xc0
> [   26.415225][    T1]  lockdep_rcu_suspicious+0x1b7/0x280
> [   26.415669][    T1]  mas_start+0x280/0x400
> [   26.416037][    T1]  mas_find+0x27a/0x400
> [   26.416391][    T1]  validate_mm+0x8b/0x2c0
> [   26.416757][    T1]  __se_sys_brk+0xa35/0xc00

Ok, that is indeed a very different stack trace.

So maybe the fix is a real fix, but the first complaint shut up
lockdep, so this is the *second* and unrelated complaint.

And indeed: it turns out that do_vma_munmap() does this:

        ret = do_vmi_align_munmap(vmi, vma, mm, start, end, uf, unlock);
        validate_mm(mm);

and so we have *another* validate_mm() that is now done outside the lock.

That one is actually pretty pointless. We've *just* validated the mm
already inside do_vmi_align_munmap(), except we only did it in one of
the two return cases.

So I think the fix is to just move that validate_mm() into the other
return case of do_vmi_align_munmap(), and remove it from the caller.

IOW, something like the attached (NOTE! This is in *addition* to the
previous patch, which is the same as the one you quoted, just with
slightly different whitespace as commit ae80b4041984: "mm: validate
the mm before dropping the mmap lock").

               Linus

View attachment "patch.diff" of type "text/x-patch" (756 bytes)