linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality
@ 2020-06-08  2:49 Wang ShaoBo
  2020-06-08  7:50 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Wang ShaoBo @ 2020-06-08  2:49 UTC (permalink / raw)
  Cc: huawei.libin, cj.chengjian, xiexiuqi, mark.rutland, hch, wcohen,
	linux-kernel, mtk.manpages, wezhang, gregkh, catalin.marinas,
	bobo.shaobowang

Currently arm64 personality syscall uses wrapper __arm64_sys_personality
to redirect to __arm64_sys_arm64_personality, it's easily confused,
Whereas using an normal hook arch_check_personality() can reject
additional settings like this for special case of different architectures.

This makes code clean and easier for subsequent modification.

Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com>
Reviewed-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 arch/arm64/kernel/sys.c  |  7 +++----
 include/linux/syscalls.h | 10 ----------
 kernel/exec_domain.c     | 14 +++++++++++++-
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
index d5ffaaab31a7..5c01816d7a77 100644
--- a/arch/arm64/kernel/sys.c
+++ b/arch/arm64/kernel/sys.c
@@ -28,12 +28,13 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
 	return ksys_mmap_pgoff(addr, len, prot, flags, fd, off >> PAGE_SHIFT);
 }
 
-SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
+int arch_check_personality(unsigned int personality)
 {
 	if (personality(personality) == PER_LINUX32 &&
 		!system_supports_32bit_el0())
 		return -EINVAL;
-	return ksys_personality(personality);
+
+	return 0;
 }
 
 asmlinkage long sys_ni_syscall(void);
@@ -46,8 +47,6 @@ asmlinkage long __arm64_sys_ni_syscall(const struct pt_regs *__unused)
 /*
  * Wrappers to pass the pt_regs argument.
  */
-#define __arm64_sys_personality		__arm64_sys_arm64_personality
-
 #undef __SYSCALL
 #define __SYSCALL(nr, sym)	asmlinkage long __arm64_##sym(const struct pt_regs *);
 #include <asm/unistd.h>
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 1815065d52f3..3dbbad498027 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1393,16 +1393,6 @@ static inline long ksys_truncate(const char __user *pathname, loff_t length)
 	return do_sys_truncate(pathname, length);
 }
 
