linux-kernel - Re: [PATCH] arm64/hw_breakpoint: Determine lengths from generic perf breakpoint macros

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: <1901fadb-1d71-4374-be8c-00935bb27854@arm.com>
Date: Mon, 26 Feb 2024 08:19:39 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Will Deacon <will@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org, broonie@...nel.org,
 Mark Rutland <mark.rutland@....com>,
 Catalin Marinas <catalin.marinas@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64/hw_breakpoint: Determine lengths from generic perf
 breakpoint macros



On 2/23/24 18:22, Will Deacon wrote:
> On Fri, Feb 23, 2024 at 05:01:02PM +0530, Anshuman Khandual wrote:
>> Both platform i.e ARM_BREAKPOINT_LEN_X and generic i.e HW_BREAKPOINT_LEN_X
>> macros are used interchangeably to convert event->attr.bp_len and platform
>> breakpoint control arch_hw_breakpoint_ctrl->len. Let's be consistent while
>> deriving one from the other. This does not cause any functional changes.
>>
>> Cc: Will Deacon <will@...nel.org>
>> Cc: Mark Rutland <mark.rutland@....com>
>> Cc: Catalin Marinas <catalin.marinas@....com>
>> Cc: linux-arm-kernel@...ts.infradead.org
>> Cc: linux-kernel@...r.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
>> ---
>> This applies on v6.8-rc5
>>
>>  arch/arm64/kernel/hw_breakpoint.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
>> index 35225632d70a..1ab9fc865ddd 100644
>> --- a/arch/arm64/kernel/hw_breakpoint.c
>> +++ b/arch/arm64/kernel/hw_breakpoint.c
>> @@ -301,28 +301,28 @@ static int get_hbp_len(u8 hbp_len)
>>  
>>  	switch (hbp_len) {
>>  	case ARM_BREAKPOINT_LEN_1:
>> -		len_in_bytes = 1;
>> +		len_in_bytes = HW_BREAKPOINT_LEN_1;
> 
> I don't think we should do this. The HW_BREAKPOINT_LEN_* definitions are
> part of the user ABI and, although they correspond to the length in bytes,
> that's not necessarily something we should rely on.

Why should not we rely on the user ABI macros if these byte lengths were
initially derived from them. But also there are similar conversions in
arch_bp_generic_fields(). These hard coded raw byte length numbers seems
cryptic, where as in reality these are just inter converted from generic
HW breakpoints lengths.