linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	David Miller <davem@davemloft.net>,
	Tony Luck <tony.luck@intel.com>, Will Deacon <will@kernel.org>
Subject: [PATCH 04/41] [ia64] sanitize elf_access_gpreg()
Date: Mon, 29 Jun 2020 19:25:51 +0100	[thread overview]
Message-ID: <20200629182628.529995-4-viro@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20200629182628.529995-1-viro@ZenIV.linux.org.uk>

From: Al Viro <viro@zeniv.linux.org.uk>

The function takes the register number, finds the corresponding field
of pt_regs for registers that are saved there or does the unwind for the
registers that end up spilled on the kernel stack.  Then it reads from
or writes to the resulting location.

Unfortunately, finding the required pt_regs field is done by rather
horrible switch.  It's microoptimized in all the wrong places - it
even uses the knowledge that fields for r8..r11 follow each other
in pt_regs layout, while r12..r13 are not adjacent to those, etc.

All of that is to encode the mapping from register numbers to offsets +
the information that r4..r7 are not to be found in pt_regs.

It's deeply in nasal demon territory, at that - the games it plays
with pointer arithmetics on addresses of structure members are
undefined behaviour.

Valid C ends up with better code in this case: just initialize a constant
array with offsets of relevant pt_regs fields and we don't need that
switch anymore.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/ia64/kernel/ptrace.c | 57 +++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 82aaacf64583..b9d068903b98 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1273,52 +1273,41 @@ struct regset_getset {
 	int ret;
 };
 
