[PATCH v3] MIPS: Fix MAX_REG_OFFSET

[PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Thorsten Blum]

Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
the marker itself, which could allow regs_get_register() to return an
invalid offset.

Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Compile-tested only.

Changes in v2:
- Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)
- Link to v1: https://lore.kernel.org/lkml/20250411090032.7844-1-thorsten.blum@linux.dev/

Changes in v3:
- Keep the marker and avoid using #ifdef by adjusting MAX_REG_OFFSET as
  suggested by Thomas and Maciej
- Link to v2: https://lore.kernel.org/lkml/20250417174712.69292-2-thorsten.blum@linux.dev/
---
 arch/mips/include/asm/ptrace.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
index 85fa9962266a..ef72c46b5568 100644
--- a/arch/mips/include/asm/ptrace.h
+++ b/arch/mips/include/asm/ptrace.h
@@ -65,7 +65,8 @@ static inline void instruction_pointer_set(struct pt_regs *regs,
 
 /* Query offset/name of register from its name/offset */
 extern int regs_query_register_offset(const char *name);
-#define MAX_REG_OFFSET (offsetof(struct pt_regs, __last))
+#define MAX_REG_OFFSET \
+	(offsetof(struct pt_regs, __last) - sizeof(unsigned long))
 
 /**
  * regs_get_register() - get register value from its offset
-- 
2.49.0

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Huacai Chen]

Hi, Thorsten,

On Sun, Apr 27, 2025 at 7:35 PM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
> the marker itself, which could allow regs_get_register() to return an
> invalid offset.
>
> Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Compile-tested only.
>
> Changes in v2:
> - Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)
> - Link to v1: https://lore.kernel.org/lkml/20250411090032.7844-1-thorsten.blum@linux.dev/
>
> Changes in v3:
> - Keep the marker and avoid using #ifdef by adjusting MAX_REG_OFFSET as
>   suggested by Thomas and Maciej
> - Link to v2: https://lore.kernel.org/lkml/20250417174712.69292-2-thorsten.blum@linux.dev/
> ---
>  arch/mips/include/asm/ptrace.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> index 85fa9962266a..ef72c46b5568 100644
> --- a/arch/mips/include/asm/ptrace.h
> +++ b/arch/mips/include/asm/ptrace.h
> @@ -65,7 +65,8 @@ static inline void instruction_pointer_set(struct pt_regs *regs,
>
>  /* Query offset/name of register from its name/offset */
>  extern int regs_query_register_offset(const char *name);
> -#define MAX_REG_OFFSET (offsetof(struct pt_regs, __last))
> +#define MAX_REG_OFFSET \
> +       (offsetof(struct pt_regs, __last) - sizeof(unsigned long))
There is no 80 columns limit now, so no new line needed here.

Huacai

>
>  /**
>   * regs_get_register() - get register value from its offset
> --
> 2.49.0
>
>

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Thomas Bogendoerfer]

On Sun, Apr 27, 2025 at 08:32:05PM +0800, Huacai Chen wrote:
> Hi, Thorsten,
> 
> On Sun, Apr 27, 2025 at 7:35 PM Thorsten Blum <thorsten.blum@linux.dev> wrote:
> >
> > Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
> > the marker itself, which could allow regs_get_register() to return an
> > invalid offset.
> >
> > Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> > Compile-tested only.
> >
> > Changes in v2:
> > - Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)
> > - Link to v1: https://lore.kernel.org/lkml/20250411090032.7844-1-thorsten.blum@linux.dev/
> >
> > Changes in v3:
> > - Keep the marker and avoid using #ifdef by adjusting MAX_REG_OFFSET as
> >   suggested by Thomas and Maciej
> > - Link to v2: https://lore.kernel.org/lkml/20250417174712.69292-2-thorsten.blum@linux.dev/
> > ---
> >  arch/mips/include/asm/ptrace.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> > index 85fa9962266a..ef72c46b5568 100644
> > --- a/arch/mips/include/asm/ptrace.h
> > +++ b/arch/mips/include/asm/ptrace.h
> > @@ -65,7 +65,8 @@ static inline void instruction_pointer_set(struct pt_regs *regs,
> >
> >  /* Query offset/name of register from its name/offset */
> >  extern int regs_query_register_offset(const char *name);
> > -#define MAX_REG_OFFSET (offsetof(struct pt_regs, __last))
> > +#define MAX_REG_OFFSET \
> > +       (offsetof(struct pt_regs, __last) - sizeof(unsigned long))
> There is no 80 columns limit now, so no new line needed here.

but not forbidden to care about it. I still prefer this limit.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Huacai Chen]

