linux-kernel - Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT

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: <mhng-08581a54-f9c8-4119-97bb-a9be86f19e41@palmerdabbelt-glaptop>
Date:   Thu, 06 May 2021 00:11:30 -0700 (PDT)
From:   Palmer Dabbelt <palmerdabbelt@...gle.com>
To:     rostedt@...dmis.org
CC:     Anup Patel <Anup.Patel@....com>, changbin.du@...il.com,
        linux-riscv@...ts.infradead.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        aou@...s.berkeley.edu, peterz@...radead.org, jpoimboe@...hat.com,
        jbaron@...mai.com, ardb@...nel.org,
        Atish Patra <Atish.Patra@....com>, akpm@...ux-foundation.org,
        rppt@...nel.org, mhiramat@...nel.org, zong.li@...ive.com,
        guoren@...ux.alibaba.com, wangkefeng.wang@...wei.com,
        0x7f454c46@...il.com, chenhuang5@...wei.com,
        linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject:     Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*

On Fri, 30 Apr 2021 12:44:37 PDT (-0700), Palmer Dabbelt wrote:
> On Fri, 30 Apr 2021 04:34:31 PDT (-0700), rostedt@...dmis.org wrote:
>> On Fri, 30 Apr 2021 04:06:35 +0000
>> Anup Patel <Anup.Patel@....com> wrote:
>>
>>> This patch only takes care of ftrace path.
>>>
>>> The RISC-V instruction patching is used by RISC-V jump label implementation
>>> as well and will called from various critical parts of core kernel.
>>>
>>> The RAW spinlock approach allows same instruction patching to be used
>>> for kprobes, ftrace, and jump label.
>>
>> So what path hits this outside of stop machine?
>
> I didn't actually dig through all the usages of jump_label, I just saw a
> handful in places where it's generally not sane to assume that sleeping
> is safe -- for example, thoughout kernel/sched.  If you think it's OK to
> rely on users of the static branch stuff (IIUC the only jump_label user
> in the kernel?) to know that it can sleep then I'm fine keeping the
> text_mutex call in jump_label and adding one to ftrace (I'm fine with
> something generic, but it's simple to do in arch/riscv).
>
> IMO if the static branch stuff can be expected to sleep it'd be good to
> call that out in the documentation, and I'd like to audit the uses
> before committing to that.  I'm happy to do that, we can just take the
> lock in arch/riscv's frace code for now to get around the lockdep
> assertion failure -- IIUC that's indicating a real bug, as nothing in
> ftrace avoids concurrency with jump_label and kprobes.

Turns out I'd actually opened the wrong thread and was looking at a 
stack trace from a bur reported a year ago, which is why nothing I was 
saying here makes any sense.  That bug has already been fixed, I have a 
proper fix for this one.  It turns out to be almost exactly the same as 
something Steven suggested in this thread.