linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/ptrace: Split gpr32_set_common
@ 2023-06-21 10:37 Christophe Leroy
  2023-06-21 11:57 ` kernel test robot
  2023-06-21 12:39 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Christophe Leroy @ 2023-06-21 10:37 UTC (permalink / raw)
  To: Michael Ellerman, Nicholas Piggin; +Cc: linuxppc-dev, linux-kernel

objtool report the following warning:

  arch/powerpc/kernel/ptrace/ptrace-view.o: warning: objtool:
    gpr32_set_common+0x23c (.text+0x860): redundant UACCESS disable

gpr32_set_common() conditionnaly opens and closes UACCESS based on
whether kbuf point is NULL or not. This is wackelig.

Split gpr32_set_common() in two fonctions, one for user one for
kernel.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/ptrace/ptrace-view.c | 106 ++++++++++++++---------
 1 file changed, 67 insertions(+), 39 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c
index 3910cd7bb2d9..1b0c2a234a7e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-view.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@ -716,73 +716,89 @@ int gpr32_get_common(struct task_struct *target,
 	return membuf_zero(&to, (ELF_NGREG - PT_REGS_COUNT) * sizeof(u32));
 }
 
-int gpr32_set_common(struct task_struct *target,
-		     const struct user_regset *regset,
-		     unsigned int pos, unsigned int count,
-		     const void *kbuf, const void __user *ubuf,
-		     unsigned long *regs)
+int gpr32_set_common_kernel(struct task_struct *target,
+			    const struct user_regset *regset,
+			    unsigned int pos, unsigned int count,
+			    const void *kbuf, unsigned long *regs)
 {
 	const compat_ulong_t *k = kbuf;
+
+	pos /= sizeof(compat_ulong_t);
+	count /= sizeof(compat_ulong_t);
+
+	for (; count > 0 && pos < PT_MSR; --count)
+		regs[pos++] = *k++;
+
+	if (count > 0 && pos == PT_MSR) {
+		set_user_msr(target, *k++);
+		++pos;
+		--count;
+	}
+
+	for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
+		regs[pos++] = *k++;
+	for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+		++k;
+
+	if (count > 0 && pos == PT_TRAP) {
+		set_user_trap(target, *k++);
+		++pos;
+		--count;
+	}
+
+	kbuf = k;
+	pos *= sizeof(compat_ulong_t);
+	count *= sizeof(compat_ulong_t);
+	user_regset_copyin_ignore(&pos, &count, &kbuf, NULL,
+				  (PT_TRAP + 1) * sizeof(compat_ulong_t), -1);
+	return 0;
+}
+
+int gpr32_set_common_user(struct task_struct *target,
+			  const struct user_regset *regset,
+			  unsigned int pos, unsigned int count,
+			  const void __user *ubuf, unsigned long *regs)
+{
 	const compat_ulong_t __user *u = ubuf;
 	compat_ulong_t reg;
 
-	if (!kbuf && !user_read_access_begin(u, count))
+	if (!user_read_access_begin(u, count))
 		return -EFAULT;
 
 	pos /= sizeof(reg);
 	count /= sizeof(reg);
 
-	if (kbuf)
-		for (; count > 0 && pos < PT_MSR; --count)
-			regs[pos++] = *k++;
-	else
-		for (; count > 0 && pos < PT_MSR; --count) {
-			unsafe_get_user(reg, u++, Efault);
-			regs[pos++] = reg;
-		}
-
+	for (; count > 0 && pos < PT_MSR; --count) {
+		unsafe_get_user(reg, u++, Efault);
+		regs[pos++] = reg;
+	}
 
 	if (count > 0 && pos == PT_MSR) {
-		if (kbuf)
-			reg = *k++;
-		else
-			unsafe_get_user(reg, u++, Efault);
+		unsafe_get_user(reg, u++, Efault);
 		set_user_msr(target, reg);
 		++pos;
 		--count;
 	}
 
-	if (kbuf) {
-		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
-			regs[pos++] = *k++;
-		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
-			++k;
-	} else {
-		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
-			unsafe_get_user(reg, u++, Efault);
-			regs[pos++] = reg;
-		}
-		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
-			unsafe_get_user(reg, u++, Efault);
+	for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
+		unsafe_get_user(reg, u++, Efault);
+		regs[pos++] = reg;
 	}
+	for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+		unsafe_get_user(reg, u++, Efault);
 
 	if (count > 0 && pos == PT_TRAP) {
-		if (kbuf)
-			reg = *k++;
-		else
-			unsafe_get_user(reg, u++, Efault);
+		unsafe_get_user(reg, u++, Efault);
 		set_user_trap(target, reg);
 		++pos;
 		--count;
 	}
-	if (!kbuf)
-		user_read_access_end();
+	user_read_access_end();
 
-	kbuf = k;
 	ubuf = u;
 	pos *= sizeof(reg);
 	count *= sizeof(reg);
-	user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+	user_regset_copyin_ignore(&pos, &count, NULL, &ubuf,
 				  (PT_TRAP + 1) * sizeof(reg), -1);
 	return 0;
 
@@ -791,6 +807,18 @@ int gpr32_set_common(struct task_struct *target,
 	return -EFAULT;
 }
 
+int gpr32_set_common(struct task_struct *target,
+		     const struct user_regset *regset,
+		     unsigned int pos, unsigned int count,
+		     const void *kbuf, const void __user *ubuf,
+		     unsigned long *regs)
+{
+	if (kbuf)
+		return gpr32_set_common_kernel(target, regset, pos, count, kbuf, regs);
+	else
+		return gpr32_set_common_user(target, regset, pos, count, ubuf, regs);
+}
+
 static int gpr32_get(struct task_struct *target,
 		     const struct user_regset *regset,
 		     struct membuf to)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/ptrace: Split gpr32_set_common
  2023-06-21 10:37 [PATCH] powerpc/ptrace: Split gpr32_set_common Christophe Leroy
@ 2023-06-21 11:57 ` kernel test robot
  2023-06-21 12:39 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2023-06-21 11:57 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman, Nicholas Piggin
  Cc: linuxppc-dev, linux-kernel, oe-kbuild-all