On Sun, Apr 27, 2025 at 8:52 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Sun, Apr 27, 2025 at 08:32:05PM +0800, Huacai Chen wrote:
> > Hi, Thorsten,
> >
> > On Sun, Apr 27, 2025 at 7:35 PM Thorsten Blum <thorsten.blum@linux.dev> wrote:
> > >
> > > Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
> > > the marker itself, which could allow regs_get_register() to return an
> > > invalid offset.
> > >
> > > Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
> > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > > ---
> > > Compile-tested only.
> > >
> > > Changes in v2:
> > > - Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)
> > > - Link to v1: https://lore.kernel.org/lkml/20250411090032.7844-1-thorsten.blum@linux.dev/
> > >
> > > Changes in v3:
> > > - Keep the marker and avoid using #ifdef by adjusting MAX_REG_OFFSET as
> > >   suggested by Thomas and Maciej
> > > - Link to v2: https://lore.kernel.org/lkml/20250417174712.69292-2-thorsten.blum@linux.dev/
> > > ---
> > >  arch/mips/include/asm/ptrace.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> > > index 85fa9962266a..ef72c46b5568 100644
> > > --- a/arch/mips/include/asm/ptrace.h
> > > +++ b/arch/mips/include/asm/ptrace.h
> > > @@ -65,7 +65,8 @@ static inline void instruction_pointer_set(struct pt_regs *regs,
> > >
> > >  /* Query offset/name of register from its name/offset */
> > >  extern int regs_query_register_offset(const char *name);
> > > -#define MAX_REG_OFFSET (offsetof(struct pt_regs, __last))
> > > +#define MAX_REG_OFFSET \
> > > +       (offsetof(struct pt_regs, __last) - sizeof(unsigned long))
> > There is no 80 columns limit now, so no new line needed here.
>
> but not forbidden to care about it. I still prefer this limit.
Of course you are free to choose. But in my opinion "force to long
lines" and "force to short lines" are both bad, code readability is
the first thing to be considered.

Huacai

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Maciej W. Rozycki]

On Sun, 27 Apr 2025, Huacai Chen wrote:

> > > There is no 80 columns limit now, so no new line needed here.
> >
> > but not forbidden to care about it. I still prefer this limit.
> Of course you are free to choose. But in my opinion "force to long
> lines" and "force to short lines" are both bad, code readability is
> the first thing to be considered.

 Correct, and I start getting lost when lines are wrapped by overrunning 
the width of my screen.  NB in the old days some terminals would actually 
truncate lines instead; at least it does not happen anymore, or at least 
you can tell your terminal not to do it via a suitable stty(1) invocation.

  Maciej

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Maciej W. Rozycki]

On Sun, 27 Apr 2025, Thorsten Blum wrote:

> Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
> the marker itself, which could allow regs_get_register() to return an
> invalid offset.
> 
> Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Compile-tested only.
> 
> Changes in v2:
> - Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)

 You're welcome; please add a `Suggested-by' tag accordingly.

  Maciej

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Thorsten Blum]

On 28. Apr 2025, at 03:35, Maciej W. Rozycki wrote:
> On Sun, 27 Apr 2025, Thorsten Blum wrote:
>> Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
>> the marker itself, which could allow regs_get_register() to return an
>> invalid offset.
>> 
>> Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> Compile-tested only.
>> 
>> Changes in v2:
>> - Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)
> 
> You're welcome; please add a `Suggested-by' tag accordingly.

Suggested-by: Maciej W. Rozycki <macro@orcam.me.uk>

Re: [PATCH v3] MIPS: Fix MAX_REG_OFFSET

[Thomas Bogendoerfer]

On Sun, Apr 27, 2025 at 01:34:24PM +0200, Thorsten Blum wrote:
> Fix MAX_REG_OFFSET to point to the last register in 'pt_regs' and not to
> the marker itself, which could allow regs_get_register() to return an
> invalid offset.
> 
> Fixes: 40e084a506eb ("MIPS: Add uprobes support.")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Compile-tested only.
> 
> Changes in v2:
> - Fix MAX_REG_OFFSET as suggested by Maciej (thanks!)
> - Link to v1: https://lore.kernel.org/lkml/20250411090032.7844-1-thorsten.blum@linux.dev/
> 
> Changes in v3:
> - Keep the marker and avoid using #ifdef by adjusting MAX_REG_OFFSET as
>   suggested by Thomas and Maciej
> - Link to v2: https://lore.kernel.org/lkml/20250417174712.69292-2-thorsten.blum@linux.dev/
> ---
>  arch/mips/include/asm/ptrace.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> index 85fa9962266a..ef72c46b5568 100644
> --- a/arch/mips/include/asm/ptrace.h
> +++ b/arch/mips/include/asm/ptrace.h
> @@ -65,7 +65,8 @@ static inline void instruction_pointer_set(struct pt_regs *regs,
>  
>  /* Query offset/name of register from its name/offset */
>  extern int regs_query_register_offset(const char *name);
> -#define MAX_REG_OFFSET (offsetof(struct pt_regs, __last))
> +#define MAX_REG_OFFSET \
> +	(offsetof(struct pt_regs, __last) - sizeof(unsigned long))
>  
>  /**
>   * regs_get_register() - get register value from its offset
> -- 
> 2.49.0

applied to mips-fixes.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]