-static inline unsigned int ksys_personality(unsigned int personality)
-{
-	unsigned int old = current->personality;
-
-	if (personality != 0xffffffff)
-		set_personality(personality);
-
-	return old;
-}
-
 /* for __ARCH_WANT_SYS_IPC */
 long ksys_semtimedop(int semid, struct sembuf __user *tsops,
 		     unsigned int nsops,
diff --git a/kernel/exec_domain.c b/kernel/exec_domain.c
index 33f07c5f2515..f3682f4bf205 100644
--- a/kernel/exec_domain.c
+++ b/kernel/exec_domain.c
@@ -35,9 +35,21 @@ static int __init proc_execdomains_init(void)
 module_init(proc_execdomains_init);
 #endif
 
+int __weak arch_check_personality(unsigned int personality)
+{
+	return 0;
+}
+
 SYSCALL_DEFINE1(personality, unsigned int, personality)
 {
-	unsigned int old = current->personality;
+	int err;
+	unsigned int old;
+
+	err = arch_check_personality(personality);
+	if (err)
+		return err;
+
+	old = current->personality;
 
 	if (personality != 0xffffffff)
 		set_personality(personality);
-- 
2.17.1


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

* Re: [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality
  2020-06-08  2:49 [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality Wang ShaoBo
@ 2020-06-08  7:50 ` kernel test robot
  2020-06-08  9:46 ` Catalin Marinas
  2020-06-09  7:25 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-06-08  7:50 UTC (permalink / raw)
  To: Wang ShaoBo
  Cc: kbuild-all, clang-built-linux, huawei.libin, cj.chengjian,
	xiexiuqi, mark.rutland, hch, wcohen, linux-kernel, mtk.manpages,
	wezhang, gregkh

[-- Attachment #1: Type: text/plain, Size: 7196 bytes --]

Hi Wang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on linus/master v5.7 next-20200605]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Wang-ShaoBo/sys_personality-Add-optional-arch-hook-arch_check_personality/20200608-105348
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-r012-20200607 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> arch/arm64/kernel/sys.c:31:5: warning: no previous prototype for function 'arch_check_personality' [-Wmissing-prototypes]
int arch_check_personality(unsigned int personality)
^
arch/arm64/kernel/sys.c:31:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int arch_check_personality(unsigned int personality)
^
static
arch/arm64/kernel/sys.c:42:17: warning: no previous prototype for function '__arm64_sys_ni_syscall' [-Wmissing-prototypes]
asmlinkage long __arm64_sys_ni_syscall(const struct pt_regs *__unused)
^
arch/arm64/kernel/sys.c:42:12: note: declare 'static' if the function is not intended to be used outside of this translation unit
asmlinkage long __arm64_sys_ni_syscall(const struct pt_regs *__unused)
^
static
In file included from arch/arm64/kernel/sys.c:59:
In file included from arch/arm64/include/asm/unistd.h:47:
In file included from arch/arm64/include/uapi/asm/unistd.h:24:
include/uapi/asm-generic/unistd.h:34:1: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
__SC_COMP(__NR_io_setup, sys_io_setup, compat_sys_io_setup)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/uapi/asm-generic/unistd.h:29:37: note: expanded from macro '__SC_COMP'
#define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
^~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: expanded from macro '__SYSCALL'
#define __SYSCALL(nr, sym)      [nr] = __arm64_##sym,
^~~~~~~~~~~~~
<scratch space>:19:1: note: expanded from here
__arm64_sys_io_setup
^~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:58:30: note: previous initialization is here
[0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall,
^~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/kernel/sys.c:59:
In file included from arch/arm64/include/asm/unistd.h:47:
In file included from arch/arm64/include/uapi/asm/unistd.h:24:
include/uapi/asm-generic/unistd.h:36:1: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
__SYSCALL(__NR_io_destroy, sys_io_destroy)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: expanded from macro '__SYSCALL'
#define __SYSCALL(nr, sym)      [nr] = __arm64_##sym,
^~~~~~~~~~~~~
<scratch space>:20:1: note: expanded from here
__arm64_sys_io_destroy
^~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:58:30: note: previous initialization is here
[0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall,
^~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/kernel/sys.c:59:
In file included from arch/arm64/include/asm/unistd.h:47:
In file included from arch/arm64/include/uapi/asm/unistd.h:24:
include/uapi/asm-generic/unistd.h:38:1: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
__SC_COMP(__NR_io_submit, sys_io_submit, compat_sys_io_submit)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/uapi/asm-generic/unistd.h:29:37: note: expanded from macro '__SC_COMP'
#define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
^~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: expanded from macro '__SYSCALL'
#define __SYSCALL(nr, sym)      [nr] = __arm64_##sym,
^~~~~~~~~~~~~
<scratch space>:21:1: note: expanded from here
__arm64_sys_io_submit
^~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:58:30: note: previous initialization is here
[0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall,
^~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/kernel/sys.c:59:
In file included from arch/arm64/include/asm/unistd.h:47:
In file included from arch/arm64/include/uapi/asm/unistd.h:24:
include/uapi/asm-generic/unistd.h:40:1: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
__SYSCALL(__NR_io_cancel, sys_io_cancel)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: expanded from macro '__SYSCALL'
#define __SYSCALL(nr, sym)      [nr] = __arm64_##sym,
^~~~~~~~~~~~~
<scratch space>:22:1: note: expanded from here
__arm64_sys_io_cancel
^~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:58:30: note: previous initialization is here
[0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall,
^~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/kernel/sys.c:59:
In file included from arch/arm64/include/asm/unistd.h:47:
In file included from arch/arm64/include/uapi/asm/unistd.h:24:
include/uapi/asm-generic/unistd.h:43:1: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
__SC_3264(__NR_io_getevents, sys_io_getevents_time32, sys_io_getevents)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/uapi/asm-generic/unistd.h:22:34: note: expanded from macro '__SC_3264'
#define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _64)
^~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: expanded from macro '__SYSCALL'
#define __SYSCALL(nr, sym)      [nr] = __arm64_##sym,
^~~~~~~~~~~~~
<scratch space>:23:1: note: expanded from here
__arm64_sys_io_getevents
^~~~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:58:30: note: previous initialization is here
[0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall,
^~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/kernel/sys.c:59:
In file included from arch/arm64/include/asm/unistd.h:47:
In file included from arch/arm64/include/uapi/asm/unistd.h:24:

vim +/arch_check_personality +31 arch/arm64/kernel/sys.c

    30	
  > 31	int arch_check_personality(unsigned int personality)
    32	{
    33		if (personality(personality) == PER_LINUX32 &&
    34			!system_supports_32bit_el0())
    35			return -EINVAL;
    36	
    37		return 0;
    38	}
    39	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30910 bytes --]

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

* Re: [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality
  2020-06-08  2:49 [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality Wang ShaoBo
  2020-06-08  7:50 ` kernel test robot
@ 2020-06-08  9:46 ` Catalin Marinas
  2020-06-08 13:16   ` Wangshaobo (bobo)
  2020-06-08 14:58   ` Dominik Brodowski
  2020-06-09  7:25 ` kernel test robot
  2 siblings, 2 replies; 6+ messages in thread
From: Catalin Marinas @ 2020-06-08  9:46 UTC (permalink / raw)
  To: Wang ShaoBo
  Cc: huawei.libin, cj.chengjian, xiexiuqi, mark.rutland, hch, wcohen,
	linux-kernel, mtk.manpages, wezhang, gregkh, Will Deacon

On Mon, Jun 08, 2020 at 10:49:25AM +0800, Wang ShaoBo wrote:
> Currently arm64 personality syscall uses wrapper __arm64_sys_personality
> to redirect to __arm64_sys_arm64_personality, it's easily confused,
> Whereas using an normal hook arch_check_personality() can reject
> additional settings like this for special case of different architectures.
> 
> This makes code clean and easier for subsequent modification.

Do you plan to add more stuff here? Curious what triggered this patch.

> diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
> index d5ffaaab31a7..5c01816d7a77 100644
> --- a/arch/arm64/kernel/sys.c
> +++ b/arch/arm64/kernel/sys.c
> @@ -28,12 +28,13 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
>  	return ksys_mmap_pgoff(addr, len, prot, flags, fd, off >> PAGE_SHIFT);
>  }
>  
> -SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
> +int arch_check_personality(unsigned int personality)
>  {
>  	if (personality(personality) == PER_LINUX32 &&
>  		!system_supports_32bit_el0())
>  		return -EINVAL;
> -	return ksys_personality(personality);
> +
> +	return 0;
>  }

We use the ksys_* pattern in other places as well, so this wouldn't be
something new.

> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 1815065d52f3..3dbbad498027 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -1393,16 +1393,6 @@ static inline long ksys_truncate(const char __user *pathname, loff_t length)
>  	return do_sys_truncate(pathname, length);
>  }
>  
> -static inline unsigned int ksys_personality(unsigned int personality)
> -{
> -	unsigned int old = current->personality;
> -
> -	if (personality != 0xffffffff)
> -		set_personality(personality);
> -
> -	return old;
> -}
> -
>  /* for __ARCH_WANT_SYS_IPC */
>  long ksys_semtimedop(int semid, struct sembuf __user *tsops,
>  		     unsigned int nsops,
> diff --git a/kernel/exec_domain.c b/kernel/exec_domain.c
> index 33f07c5f2515..f3682f4bf205 100644
> --- a/kernel/exec_domain.c
> +++ b/kernel/exec_domain.c
> @@ -35,9 +35,21 @@ static int __init proc_execdomains_init(void)
>  module_init(proc_execdomains_init);
>  #endif
>  
> +int __weak arch_check_personality(unsigned int personality)
> +{
> +	return 0;
> +}
> +
>  SYSCALL_DEFINE1(personality, unsigned int, personality)
>  {
> -	unsigned int old = current->personality;
> +	int err;
> +	unsigned int old;
> +
> +	err = arch_check_personality(personality);
> +	if (err)
> +		return err;
> +
> +	old = current->personality;

I'm surprised that the generic sys_personality() doesn't call
ksys_personality() directly but rather duplicates the code.

Anyway, without knowing what else you plan to do with
arch_check_personality(), I don't think it's worth changing. Calling
ksys_personality() directly from sys_personality() would be a good
clean-up though.

-- 
Catalin

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

* Re: [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality
  2020-06-08  9:46 ` Catalin Marinas
@ 2020-06-08 13:16   ` Wangshaobo (bobo)
  2020-06-08 14:58   ` Dominik Brodowski
  1 sibling, 0 replies; 6+ messages in thread
From: Wangshaobo (bobo) @ 2020-06-08 13:16 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: huawei.libin, cj.chengjian, xiexiuqi, mark.rutland, hch, wcohen,
	linux-kernel, mtk.manpages, wezhang, gregkh, Will Deacon


在 2020/6/8 17:46, Catalin Marinas 写道:
> On Mon, Jun 08, 2020 at 10:49:25AM +0800, Wang ShaoBo wrote:
>> Currently arm64 personality syscall uses wrapper __arm64_sys_personality
>> to redirect to __arm64_sys_arm64_personality, it's easily confused,
>> Whereas using an normal hook arch_check_personality() can reject
>> additional settings like this for special case of different architectures.
>>
>> This makes code clean and easier for subsequent modification.
> Do you plan to add more stuff here? Curious what triggered this patch.
>
>> diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
>> index d5ffaaab31a7..5c01816d7a77 100644
>> --- a/arch/arm64/kernel/sys.c
>> +++ b/arch/arm64/kernel/sys.c
>> @@ -28,12 +28,13 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
>>   	return ksys_mmap_pgoff(addr, len, prot, flags, fd, off >> PAGE_SHIFT);
>>   }
>>   
>> -SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
>> +int arch_check_personality(unsigned int personality)
>>   {
>>   	if (personality(personality) == PER_LINUX32 &&
>>   		!system_supports_32bit_el0())
>>   		return -EINVAL;
>> -	return ksys_personality(personality);
>> +
>> +	return 0;
>>   }
> We use the ksys_* pattern in other places as well, so this wouldn't be
> something new.
>
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 1815065d52f3..3dbbad498027 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -1393,16 +1393,6 @@ static inline long ksys_truncate(const char __user *pathname, loff_t length)
>>   	return do_sys_truncate(pathname, length);
>>   }
>>   
>> -static inline unsigned int ksys_personality(unsigned int personality)
>> -{
>> -	unsigned int old = current->personality;
>> -
>> -	if (personality != 0xffffffff)
>> -		set_personality(personality);
>> -
>> -	return old;
>> -}
>> -
>>   /* for __ARCH_WANT_SYS_IPC */
>>   long ksys_semtimedop(int semid, struct sembuf __user *tsops,
>>   		     unsigned int nsops,
>> diff --git a/kernel/exec_domain.c b/kernel/exec_domain.c
>> index 33f07c5f2515..f3682f4bf205 100644
>> --- a/kernel/exec_domain.c
>> +++ b/kernel/exec_domain.c
>> @@ -35,9 +35,21 @@ static int __init proc_execdomains_init(void)
>>   module_init(proc_execdomains_init);
>>   #endif
>>   
>> +int __weak arch_check_personality(unsigned int personality)
>> +{
>> +	return 0;
>> +}
>> +
>>   SYSCALL_DEFINE1(personality, unsigned int, personality)
>>   {
>> -	unsigned int old = current->personality;
>> +	int err;
>> +	unsigned int old;
>> +
>> +	err = arch_check_personality(personality);
>> +	if (err)
>> +		return err;
>> +
>> +	old = current->personality;
> I'm surprised that the generic sys_personality() doesn't call
> ksys_personality() directly but rather duplicates the code.
>
> Anyway, without knowing what else you plan to do with
> arch_check_personality(), I don't think it's worth changing. Calling
> ksys_personality() directly from sys_personality() would be a good
> clean-up though.

Hi catalin,

I have sent a version just calling ksys_personality() directly from 
sys_personality() before:

https://lore.kernel.org/patchwork/patch/1158872/

thanks,

Wang ShaoBo

>


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

* Re: [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality
  2020-06-08  9:46 ` Catalin Marinas
  2020-06-08 13:16   ` Wangshaobo (bobo)
@ 2020-06-08 14:58   ` Dominik Brodowski
  1 sibling, 0 replies; 6+ messages in thread
From: Dominik Brodowski @ 2020-06-08 14:58 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Wang ShaoBo, huawei.libin, cj.chengjian, xiexiuqi, mark.rutland,
	hch, wcohen, linux-kernel, mtk.manpages, wezhang, gregkh,
	Will Deacon

On Mon, Jun 08, 2020 at 10:46:41AM +0100, Catalin Marinas wrote:
> On Mon, Jun 08, 2020 at 10:49:25AM +0800, Wang ShaoBo wrote:
> > Currently arm64 personality syscall uses wrapper __arm64_sys_personality
> > to redirect to __arm64_sys_arm64_personality, it's easily confused,
> > Whereas using an normal hook arch_check_personality() can reject
> > additional settings like this for special case of different architectures.
> > 
> > This makes code clean and easier for subsequent modification.
> 
> Do you plan to add more stuff here? Curious what triggered this patch.
> 
> > diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
> > index d5ffaaab31a7..5c01816d7a77 100644
> > --- a/arch/arm64/kernel/sys.c
> > +++ b/arch/arm64/kernel/sys.c
> > @@ -28,12 +28,13 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
> >  	return ksys_mmap_pgoff(addr, len, prot, flags, fd, off >> PAGE_SHIFT);
> >  }
> >  
> > -SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
> > +int arch_check_personality(unsigned int personality)
> >  {
> >  	if (personality(personality) == PER_LINUX32 &&
> >  		!system_supports_32bit_el0())
> >  		return -EINVAL;
> > -	return ksys_personality(personality);
> > +
> > +	return 0;
> >  }
> 
> We use the ksys_* pattern in other places as well, so this wouldn't be
> something new.
> 
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index 1815065d52f3..3dbbad498027 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -1393,16 +1393,6 @@ static inline long ksys_truncate(const char __user *pathname, loff_t length)
> >  	return do_sys_truncate(pathname, length);
> >  }
> >  
> > -static inline unsigned int ksys_personality(unsigned int personality)
> > -{
> > -	unsigned int old = current->personality;
> > -
> > -	if (personality != 0xffffffff)
> > -		set_personality(personality);
> > -
> > -	return old;
> > -}
> > -
> >  /* for __ARCH_WANT_SYS_IPC */
> >  long ksys_semtimedop(int semid, struct sembuf __user *tsops,
> >  		     unsigned int nsops,
> > diff --git a/kernel/exec_domain.c b/kernel/exec_domain.c
> > index 33f07c5f2515..f3682f4bf205 100644
> > --- a/kernel/exec_domain.c
> > +++ b/kernel/exec_domain.c
> > @@ -35,9 +35,21 @@ static int __init proc_execdomains_init(void)
> >  module_init(proc_execdomains_init);
> >  #endif
> >  
> > +int __weak arch_check_personality(unsigned int personality)
> > +{
> > +	return 0;
> > +}
> > +
> >  SYSCALL_DEFINE1(personality, unsigned int, personality)
> >  {
> > -	unsigned int old = current->personality;
> > +	int err;
> > +	unsigned int old;
> > +
> > +	err = arch_check_personality(personality);
> > +	if (err)
> > +		return err;
> > +
> > +	old = current->personality;
> 
> I'm surprised that the generic sys_personality() doesn't call
> ksys_personality() directly but rather duplicates the code.

It was the other way round, and the duplication is based on a
suggestion by Christoph Hellwig IIRC,
	https://lore.kernel.org/lkml/20180514120756.GA11638@infradead.org/

Thanks,
	Dominik

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

* Re: [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality
  2020-06-08  2:49 [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality Wang ShaoBo
  2020-06-08  7:50 ` kernel test robot
  2020-06-08  9:46 ` Catalin Marinas
@ 2020-06-09  7:25 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-06-09  7:25 UTC (permalink / raw)
  To: Wang ShaoBo
  Cc: kbuild-all, huawei.libin, cj.chengjian, xiexiuqi, mark.rutland,
	hch, wcohen, linux-kernel, mtk.manpages, wezhang, gregkh

[-- Attachment #1: Type: text/plain, Size: 7209 bytes --]

Hi Wang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on linux/master v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Wang-ShaoBo/sys_personality-Add-optional-arch-hook-arch_check_personality/20200608-105348
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-c021-20200608 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> arch/arm64/kernel/sys.c:31:5: warning: no previous prototype for 'arch_check_personality' [-Wmissing-prototypes]
31 | int arch_check_personality(unsigned int personality)
|     ^~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:42:17: warning: no previous prototype for '__arm64_sys_ni_syscall' [-Wmissing-prototypes]
42 | asmlinkage long __arm64_sys_ni_syscall(const struct pt_regs *__unused)
|                 ^~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:29:37: note: in expansion of macro '__SYSCALL'
29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
|                                     ^~~~~~~~~
include/uapi/asm-generic/unistd.h:34:1: note: in expansion of macro '__SC_COMP'
34 | __SC_COMP(__NR_io_setup, sys_io_setup, compat_sys_io_setup)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: (near initialization for 'sys_call_table[0]')
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:29:37: note: in expansion of macro '__SYSCALL'
29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
|                                     ^~~~~~~~~
include/uapi/asm-generic/unistd.h:34:1: note: in expansion of macro '__SC_COMP'
34 | __SC_COMP(__NR_io_setup, sys_io_setup, compat_sys_io_setup)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:36:1: note: in expansion of macro '__SYSCALL'
36 | __SYSCALL(__NR_io_destroy, sys_io_destroy)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: (near initialization for 'sys_call_table[1]')
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:36:1: note: in expansion of macro '__SYSCALL'
36 | __SYSCALL(__NR_io_destroy, sys_io_destroy)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:29:37: note: in expansion of macro '__SYSCALL'
29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
|                                     ^~~~~~~~~
include/uapi/asm-generic/unistd.h:38:1: note: in expansion of macro '__SC_COMP'
38 | __SC_COMP(__NR_io_submit, sys_io_submit, compat_sys_io_submit)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: (near initialization for 'sys_call_table[2]')
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:29:37: note: in expansion of macro '__SYSCALL'
29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
|                                     ^~~~~~~~~
include/uapi/asm-generic/unistd.h:38:1: note: in expansion of macro '__SC_COMP'
38 | __SC_COMP(__NR_io_submit, sys_io_submit, compat_sys_io_submit)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:40:1: note: in expansion of macro '__SYSCALL'
40 | __SYSCALL(__NR_io_cancel, sys_io_cancel)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: (near initialization for 'sys_call_table[3]')
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:40:1: note: in expansion of macro '__SYSCALL'
40 | __SYSCALL(__NR_io_cancel, sys_io_cancel)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:22:34: note: in expansion of macro '__SYSCALL'
22 | #define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _64)
|                                  ^~~~~~~~~
include/uapi/asm-generic/unistd.h:43:1: note: in expansion of macro '__SC_3264'
43 | __SC_3264(__NR_io_getevents, sys_io_getevents_time32, sys_io_getevents)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: (near initialization for 'sys_call_table[4]')
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:22:34: note: in expansion of macro '__SYSCALL'
22 | #define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _64)
|                                  ^~~~~~~~~
include/uapi/asm-generic/unistd.h:43:1: note: in expansion of macro '__SC_3264'
43 | __SC_3264(__NR_io_getevents, sys_io_getevents_time32, sys_io_getevents)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:48:1: note: in expansion of macro '__SYSCALL'
48 | __SYSCALL(__NR_setxattr, sys_setxattr)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: note: (near initialization for 'sys_call_table[5]')
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:48:1: note: in expansion of macro '__SYSCALL'
48 | __SYSCALL(__NR_setxattr, sys_setxattr)
| ^~~~~~~~~
arch/arm64/kernel/sys.c:55:35: warning: initialized field overwritten [-Woverride-init]
55 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
|                                   ^~~~~~~~
include/uapi/asm-generic/unistd.h:50:1: note: in expansion of macro '__SYSCALL'
50 | __SYSCALL(__NR_lsetxattr, sys_lsetxattr)

vim +/arch_check_personality +31 arch/arm64/kernel/sys.c

    30	
  > 31	int arch_check_personality(unsigned int personality)
    32	{
    33		if (personality(personality) == PER_LINUX32 &&
    34			!system_supports_32bit_el0())
    35			return -EINVAL;
    36	
    37		return 0;
    38	}
    39	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33702 bytes --]

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

end of thread, other threads:[~2020-06-09  7:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08  2:49 [RESEND PATCH] sys_personality: Add optional arch hook arch_check_personality Wang ShaoBo
2020-06-08  7:50 ` kernel test robot
2020-06-08  9:46 ` Catalin Marinas
2020-06-08 13:16   ` Wangshaobo (bobo)
2020-06-08 14:58   ` Dominik Brodowski
2020-06-09  7:25 ` 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).