linux-kernel - Re: [PATCH 1/3] arm64: kprobes: Move extable address check into arch_prepare

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]

Message-ID: <92c160a8-7627-0c64-ed73-df616e9c057d@arm.com>
Date:   Fri, 11 Jan 2019 18:22:38 +0000
From:   James Morse <james.morse@....com>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Pratyush Anand <panand@...hat.com>,
        "David A . Long" <dave.long@...aro.org>,
        linux-arm-kernel@...ts.infradead.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] arm64: kprobes: Move extable address check into
 arch_prepare_kprobe()

Hi,

On 09/01/2019 02:05, Masami Hiramatsu wrote:
> On Tue, 8 Jan 2019 17:13:36 +0000
> James Morse <james.morse@....com> wrote:
>> On 08/01/2019 02:39, Masami Hiramatsu wrote:
>>> On Thu, 3 Jan 2019 17:05:18 +0000
>>> James Morse <james.morse@....com> wrote:
>>>> On 17/12/2018 06:40, Masami Hiramatsu wrote:
>>>>> Move extable address check into arch_prepare_kprobe() from
>>>>> arch_within_kprobe_blacklist().
>>>>
>>>> I'm trying to work out the pattern for what should go in the blacklist, and what
>>>> should be rejected by the arch code.
>>>>
>>>> It seems address-ranges should be blacklisted as the contents don't matter.
>>>> easy-example: the idmap text.
>>>
>>> Yes, more precisely, the code smaller than a function (symbol), it must be
>>> rejected by arch_prepare_kprobe(), since blacklist is poplated based on
>>> kallsyms.
>>
>> Ah, okay, so the pattern is the blacklist should only be for whole symbols,
>> (which explains why its usually based on sections).
> 
> Correct. Actually, the blacklist is generated based on the symbol info
> from symbol address.
> 
>> I see kprobe_add_ksym_blacklist() would go wrong if you give it something like:
>> platform_drv_probe+0x50/0xb0, as it will log platform_drv_probe+0x50 as the
>> start_addr and platform_drv_probe+0x50+0xb0 as the end.
> 
> Yes, it expects given address is the entry of a symbol.

>> But how does anything from the arch code's blacklist get into the
>> kprobe_blacklist list?
> 
> It should be done via arch_populate_kprobe_blacklist().

>> We don't have an arch_populate_kprobe_blacklist(), so rely on
>> within_kprobe_blacklist() calling arch_within_kprobe_blacklist() with the
>> address, as well as walking kprobe_blacklist.
>>
>> Is this cleanup ahead of a series that does away with
>> arch_within_kprobe_blacklist() so that debugfs list is always complete?
> 
> Right, after this cleanup, I will send arch_populate_kprobe_blacklist()
> patch for arm64 and others. My plan is to move all arch_within_kprobe_blacklist()
> to arch_populate_kprobe_blacklist() so that user can get more precise blacklist
> via debugfs.

Thanks, now it all makes sense!

Reviewed-by: James Morse <james.morse@....com>


Could you include a paragraph like that in the cover-letter or commit-message?
The 'fix' in the cover-letter subject had me looking for the bug!


Thanks,

James