+static const ptrdiff_t pt_offsets[16] =
+{
+#define R(n) offsetof(struct pt_regs, r##n)
+	[0] = -1, R(1), R(2), R(3),
+	[4] = -1, [5] = -1, [6] = -1, [7] = -1,
+	R(8), R(9), R(10), R(11), R(12), R(13), R(14), R(15),
+#undef R
+};
+
 static int
 access_elf_gpreg(struct task_struct *target, struct unw_frame_info *info,
 		unsigned long addr, unsigned long *data, int write_access)
 {
-	struct pt_regs *pt;
-	unsigned long *ptr = NULL;
-	int ret;
-	char nat = 0;
+	struct pt_regs *pt = task_pt_regs(target);
+	unsigned reg = addr / sizeof(unsigned long);
+	ptrdiff_t d = pt_offsets[reg];
 
-	pt = task_pt_regs(target);
-	switch (addr) {
-	case ELF_GR_OFFSET(1):
-		ptr = &pt->r1;
-		break;
-	case ELF_GR_OFFSET(2):
-	case ELF_GR_OFFSET(3):
-		ptr = (void *)&pt->r2 + (addr - ELF_GR_OFFSET(2));
-		break;
-	case ELF_GR_OFFSET(4) ... ELF_GR_OFFSET(7):
+	if (d >= 0) {
+		unsigned long *ptr = (void *)pt + d;
+		if (write_access)
+			*ptr = *data;
+		else
+			*data = *ptr;
+		return 0;
+	} else {
+		char nat = 0;
 		if (write_access) {
 			/* read NaT bit first: */
 			unsigned long dummy;
-
-			ret = unw_get_gr(info, addr/8, &dummy, &nat);
+			int ret = unw_get_gr(info, reg, &dummy, &nat);
 			if (ret < 0)
 				return ret;
 		}
-		return unw_access_gr(info, addr/8, data, &nat, write_access);
-	case ELF_GR_OFFSET(8) ... ELF_GR_OFFSET(11):
-		ptr = (void *)&pt->r8 + addr - ELF_GR_OFFSET(8);
-		break;
-	case ELF_GR_OFFSET(12):
-	case ELF_GR_OFFSET(13):
-		ptr = (void *)&pt->r12 + addr - ELF_GR_OFFSET(12);
-		break;
-	case ELF_GR_OFFSET(14):
-		ptr = &pt->r14;
-		break;
-	case ELF_GR_OFFSET(15):
-		ptr = &pt->r15;
+		return unw_access_gr(info, reg, data, &nat, write_access);
 	}
-	if (write_access)
-		*ptr = *data;
-	else
-		*data = *ptr;
-	return 0;
 }
 
 static int
-- 
2.11.0


  parent reply	other threads:[~2020-06-29 18:42 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29 18:23 [RFC][PATCHSET] regset ->get() rework Al Viro
2020-06-29 18:25 ` [PATCH 01/41] introduction of regset ->get() wrappers, switching ELF coredumps to those Al Viro
2020-06-29 18:25   ` [PATCH 02/41] x86: copy_fpstate_to_sigframe(): have fpregs_soft_get() use kernel buffer Al Viro
2020-06-29 18:25   ` [PATCH 03/41] x86: kill dump_fpu() Al Viro
2020-06-29 18:25   ` Al Viro [this message]
2020-06-29 18:25   ` [PATCH 05/41] [ia64] teach elf_access_reg() to handle the missing range (r16..r31) Al Viro
2020-06-29 18:25   ` [PATCH 06/41] [ia64] regularize do_gpregs_[gs]et() Al Viro
2020-06-29 18:25   ` [PATCH 07/41] [ia64] access_uarea(): stop bothering with gpregs_[gs]et() Al Viro
2020-06-29 18:25   ` [PATCH 08/41] [ia64] access_uarea(): don't bother with fpregs_[gs]et() Al Viro
2020-06-29 18:25   ` [PATCH 09/41] sparc64: switch genregs32_get() to use of get_from_target() Al Viro
2020-06-29 18:25   ` [PATCH 10/41] sparc32: get rid of odd callers of copy_regset_to_user() Al Viro
2020-06-29 18:25   ` [PATCH 11/41] sparc64: " Al Viro
2020-06-29 18:25   ` [PATCH 12/41] sparc32: get rid of odd callers of copy_regset_from_user() Al Viro
2020-06-29 18:26   ` [PATCH 13/41] sparc64: " Al Viro
2020-06-29 18:26   ` [PATCH 14/41] arm64: take fetching compat reg out of pt_regs into a new helper Al Viro
2020-06-29 18:26   ` [PATCH 15/41] arm64: get rid of copy_regset_to_user() in compat_ptrace_read_user() Al Viro
2020-06-29 18:26   ` [PATCH 16/41] arm64: sanitize compat_ptrace_write_user() Al Viro
2020-06-29 18:26   ` [PATCH 17/41] copy_regset_to_user(): do all copyout at once Al Viro
2020-06-29 18:26   ` [PATCH 18/41] regset: new method and helpers for it Al Viro
2020-06-29 19:23     ` Linus Torvalds
2020-06-29 20:30       ` Al Viro
2020-06-30 13:25         ` Al Viro
2020-06-30 16:53           ` Linus Torvalds
2020-06-30 19:40             ` Al Viro
2020-06-29 18:26   ` [PATCH 19/41] x86: switch to ->get2() Al Viro
2020-06-29 18:26   ` [PATCH 20/41] powerpc: " Al Viro
2020-06-29 18:26   ` [PATCH 21/41] s390: " Al Viro
2020-06-29 18:26   ` [PATCH 22/41] sparc: " Al Viro
2020-06-30 13:16     ` Al Viro
2020-06-29 18:26   ` [PATCH 23/41] mips: " Al Viro
2020-06-29 18:26   ` [PATCH 24/41] arm64: " Al Viro
2020-06-29 18:26   ` [PATCH 25/41] sh: convert " Al Viro
2020-06-29 18:26   ` [PATCH 26/41] arm: switch " Al Viro
2020-06-29 18:26   ` [PATCH 27/41] arc: " Al Viro
2020-06-29 18:26   ` [PATCH 28/41] ia64: " Al Viro
2020-06-29 18:26   ` [PATCH 29/41] c6x: " Al Viro
2020-06-29 18:26   ` [PATCH 30/41] riscv: " Al Viro
2020-06-29 18:26   ` [PATCH 31/41] openrisc: " Al Viro
2020-06-29 18:26   ` [PATCH 32/41] h8300: " Al Viro
2020-06-29 18:26   ` [PATCH 33/41] hexagon: " Al Viro
2020-08-11 18:35     ` Brian Cain
2020-06-29 18:26   ` [PATCH 34/41] nios2: " Al Viro
2020-06-29 18:26   ` [PATCH 35/41] nds32: " Al Viro
2020-06-29 18:26   ` [PATCH 36/41] parisc: " Al Viro
2020-06-29 18:26   ` [PATCH 37/41] xtensa: " Al Viro
2020-06-29 18:26   ` [PATCH 38/41] csky: " Al Viro
2020-06-29 18:26   ` [PATCH 39/41] regset: kill ->get() Al Viro
2020-06-29 18:26   ` [PATCH 40/41] regset(): kill ->get_size() Al Viro
2020-06-29 18:26   ` [PATCH 41/41] regset: kill user_regset_copyout{,_zero}() Al Viro
2020-07-23  8:00 ` [RFC][PATCHSET] regset ->get() rework Christoph Hellwig
2020-07-23 17:27   ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200629182628.529995-4-viro@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davem@davemloft.net \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).