linux-kernel - Re: [PATCH v2] MIPS: Fix MAX_REG_OFFSET and remove zero-length struct member

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: <EC98BAE8-8269-4169-B3A2-5F426E77C223@linux.dev>
Date: Fri, 18 Apr 2025 13:05:49 +0200
From: Thorsten Blum <thorsten.blum@...ux.dev>
To: "Maciej W. Rozycki" <macro@...am.me.uk>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 Oleg Nesterov <oleg@...hat.com>,
 linux-mips@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] MIPS: Fix MAX_REG_OFFSET and remove zero-length struct
 member

On 18. Apr 2025, at 12:36, Maciej W. Rozycki wrote:
> On Fri, 18 Apr 2025, Thorsten Blum wrote:
>>>> Remove the unnecessary zero-length struct member '__last' and fix
>>>> MAX_REG_OFFSET to point to the last register in 'pt_regs'.
>>>> 
>>>> Fixes: 40e084a506eba ("MIPS: Add uprobes support.")
>>> 
>>> what does it fix ?
>> 
>> The value of MAX_REG_OFFSET and thus how regs_get_register() behaves.
>> 
>> From my understanding, MAX_REG_OFFSET points to the marker '__last[0]'
>> instead of the actual last register in 'pt_regs', which could allow
>> regs_get_register() to return an invalid offset.
> 
> Or actually it permits an out-of-range access beyond the end of `struct 
> pt_regs': if you call `regs_get_register(pt_regs, MAX_REG_OFFSET)', it'll 
> read memory beyond `pt_regs' rather than returning 0 right away.  It may 
> not happen in reality (I haven't checked), but it's a QoI issue we should 
> address IMO.  Other platforms that I've checked (riscv, x86) get it right.
> 
> Though the fix is incorrect for CPU_CAVIUM_OCTEON, because it doesn't 
> allow one to access the second half of the last register, and I find it 
> exceedingly complex anyway.  Just:
> 
> #define MAX_REG_OFFSET \
> (offsetof(struct pt_regs, __last) - sizeof(unsigned long))
> 
> will do (as `regs_get_register' operates on `unsigned long' quantities).

Does regs_get_register() even work for CPU_CAVIUM_OCTEON when accessing
the last two registers because they're both ULL, not UL? (independent of
my patch)

Thorsten