Hi Christophe,

kernel test robot noticed the following build errors:

[auto build test ERROR on powerpc/next]
[also build test ERROR on powerpc/fixes linus/master v6.4-rc7 next-20230621]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christophe-Leroy/powerpc-ptrace-Split-gpr32_set_common/20230621-183932
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
patch link:    https://lore.kernel.org/r/3086d189fa629e6c7bf800832921669450cc09bf.1687343697.git.christophe.leroy%40csgroup.eu
patch subject: [PATCH] powerpc/ptrace: Split gpr32_set_common
config: powerpc-allnoconfig (https://download.01.org/0day-ci/archive/20230621/202306211940.y4kIhSei-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230621/202306211940.y4kIhSei-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306211940.y4kIhSei-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/powerpc/kernel/ptrace/ptrace-view.c:719:5: error: no previous prototype for 'gpr32_set_common_kernel' [-Werror=missing-prototypes]
     719 | int gpr32_set_common_kernel(struct task_struct *target,
         |     ^~~~~~~~~~~~~~~~~~~~~~~
>> arch/powerpc/kernel/ptrace/ptrace-view.c:757:5: error: no previous prototype for 'gpr32_set_common_user' [-Werror=missing-prototypes]
     757 | int gpr32_set_common_user(struct task_struct *target,
         |     ^~~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +/gpr32_set_common_kernel +719 arch/powerpc/kernel/ptrace/ptrace-view.c

   718	
 > 719	int gpr32_set_common_kernel(struct task_struct *target,
   720				    const struct user_regset *regset,
   721				    unsigned int pos, unsigned int count,
   722				    const void *kbuf, unsigned long *regs)
   723	{
   724		const compat_ulong_t *k = kbuf;
   725	
   726		pos /= sizeof(compat_ulong_t);
   727		count /= sizeof(compat_ulong_t);
   728	
   729		for (; count > 0 && pos < PT_MSR; --count)
   730			regs[pos++] = *k++;
   731	
   732		if (count > 0 && pos == PT_MSR) {
   733			set_user_msr(target, *k++);
   734			++pos;
   735			--count;
   736		}
   737	
   738		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
   739			regs[pos++] = *k++;
   740		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
   741			++k;
   742	
   743		if (count > 0 && pos == PT_TRAP) {
   744			set_user_trap(target, *k++);
   745			++pos;
   746			--count;
   747		}
   748	
   749		kbuf = k;
   750		pos *= sizeof(compat_ulong_t);
   751		count *= sizeof(compat_ulong_t);
   752		user_regset_copyin_ignore(&pos, &count, &kbuf, NULL,
   753					  (PT_TRAP + 1) * sizeof(compat_ulong_t), -1);
   754		return 0;
   755	}
   756	
 > 757	int gpr32_set_common_user(struct task_struct *target,
   758				  const struct user_regset *regset,
   759				  unsigned int pos, unsigned int count,
   760				  const void __user *ubuf, unsigned long *regs)
   761	{
   762		const compat_ulong_t __user *u = ubuf;
   763		compat_ulong_t reg;
   764	
   765		if (!user_read_access_begin(u, count))
   766			return -EFAULT;
   767	
   768		pos /= sizeof(reg);
   769		count /= sizeof(reg);
   770	
   771		for (; count > 0 && pos < PT_MSR; --count) {
   772			unsafe_get_user(reg, u++, Efault);
   773			regs[pos++] = reg;
   774		}
   775	
   776		if (count > 0 && pos == PT_MSR) {
   777			unsafe_get_user(reg, u++, Efault);
   778			set_user_msr(target, reg);
   779			++pos;
   780			--count;
   781		}
   782	
   783		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
   784			unsafe_get_user(reg, u++, Efault);
   785			regs[pos++] = reg;
   786		}
   787		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
   788			unsafe_get_user(reg, u++, Efault);
   789	
   790		if (count > 0 && pos == PT_TRAP) {
   791			unsafe_get_user(reg, u++, Efault);
   792			set_user_trap(target, reg);
   793			++pos;
   794			--count;
   795		}
   796		user_read_access_end();
   797	
   798		ubuf = u;
   799		pos *= sizeof(reg);
   800		count *= sizeof(reg);
   801		user_regset_copyin_ignore(&pos, &count, NULL, &ubuf,
   802					  (PT_TRAP + 1) * sizeof(reg), -1);
   803		return 0;
   804	
   805	Efault:
   806		user_read_access_end();
   807		return -EFAULT;
   808	}
   809	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/ptrace: Split gpr32_set_common
  2023-06-21 10:37 [PATCH] powerpc/ptrace: Split gpr32_set_common Christophe Leroy
  2023-06-21 11:57 ` kernel test robot
@ 2023-06-21 12:39 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2023-06-21 12:39 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman, Nicholas Piggin
  Cc: linuxppc-dev, linux-kernel, oe-kbuild-all

Hi Christophe,

kernel test robot noticed the following build warnings:

[auto build test WARNING on powerpc/next]
[also build test WARNING on powerpc/fixes linus/master v6.4-rc7 next-20230621]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christophe-Leroy/powerpc-ptrace-Split-gpr32_set_common/20230621-183932
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
patch link:    https://lore.kernel.org/r/3086d189fa629e6c7bf800832921669450cc09bf.1687343697.git.christophe.leroy%40csgroup.eu
patch subject: [PATCH] powerpc/ptrace: Split gpr32_set_common
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20230621/202306212029.AbMrgmSQ-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230621/202306212029.AbMrgmSQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306212029.AbMrgmSQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/powerpc/kernel/ptrace/ptrace-view.c:719:5: warning: no previous prototype for 'gpr32_set_common_kernel' [-Wmissing-prototypes]
     719 | int gpr32_set_common_kernel(struct task_struct *target,
         |     ^~~~~~~~~~~~~~~~~~~~~~~
>> arch/powerpc/kernel/ptrace/ptrace-view.c:757:5: warning: no previous prototype for 'gpr32_set_common_user' [-Wmissing-prototypes]
     757 | int gpr32_set_common_user(struct task_struct *target,
         |     ^~~~~~~~~~~~~~~~~~~~~


vim +/gpr32_set_common_kernel +719 arch/powerpc/kernel/ptrace/ptrace-view.c

   718	
 > 719	int gpr32_set_common_kernel(struct task_struct *target,
   720				    const struct user_regset *regset,
   721				    unsigned int pos, unsigned int count,
   722				    const void *kbuf, unsigned long *regs)
   723	{
   724		const compat_ulong_t *k = kbuf;
   725	
   726		pos /= sizeof(compat_ulong_t);
   727		count /= sizeof(compat_ulong_t);
   728	
   729		for (; count > 0 && pos < PT_MSR; --count)
   730			regs[pos++] = *k++;
   731	
   732		if (count > 0 && pos == PT_MSR) {
   733			set_user_msr(target, *k++);
   734			++pos;
   735			--count;
   736		}
   737	
   738		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
   739			regs[pos++] = *k++;
   740		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
   741			++k;
   742	
   743		if (count > 0 && pos == PT_TRAP) {
   744			set_user_trap(target, *k++);
   745			++pos;
   746			--count;
   747		}
   748	
   749		kbuf = k;
   750		pos *= sizeof(compat_ulong_t);
   751		count *= sizeof(compat_ulong_t);
   752		user_regset_copyin_ignore(&pos, &count, &kbuf, NULL,
   753					  (PT_TRAP + 1) * sizeof(compat_ulong_t), -1);
   754		return 0;
   755	}
   756	
 > 757	int gpr32_set_common_user(struct task_struct *target,
   758				  const struct user_regset *regset,
   759				  unsigned int pos, unsigned int count,
   760				  const void __user *ubuf, unsigned long *regs)
   761	{
   762		const compat_ulong_t __user *u = ubuf;
   763		compat_ulong_t reg;
   764	
   765		if (!user_read_access_begin(u, count))
   766			return -EFAULT;
   767	
   768		pos /= sizeof(reg);
   769		count /= sizeof(reg);
   770	
   771		for (; count > 0 && pos < PT_MSR; --count) {
   772			unsafe_get_user(reg, u++, Efault);
   773			regs[pos++] = reg;
   774		}
   775	
   776		if (count > 0 && pos == PT_MSR) {
   777			unsafe_get_user(reg, u++, Efault);
   778			set_user_msr(target, reg);
   779			++pos;
   780			--count;
   781		}
   782	
   783		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
   784			unsafe_get_user(reg, u++, Efault);
   785			regs[pos++] = reg;
   786		}
   787		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
   788			unsafe_get_user(reg, u++, Efault);
   789	
   790		if (count > 0 && pos == PT_TRAP) {
   791			unsafe_get_user(reg, u++, Efault);
   792			set_user_trap(target, reg);
   793			++pos;
   794			--count;
   795		}
   796		user_read_access_end();
   797	
   798		ubuf = u;
   799		pos *= sizeof(reg);
   800		count *= sizeof(reg);
   801		user_regset_copyin_ignore(&pos, &count, NULL, &ubuf,
   802					  (PT_TRAP + 1) * sizeof(reg), -1);
   803		return 0;
   804	
   805	Efault:
   806		user_read_access_end();
   807		return -EFAULT;
   808	}
   809	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-06-21 12:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-21 10:37 [PATCH] powerpc/ptrace: Split gpr32_set_common Christophe Leroy
2023-06-21 11:57 ` kernel test robot
2023-06-21 12:39 ` kernel test robot

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).