Similarly to the work done earlier with writes, this series
converts signal32 to using user_read_access_begin/end and
unsafe_get_user() and friends.
Applies on to of the signal64 series, ie on merge-test (ca6e327fefb2)
Christophe Leroy (10):
signal: Add unsafe_get_compat_sigset()
powerpc/uaccess: Also perform 64 bits copies in
unsafe_copy_from_user() on ppc32
powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
powerpc/signal32: Rename save_user_regs_unsafe() and
save_general_regs_unsafe()
powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
powerpc/signal32: Perform access_ok() inside restore_user_regs()
powerpc/signal32: Reorder user reads in restore_tm_user_regs()
powerpc/signal32: Convert restore_[tm]_user_regs() to user access
block
powerpc/signal32: Convert do_setcontext[_tm]() to user access block
powerpc/signal32: Simplify logging in sigreturn()
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/include/asm/uaccess.h | 6 +-
arch/powerpc/kernel/signal.h | 22 +++
arch/powerpc/kernel/signal_32.c | 251 ++++++++++++++++-------------
include/linux/compat.h | 35 ++++
include/linux/uaccess.h | 1 +
6 files changed, 205 insertions(+), 112 deletions(-)
--
2.25.0
Similarly to commit 5cf773fc8f37 ("powerpc/uaccess: Also perform
64 bits copies in unsafe_copy_to_user() on ppc32")
ppc32 has an efficiant 64 bits unsafe_get_user(), so also use it in
order to unroll loops more.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/uaccess.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 77d837b16e4d..a4e791bcd3fe 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -432,9 +432,9 @@ do { \
size_t _len = (l); \
int _i; \
\
- for (_i = 0; _i < (_len & ~(sizeof(long) - 1)); _i += sizeof(long)) \
- unsafe_get_user(*(long *)(_dst + _i), (long __user *)(_src + _i), e); \
- if (IS_ENABLED(CONFIG_PPC64) && (_len & 4)) { \
+ for (_i = 0; _i < (_len & ~(sizeof(u64) - 1)); _i += sizeof(u64)) \
+ unsafe_get_user(*(u64 *)(_dst + _i), (u64 __user *)(_src + _i), e); \
+ if (_len & 4) { \
unsafe_get_user(*(u32 *)(_dst + _i), (u32 __user *)(_src + _i), e); \
_i += 4; \
} \
--
2.25.0
Convention is to prefix functions with __unsafe_ instead of
suffixing it with _unsafe.
Rename save_user_regs_unsafe() and save_general_regs_unsafe()
accordingly, that is respectively __unsafe_save_general_regs() and
__unsafe_save_user_regs().
Suggested-by: Christopher M. Riedl <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index c505b444a613..3b78748d6d85 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -94,7 +94,7 @@ static inline int get_sigset_t(sigset_t *set,
#define from_user_ptr(p) compat_ptr(p)
static __always_inline int
-save_general_regs_unsafe(struct pt_regs *regs, struct mcontext __user *frame)
+__unsafe_save_general_regs(struct pt_regs *regs, struct mcontext __user *frame)
{
elf_greg_t64 *gregs = (elf_greg_t64 *)regs;
int val, i;
@@ -151,7 +151,7 @@ static inline int get_sigset_t(sigset_t *set, const sigset_t __user *uset)
#define from_user_ptr(p) ((void __user *)(p))
static __always_inline int
-save_general_regs_unsafe(struct pt_regs *regs, struct mcontext __user *frame)
+__unsafe_save_general_regs(struct pt_regs *regs, struct mcontext __user *frame)
{
WARN_ON(!FULL_REGS(regs));
unsafe_copy_to_user(&frame->mc_gregs, regs, GP_REGS_SIZE, failed);
@@ -177,7 +177,7 @@ static inline int restore_general_regs(struct pt_regs *regs,
#endif
#define unsafe_save_general_regs(regs, frame, label) do { \
- if (save_general_regs_unsafe(regs, frame)) \
+ if (__unsafe_save_general_regs(regs, frame)) \
goto label; \
} while (0)
@@ -260,8 +260,8 @@ static void prepare_save_user_regs(int ctx_has_vsx_region)
#endif
}
-static int save_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user *frame,
- struct mcontext __user *tm_frame, int ctx_has_vsx_region)
+static int __unsafe_save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
+ struct mcontext __user *tm_frame, int ctx_has_vsx_region)
{
unsigned long msr = regs->msr;
@@ -338,7 +338,7 @@ static int save_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user *f
}
#define unsafe_save_user_regs(regs, frame, tm_frame, has_vsx, label) do { \
- if (save_user_regs_unsafe(regs, frame, tm_frame, has_vsx)) \
+ if (__unsafe_save_user_regs(regs, frame, tm_frame, has_vsx)) \
goto label; \
} while (0)
@@ -350,7 +350,7 @@ static int save_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user *f
* We also save the transactional registers to a second ucontext in the
* frame.
*
- * See save_user_regs_unsafe() and signal_64.c:setup_tm_sigcontexts().
+ * See __unsafe_save_user_regs() and signal_64.c:setup_tm_sigcontexts().
*/
static void prepare_save_tm_user_regs(void)
{
@@ -441,7 +441,7 @@ static int save_tm_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user
#endif /* CONFIG_VSX */
#ifdef CONFIG_SPE
/* SPE regs are not checkpointed with TM, so this section is
- * simply the same as in save_user_regs_unsafe().
+ * simply the same as in __unsafe_save_user_regs().
*/
if (current->thread.used_spe) {
unsafe_copy_to_user(&frame->mc_vregs, current->thread.evr,
--
2.25.0
Same spirit as commit debf122c777f ("powerpc/signal32: Simplify logging
in handle_rt_signal32()"), remove this intermediate 'addr' local var.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 592b889e3836..5be267b3a13e 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1352,7 +1352,6 @@ SYSCALL_DEFINE0(sigreturn)
struct sigcontext __user *sc;
struct sigcontext sigctx;
struct mcontext __user *sr;
- void __user *addr;
sigset_t set;
struct mcontext __user *mcp;
struct mcontext __user *tm_mcp = NULL;
@@ -1363,7 +1362,6 @@ SYSCALL_DEFINE0(sigreturn)
sf = (struct sigframe __user *)(regs->gpr[1] + __SIGNAL_FRAMESIZE);
sc = &sf->sctx;
- addr = sc;
if (copy_from_user(&sigctx, sc, sizeof(sigctx)))
goto badframe;
@@ -1392,16 +1390,19 @@ SYSCALL_DEFINE0(sigreturn)
goto badframe;
} else {
sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
- addr = sr;
- if (restore_user_regs(regs, sr, 1))
- goto badframe;
+ if (restore_user_regs(regs, sr, 1)) {
+ signal_fault(current, regs, "sys_sigreturn", sr);
+
+ force_sig(SIGSEGV);
+ return 0;
+ }
}
set_thread_flag(TIF_RESTOREALL);
return 0;
badframe:
- signal_fault(current, regs, "sys_sigreturn", addr);
+ signal_fault(current, regs, "sys_sigreturn", sc);
force_sig(SIGSEGV);
return 0;
--
2.25.0
Convert restore_user_regs() and restore_tm_user_regs()
to use user_access_read_begin/end blocks.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/kernel/signal_32.c | 141 +++++++++++++++---------------
2 files changed, 72 insertions(+), 71 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index f10498e1b3f6..95600f3a6523 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -245,7 +245,7 @@ static inline bool trap_norestart(struct pt_regs *regs)
return regs->trap & 0x10;
}
-static inline void set_trap_norestart(struct pt_regs *regs)
+static __always_inline void set_trap_norestart(struct pt_regs *regs)
{
regs->trap |= 0x10;
}
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 088c83853026..0b1a6f53e553 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -116,8 +116,8 @@ __unsafe_save_general_regs(struct pt_regs *regs, struct mcontext __user *frame)
return 1;
}
-static inline int restore_general_regs(struct pt_regs *regs,
- struct mcontext __user *sr)
+static __always_inline int
+__unsafe_restore_general_regs(struct pt_regs *regs, struct mcontext __user *sr)
{
elf_greg_t64 *gregs = (elf_greg_t64 *)regs;
int i;
@@ -125,10 +125,12 @@ static inline int restore_general_regs(struct pt_regs *regs,
for (i = 0; i <= PT_RESULT; i++) {
if ((i == PT_MSR) || (i == PT_SOFTE))
continue;
- if (__get_user(gregs[i], &sr->mc_gregs[i]))
- return -EFAULT;
+ unsafe_get_user(gregs[i], &sr->mc_gregs[i], failed);
}
return 0;
+
+failed:
+ return 1;
}
#else /* CONFIG_PPC64 */
@@ -161,18 +163,20 @@ __unsafe_save_general_regs(struct pt_regs *regs, struct mcontext __user *frame)
return 1;
}
-static inline int restore_general_regs(struct pt_regs *regs,
- struct mcontext __user *sr)
+static __always_inline
+int __unsafe_restore_general_regs(struct pt_regs *regs, struct mcontext __user *sr)
{
/* copy up to but not including MSR */
- if (__copy_from_user(regs, &sr->mc_gregs,
- PT_MSR * sizeof(elf_greg_t)))
- return -EFAULT;
+ unsafe_copy_from_user(regs, &sr->mc_gregs, PT_MSR * sizeof(elf_greg_t), failed);
+
/* copy from orig_r3 (the word after the MSR) up to the end */
- if (__copy_from_user(®s->orig_gpr3, &sr->mc_gregs[PT_ORIG_R3],
- GP_REGS_SIZE - PT_ORIG_R3 * sizeof(elf_greg_t)))
- return -EFAULT;
+ unsafe_copy_from_user(®s->orig_gpr3, &sr->mc_gregs[PT_ORIG_R3],
+ GP_REGS_SIZE - PT_ORIG_R3 * sizeof(elf_greg_t), failed);
+
return 0;
+
+failed:
+ return 1;
}
#endif
@@ -181,6 +185,11 @@ static inline int restore_general_regs(struct pt_regs *regs,
goto label; \
} while (0)
+#define unsafe_restore_general_regs(regs, frame, label) do { \
+ if (__unsafe_restore_general_regs(regs, frame)) \
+ goto label; \
+} while (0)
+
/*
* When we have signals to deliver, we set up on the
* user stack, going down from the original stack pointer:
@@ -485,14 +494,13 @@ static int save_tm_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user
static long restore_user_regs(struct pt_regs *regs,
struct mcontext __user *sr, int sig)
{
- long err;
unsigned int save_r2 = 0;
unsigned long msr;
#ifdef CONFIG_VSX
int i;
#endif
- if (!access_ok(sr, sizeof(*sr)))
+ if (!user_read_access_begin(sr, sizeof(*sr)))
return 1;
/*
* restore general registers but not including MSR or SOFTE. Also
@@ -500,13 +508,11 @@ static long restore_user_regs(struct pt_regs *regs,
*/
if (!sig)
save_r2 = (unsigned int)regs->gpr[2];
- err = restore_general_regs(regs, sr);
+ unsafe_restore_general_regs(regs, sr, failed);
set_trap_norestart(regs);
- err |= __get_user(msr, &sr->mc_gregs[PT_MSR]);
+ unsafe_get_user(msr, &sr->mc_gregs[PT_MSR], failed);
if (!sig)
regs->gpr[2] = (unsigned long) save_r2;
- if (err)
- return 1;
/* if doing signal return, restore the previous little-endian mode */
if (sig)
@@ -520,22 +526,19 @@ static long restore_user_regs(struct pt_regs *regs,
regs->msr &= ~MSR_VEC;
if (msr & MSR_VEC) {
/* restore altivec registers from the stack */
- if (__copy_from_user(¤t->thread.vr_state, &sr->mc_vregs,
- sizeof(sr->mc_vregs)))
- return 1;
+ unsafe_copy_from_user(¤t->thread.vr_state, &sr->mc_vregs,
+ sizeof(sr->mc_vregs), failed);
current->thread.used_vr = true;
} else if (current->thread.used_vr)
memset(¤t->thread.vr_state, 0,
ELF_NVRREG * sizeof(vector128));
/* Always get VRSAVE back */
- if (__get_user(current->thread.vrsave, (u32 __user *)&sr->mc_vregs[32]))
- return 1;
+ unsafe_get_user(current->thread.vrsave, (u32 __user *)&sr->mc_vregs[32], failed);
if (cpu_has_feature(CPU_FTR_ALTIVEC))
mtspr(SPRN_VRSAVE, current->thread.vrsave);
#endif /* CONFIG_ALTIVEC */
- if (copy_fpr_from_user(current, &sr->mc_fregs))
- return 1;
+ unsafe_copy_fpr_from_user(current, &sr->mc_fregs, failed);
#ifdef CONFIG_VSX
/*
@@ -548,8 +551,7 @@ static long restore_user_regs(struct pt_regs *regs,
* Restore altivec registers from the stack to a local
* buffer, then write this out to the thread_struct
*/
- if (copy_vsx_from_user(current, &sr->mc_vsregs))
- return 1;
+ unsafe_copy_vsx_from_user(current, &sr->mc_vsregs, failed);
current->thread.used_vsr = true;
} else if (current->thread.used_vsr)
for (i = 0; i < 32 ; i++)
@@ -567,19 +569,22 @@ static long restore_user_regs(struct pt_regs *regs,
regs->msr &= ~MSR_SPE;
if (msr & MSR_SPE) {
/* restore spe registers from the stack */
- if (__copy_from_user(current->thread.evr, &sr->mc_vregs,
- ELF_NEVRREG * sizeof(u32)))
- return 1;
+ unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs,
+ ELF_NEVRREG * sizeof(u32));
current->thread.used_spe = true;
} else if (current->thread.used_spe)
memset(current->thread.evr, 0, ELF_NEVRREG * sizeof(u32));
/* Always get SPEFSCR back */
- if (__get_user(current->thread.spefscr, (u32 __user *)&sr->mc_vregs + ELF_NEVRREG))
- return 1;
+ unsafe_get_user(current->thread.spefscr, (u32 __user *)&sr->mc_vregs + ELF_NEVRREG, failed);
#endif /* CONFIG_SPE */
+ user_read_access_end();
return 0;
+
+failed:
+ user_read_access_end();
+ return 1;
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -592,7 +597,6 @@ static long restore_tm_user_regs(struct pt_regs *regs,
struct mcontext __user *sr,
struct mcontext __user *tm_sr)
{
- long err;
unsigned long msr, msr_hi;
#ifdef CONFIG_VSX
int i;
@@ -607,14 +611,13 @@ static long restore_tm_user_regs(struct pt_regs *regs,
* TFHAR is restored from the checkpointed NIP; TEXASR and TFIAR
* were set by the signal delivery.
*/
- err = restore_general_regs(¤t->thread.ckpt_regs, sr);
-
- err |= __get_user(current->thread.tm_tfhar, &sr->mc_gregs[PT_NIP]);
-
- err |= __get_user(msr, &sr->mc_gregs[PT_MSR]);
- if (err)
+ if (!user_read_access_begin(sr, sizeof(*sr)))
return 1;
+ unsafe_restore_general_regs(¤t->thread.ckpt_regs, sr, failed);
+ unsafe_get_user(current->thread.tm_tfhar, &sr->mc_gregs[PT_NIP], failed);
+ unsafe_get_user(msr, &sr->mc_gregs[PT_MSR], failed);
+
/* Restore the previous little-endian mode */
regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
@@ -622,9 +625,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,
regs->msr &= ~MSR_VEC;
if (msr & MSR_VEC) {
/* restore altivec registers from the stack */
- if (__copy_from_user(¤t->thread.ckvr_state, &sr->mc_vregs,
- sizeof(sr->mc_vregs)))
- return 1;
+ unsafe_copy_from_user(¤t->thread.ckvr_state, &sr->mc_vregs,
+ sizeof(sr->mc_vregs), failed);
current->thread.used_vr = true;
} else if (current->thread.used_vr) {
memset(¤t->thread.vr_state, 0,
@@ -634,17 +636,15 @@ static long restore_tm_user_regs(struct pt_regs *regs,
}
/* Always get VRSAVE back */
- if (__get_user(current->thread.ckvrsave,
- (u32 __user *)&sr->mc_vregs[32]))
- return 1;
+ unsafe_get_user(current->thread.ckvrsave,
+ (u32 __user *)&sr->mc_vregs[32], failed);
if (cpu_has_feature(CPU_FTR_ALTIVEC))
mtspr(SPRN_VRSAVE, current->thread.ckvrsave);
#endif /* CONFIG_ALTIVEC */
regs->msr &= ~(MSR_FP | MSR_FE0 | MSR_FE1);
- if (copy_fpr_from_user(current, &sr->mc_fregs))
- return 1;
+ unsafe_copy_fpr_from_user(current, &sr->mc_fregs, failed);
#ifdef CONFIG_VSX
regs->msr &= ~MSR_VSX;
@@ -653,8 +653,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
* Restore altivec registers from the stack to a local
* buffer, then write this out to the thread_struct
*/
- if (copy_ckvsx_from_user(current, &sr->mc_vsregs))
- return 1;
+ unsafe_copy_ckvsx_from_user(current, &sr->mc_vsregs, failed);
current->thread.used_vsr = true;
} else if (current->thread.used_vsr)
for (i = 0; i < 32 ; i++) {
@@ -669,39 +668,36 @@ static long restore_tm_user_regs(struct pt_regs *regs,
*/
regs->msr &= ~MSR_SPE;
if (msr & MSR_SPE) {
- if (__copy_from_user(current->thread.evr, &sr->mc_vregs,
- ELF_NEVRREG * sizeof(u32)))
- return 1;
+ unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs,
+ ELF_NEVRREG * sizeof(u32), failed);
current->thread.used_spe = true;
} else if (current->thread.used_spe)
memset(current->thread.evr, 0, ELF_NEVRREG * sizeof(u32));
/* Always get SPEFSCR back */
- if (__get_user(current->thread.spefscr, (u32 __user *)&sr->mc_vregs
- + ELF_NEVRREG))
- return 1;
+ unsafe_get_user(current->thread.spefscr,
+ (u32 __user *)&sr->mc_vregs + ELF_NEVRREG, failed);
#endif /* CONFIG_SPE */
- err = restore_general_regs(regs, tm_sr);
- if (err)
+ user_read_access_end();
+
+ if (!user_read_access_begin(tm_sr, sizeof(*tm_sr)))
return 1;
+ unsafe_restore_general_regs(regs, tm_sr, failed);
+
#ifdef CONFIG_ALTIVEC
/* restore altivec registers from the stack */
if (msr & MSR_VEC)
- if (__copy_from_user(¤t->thread.vr_state,
- &tm_sr->mc_vregs,
- sizeof(sr->mc_vregs)))
- return 1;
+ unsafe_copy_from_user(¤t->thread.vr_state, &tm_sr->mc_vregs,
+ sizeof(sr->mc_vregs), failed);
/* Always get VRSAVE back */
- if (__get_user(current->thread.vrsave,
- (u32 __user *)&tm_sr->mc_vregs[32]))
- return 1;
+ unsafe_get_user(current->thread.vrsave,
+ (u32 __user *)&tm_sr->mc_vregs[32], failed);
#endif /* CONFIG_ALTIVEC */
- if (copy_ckfpr_from_user(current, &tm_sr->mc_fregs))
- return 1;
+ unsafe_copy_ckfpr_from_user(current, &tm_sr->mc_fregs, failed);
#ifdef CONFIG_VSX
if (msr & MSR_VSX) {
@@ -709,16 +705,17 @@ static long restore_tm_user_regs(struct pt_regs *regs,
* Restore altivec registers from the stack to a local
* buffer, then write this out to the thread_struct
*/
- if (copy_vsx_from_user(current, &tm_sr->mc_vsregs))
- return 1;
+ unsafe_copy_vsx_from_user(current, &tm_sr->mc_vsregs, failed);
current->thread.used_vsr = true;
}
#endif /* CONFIG_VSX */
/* Get the top half of the MSR from the user context */
- if (__get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR]))
- return 1;
+ unsafe_get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR], failed);
msr_hi <<= 32;
+
+ user_read_access_end();
+
/* If TM bits are set to the reserved value, it's an invalid context */
if (MSR_TM_RESV(msr_hi))
return 1;
@@ -766,6 +763,10 @@ static long restore_tm_user_regs(struct pt_regs *regs,
preempt_enable();
return 0;
+
+failed:
+ user_read_access_end();
+ return 1;
}
#else
static long restore_tm_user_regs(struct pt_regs *regs, struct mcontext __user *sr,
--
2.25.0
In restore_tm_user_regs(), regroup the reads from 'sr' and the ones
from 'tm_sr' together in order to allow two block user accesses
in following patch.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 49 +++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index e2b1d2a0abad..088c83853026 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -607,8 +607,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
* TFHAR is restored from the checkpointed NIP; TEXASR and TFIAR
* were set by the signal delivery.
*/
- err = restore_general_regs(regs, tm_sr);
- err |= restore_general_regs(¤t->thread.ckpt_regs, sr);
+ err = restore_general_regs(¤t->thread.ckpt_regs, sr);
err |= __get_user(current->thread.tm_tfhar, &sr->mc_gregs[PT_NIP]);
@@ -624,9 +623,6 @@ static long restore_tm_user_regs(struct pt_regs *regs,
if (msr & MSR_VEC) {
/* restore altivec registers from the stack */
if (__copy_from_user(¤t->thread.ckvr_state, &sr->mc_vregs,
- sizeof(sr->mc_vregs)) ||
- __copy_from_user(¤t->thread.vr_state,
- &tm_sr->mc_vregs,
sizeof(sr->mc_vregs)))
return 1;
current->thread.used_vr = true;
@@ -639,9 +635,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
/* Always get VRSAVE back */
if (__get_user(current->thread.ckvrsave,
- (u32 __user *)&sr->mc_vregs[32]) ||
- __get_user(current->thread.vrsave,
- (u32 __user *)&tm_sr->mc_vregs[32]))
+ (u32 __user *)&sr->mc_vregs[32]))
return 1;
if (cpu_has_feature(CPU_FTR_ALTIVEC))
mtspr(SPRN_VRSAVE, current->thread.ckvrsave);
@@ -649,8 +643,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
regs->msr &= ~(MSR_FP | MSR_FE0 | MSR_FE1);
- if (copy_fpr_from_user(current, &sr->mc_fregs) ||
- copy_ckfpr_from_user(current, &tm_sr->mc_fregs))
+ if (copy_fpr_from_user(current, &sr->mc_fregs))
return 1;
#ifdef CONFIG_VSX
@@ -660,8 +653,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
* Restore altivec registers from the stack to a local
* buffer, then write this out to the thread_struct
*/
- if (copy_vsx_from_user(current, &tm_sr->mc_vsregs) ||
- copy_ckvsx_from_user(current, &sr->mc_vsregs))
+ if (copy_ckvsx_from_user(current, &sr->mc_vsregs))
return 1;
current->thread.used_vsr = true;
} else if (current->thread.used_vsr)
@@ -690,6 +682,39 @@ static long restore_tm_user_regs(struct pt_regs *regs,
return 1;
#endif /* CONFIG_SPE */
+ err = restore_general_regs(regs, tm_sr);
+ if (err)
+ return 1;
+
+#ifdef CONFIG_ALTIVEC
+ /* restore altivec registers from the stack */
+ if (msr & MSR_VEC)
+ if (__copy_from_user(¤t->thread.vr_state,
+ &tm_sr->mc_vregs,
+ sizeof(sr->mc_vregs)))
+ return 1;
+
+ /* Always get VRSAVE back */
+ if (__get_user(current->thread.vrsave,
+ (u32 __user *)&tm_sr->mc_vregs[32]))
+ return 1;
+#endif /* CONFIG_ALTIVEC */
+
+ if (copy_ckfpr_from_user(current, &tm_sr->mc_fregs))
+ return 1;
+
+#ifdef CONFIG_VSX
+ if (msr & MSR_VSX) {
+ /*
+ * Restore altivec registers from the stack to a local
+ * buffer, then write this out to the thread_struct
+ */
+ if (copy_vsx_from_user(current, &tm_sr->mc_vsregs))
+ return 1;
+ current->thread.used_vsr = true;
+ }
+#endif /* CONFIG_VSX */
+
/* Get the top half of the MSR from the user context */
if (__get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR]))
return 1;
--
2.25.0
In preparation of using user_access_begin/end in restore_user_regs(),
move the access_ok() inside the function.
It makes no difference as the behaviour on a failed access_ok() is
the same as on failed restore_user_regs().
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 8dfe4fe77706..e2b1d2a0abad 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -492,6 +492,8 @@ static long restore_user_regs(struct pt_regs *regs,
int i;
#endif
+ if (!access_ok(sr, sizeof(*sr)))
+ return 1;
/*
* restore general registers but not including MSR or SOFTE. Also
* take care of keeping r2 (TLS) intact if not a signal
@@ -963,13 +965,10 @@ static int do_setcontext(struct ucontext __user *ucp, struct pt_regs *regs, int
if (__get_user(cmcp, &ucp->uc_regs))
return -EFAULT;
mcp = (struct mcontext __user *)(u64)cmcp;
- /* no need to check access_ok(mcp), since mcp < 4GB */
}
#else
if (__get_user(mcp, &ucp->uc_regs))
return -EFAULT;
- if (!access_ok(mcp, sizeof(*mcp)))
- return -EFAULT;
#endif
set_current_blocked(&set);
if (restore_user_regs(regs, mcp, sig))
@@ -1362,8 +1361,7 @@ SYSCALL_DEFINE0(sigreturn)
} else {
sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
addr = sr;
- if (!access_ok(sr, sizeof(*sr))
- || restore_user_regs(regs, sr, 1))
+ if (restore_user_regs(regs, sr, 1))
goto badframe;
}
--
2.25.0
In the same spirit as commit f1cf4f93de2f ("powerpc/signal32: Remove
ifdefery in middle of if/else")
MSR_TM_ACTIVE() is always defined and returns always 0 when
CONFIG_PPC_TRANSACTIONAL_MEM is not selected, so the awful
ifdefery in the middle of an if/else can be removed.
Make 'msr_hi' a 'long long' to avoid build failure on PPC32
due to the 32 bits left shift.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 3b78748d6d85..8dfe4fe77706 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -740,6 +740,12 @@ static long restore_tm_user_regs(struct pt_regs *regs,
return 0;
}
+#else
+static long restore_tm_user_regs(struct pt_regs *regs, struct mcontext __user *sr,
+ struct mcontext __user *tm_sr)
+{
+ return 0;
+}
#endif
#ifdef CONFIG_PPC64
@@ -1317,10 +1323,9 @@ SYSCALL_DEFINE0(sigreturn)
struct mcontext __user *sr;
void __user *addr;
sigset_t set;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- struct mcontext __user *mcp, *tm_mcp;
- unsigned long msr_hi;
-#endif
+ struct mcontext __user *mcp;
+ struct mcontext __user *tm_mcp = NULL;
+ unsigned long long msr_hi = 0;
/* Always make any pending restarted system calls return -EINTR */
current->restart_block.fn = do_no_restart_syscall;
@@ -1343,19 +1348,18 @@ SYSCALL_DEFINE0(sigreturn)
#endif
set_current_blocked(&set);
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
mcp = (struct mcontext __user *)&sf->mctx;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
tm_mcp = (struct mcontext __user *)&sf->mctx_transact;
if (__get_user(msr_hi, &tm_mcp->mc_gregs[PT_MSR]))
goto badframe;
+#endif
if (MSR_TM_ACTIVE(msr_hi<<32)) {
if (!cpu_has_feature(CPU_FTR_TM))
goto badframe;
if (restore_tm_user_regs(regs, mcp, tm_mcp))
goto badframe;
- } else
-#endif
- {
+ } else {
sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
addr = sr;
if (!access_ok(sr, sizeof(*sr))
--
2.25.0
Add unsafe_get_user_sigset() and transform PPC32 get_sigset_t()
into an unsafe version unsafe_get_sigset_t().
Then convert do_setcontext() and do_setcontext_tm() to use
user_read_access_begin/end.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal.h | 2 ++
arch/powerpc/kernel/signal_32.c | 42 +++++++++++++++++++--------------
2 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index a5152ff3c52f..f4aafa337c2e 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -25,6 +25,8 @@ static inline int __get_user_sigset(sigset_t *dst, const sigset_t __user *src)
return __get_user(dst->sig[0], (u64 __user *)&src->sig[0]);
}
+#define unsafe_get_user_sigset(dst, src, label) \
+ unsafe_get_user((dst)->sig[0], (u64 __user *)&(src)->sig[0], label)
#ifdef CONFIG_VSX
extern unsigned long copy_vsx_to_user(void __user *to,
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 0b1a6f53e553..592b889e3836 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -83,12 +83,7 @@
* implementation that makes things simple for little endian only)
*/
#define unsafe_put_sigset_t unsafe_put_compat_sigset
-
-static inline int get_sigset_t(sigset_t *set,
- const compat_sigset_t __user *uset)
-{
- return get_compat_sigset(set, uset);
-}
+#define unsafe_get_sigset_t unsafe_get_compat_sigset
#define to_user_ptr(p) ptr_to_compat(p)
#define from_user_ptr(p) compat_ptr(p)
@@ -144,10 +139,7 @@ __unsafe_restore_general_regs(struct pt_regs *regs, struct mcontext __user *sr)
unsafe_copy_to_user(__us, __s, sizeof(*__us), label); \
} while (0)
-static inline int get_sigset_t(sigset_t *set, const sigset_t __user *uset)
-{
- return __get_user_sigset(set, uset);
-}
+#define unsafe_get_sigset_t unsafe_get_user_sigset
#define to_user_ptr(p) ((unsigned long)(p))
#define from_user_ptr(p) ((void __user *)(p))
@@ -982,25 +974,31 @@ static int do_setcontext(struct ucontext __user *ucp, struct pt_regs *regs, int
sigset_t set;
struct mcontext __user *mcp;
- if (get_sigset_t(&set, &ucp->uc_sigmask))
+ if (user_read_access_begin(ucp, sizeof(*ucp)))
return -EFAULT;
+
+ unsafe_get_sigset_t(&set, &ucp->uc_sigmask, failed);
#ifdef CONFIG_PPC64
{
u32 cmcp;
- if (__get_user(cmcp, &ucp->uc_regs))
- return -EFAULT;
+ unsafe_get_user(cmcp, &ucp->uc_regs, failed);
mcp = (struct mcontext __user *)(u64)cmcp;
}
#else
- if (__get_user(mcp, &ucp->uc_regs))
- return -EFAULT;
+ unsafe_get_user(mcp, &ucp->uc_regs, failed);
#endif
+ user_read_access_end();
+
set_current_blocked(&set);
if (restore_user_regs(regs, mcp, sig))
return -EFAULT;
return 0;
+
+failed:
+ user_read_access_end();
+ return -EFAULT;
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -1014,11 +1012,15 @@ static int do_setcontext_tm(struct ucontext __user *ucp,
u32 cmcp;
u32 tm_cmcp;
- if (get_sigset_t(&set, &ucp->uc_sigmask))
+ if (user_read_access_begin(ucp, sizeof(*ucp)))
return -EFAULT;
- if (__get_user(cmcp, &ucp->uc_regs) ||
- __get_user(tm_cmcp, &tm_ucp->uc_regs))
+ unsafe_get_sigset_t(&set, &ucp->uc_sigmask, failed);
+ unsafe_get_user(cmcp, &ucp->uc_regs, failed);
+
+ user_read_access_end();
+
+ if (__get_user(tm_cmcp, &tm_ucp->uc_regs))
return -EFAULT;
mcp = (struct mcontext __user *)(u64)cmcp;
tm_mcp = (struct mcontext __user *)(u64)tm_cmcp;
@@ -1029,6 +1031,10 @@ static int do_setcontext_tm(struct ucontext __user *ucp,
return -EFAULT;
return 0;
+
+failed:
+ user_read_access_end();
+ return -EFAULT;
}
#endif
--
2.25.0
Le 19/03/2021 à 12:06, Christophe Leroy a écrit :
> Convert restore_user_regs() and restore_tm_user_regs()
> to use user_access_read_begin/end blocks.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/include/asm/ptrace.h | 2 +-
> arch/powerpc/kernel/signal_32.c | 141 +++++++++++++++---------------
> 2 files changed, 72 insertions(+), 71 deletions(-)
>
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 088c83853026..0b1a6f53e553 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -567,19 +569,22 @@ static long restore_user_regs(struct pt_regs *regs,
> regs->msr &= ~MSR_SPE;
> if (msr & MSR_SPE) {
> /* restore spe registers from the stack */
> - if (__copy_from_user(current->thread.evr, &sr->mc_vregs,
> - ELF_NEVRREG * sizeof(u32)))
> - return 1;
> + unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs,
> + ELF_NEVRREG * sizeof(u32));
Missing the , failed); here at the end of the line.
Michael can you add it ?
Thanks
Christophe
On Fri, 19 Mar 2021 11:06:49 +0000 (UTC), Christophe Leroy wrote:
> Similarly to the work done earlier with writes, this series
> converts signal32 to using user_read_access_begin/end and
> unsafe_get_user() and friends.
>
> Applies on to of the signal64 series, ie on merge-test (ca6e327fefb2)
>
> Christophe Leroy (10):
> signal: Add unsafe_get_compat_sigset()
> powerpc/uaccess: Also perform 64 bits copies in
> unsafe_copy_from_user() on ppc32
> powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
> powerpc/signal32: Rename save_user_regs_unsafe() and
> save_general_regs_unsafe()
> powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
> powerpc/signal32: Perform access_ok() inside restore_user_regs()
> powerpc/signal32: Reorder user reads in restore_tm_user_regs()
> powerpc/signal32: Convert restore_[tm]_user_regs() to user access
> block
> powerpc/signal32: Convert do_setcontext[_tm]() to user access block
> powerpc/signal32: Simplify logging in sigreturn()
>
> [...]
Applied to powerpc/next.
[01/10] signal: Add unsafe_get_compat_sigset()
https://git.kernel.org/powerpc/c/fb05121fd6a20f0830ff2a4420c51af6ca4ac6e7
[02/10] powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_from_user() on ppc32
https://git.kernel.org/powerpc/c/c1cc1570bc8d94f288060f262f11be8f7672578c
[03/10] powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
https://git.kernel.org/powerpc/c/7c11f8893a76ac4e86c07f4b57371d5fa593627f
[04/10] powerpc/signal32: Rename save_user_regs_unsafe() and save_general_regs_unsafe()
https://git.kernel.org/powerpc/c/f918a81e209f24acb45cd935bcfb78d2c024f6a1
[05/10] powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
https://git.kernel.org/powerpc/c/ca9e1605cdd9473a0eb4d6da238d2524be12591a
[06/10] powerpc/signal32: Perform access_ok() inside restore_user_regs()
https://git.kernel.org/powerpc/c/362471b3192e4184fff5fedee1ea20bdf637a0c8
[07/10] powerpc/signal32: Reorder user reads in restore_tm_user_regs()
https://git.kernel.org/powerpc/c/036fc2cb1dc2245c2ea7d2f03c7af80417b6310c
[08/10] powerpc/signal32: Convert restore_[tm]_user_regs() to user access block
https://git.kernel.org/powerpc/c/627b72bee84d6652e0af26617e71ce2b3c18fcd5
[09/10] powerpc/signal32: Convert do_setcontext[_tm]() to user access block
https://git.kernel.org/powerpc/c/887f3ceb51cd34109ac17bfc98695162e299e657
[10/10] powerpc/signal32: Simplify logging in sigreturn()
https://git.kernel.org/powerpc/c/c7393a71eb1abdda7e3a3ef798bae60de11540ec
cheers
On Fri, Mar 19, 2021 at 11:06:57AM +0000, Christophe Leroy wrote:
> Convert restore_user_regs() and restore_tm_user_regs()
> to use user_access_read_begin/end blocks.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
...
> static long restore_user_regs(struct pt_regs *regs,
> struct mcontext __user *sr, int sig)
> {
...
> @@ -567,19 +569,22 @@ static long restore_user_regs(struct pt_regs *regs,
> regs->msr &= ~MSR_SPE;
> if (msr & MSR_SPE) {
> /* restore spe registers from the stack */
> - if (__copy_from_user(current->thread.evr, &sr->mc_vregs,
> - ELF_NEVRREG * sizeof(u32)))
> - return 1;
> + unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs,
> + ELF_NEVRREG * sizeof(u32));
arch/powerpc/kernel/signal_32.c: In function 'restore_user_regs':
arch/powerpc/kernel/signal_32.c:565:36: error: macro "unsafe_copy_from_user" requires 4 arguments, but only 3 given
Guenter