All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
@ 2015-08-18 10:25 ` Nikola Veljkovic
  0 siblings, 0 replies; 8+ messages in thread
From: Nikola Veljkovic @ 2015-08-18 10:25 UTC (permalink / raw)
  To: linux-mips
  Cc: ralf, Chris Dearman, Raghu Gandham, Miodrag Dinic,
	Petar Jovanovic, Lazar Trsic

There is a discrepancy in the personality() syscall on mips64.

For a 32bit process (o32 or n32) sys_32_personality() [1] is called, which
converts PER_LINUX32 -> PER_LINUX:

ret = sys_personality(p);
if (ret != -1 && (ret & PER_MASK) == PER_LINUX32)  // personality macro applied
   ret = (ret & ~PER_MASK) | PER_LINUX;            // to make it clearer

This is different than what Intel and ARM are doing. They directly invoke
sys_personality(). On Android, this causes a bionic test to fail for mips, as it
expects PER_LINUX32 but receives PER_LINUX, when ran on mips64 core.

The code itself comes from older kernel versions (2.4), where it was also used
by other architectures [2], [3], [4]. However, since kernel 2.6 other archs are
invoking sys_personality() directly, for all ABIs.

Is there any reason why we should do PER_LINUX32 -> PER_LINUX on mips? Also, if
we do change this for o32, should we change it for n32, as well? There is no n32
support for Android, so it would have to be tested elsewhere.

Tested changing only o32 on Android, there were no regressions:

diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index ab93427..b94ee4e 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -358,7 +358,7 @@ sys_call_table:
        sys     sys_fchdir              1
        sys     sys_bdflush             2
        sys     sys_sysfs               3       /* 4135 */
-       sys     sys_32_personality      1
+       sys     sys_personality         1
        sys     sys_ni_syscall          0       /* for afs_syscall */
        sys     sys_setfsuid            1
        sys     sys_setfsgid            1
--

[1] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/mips/kernel/linux32.c#n122
[2] http://git.linux-mips.org/cgit/ralf/linux.git/tree/include/asm-mips64/elf.h?h=linux-2.4#n124
[3] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/x86_64/ia32/sys_ia32.c?h=linux-2.4#n1974
[4] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/ia64/ia32/sys_ia32.c?h=linux-2.4#n4005
--
Nikola Veljkovic
From Markos.Chandras@imgtec.com Tue Aug 18 14:56:12 2015
Received: with ECARTIS (v1.0.0; list linux-mips); Tue, 18 Aug 2015 14:56:14 +0200 (CEST)
Received: from mailapp01.imgtec.com ([195.59.15.196]:8845 "EHLO
        mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org
        with ESMTP id S27012310AbbHRM4MYiHU- (ORCPT
        <rfc822;linux-mips@linux-mips.org>); Tue, 18 Aug 2015 14:56:12 +0200
Received: from KLMAIL01.kl.imgtec.org (unknown [192.168.5.35])
        by Websense Email Security Gateway with ESMTPS id 356EA9FEB21C2;
        Tue, 18 Aug 2015 13:56:04 +0100 (IST)
Received: from LEMAIL01.le.imgtec.org (192.168.152.62) by
 KLMAIL01.kl.imgtec.org (192.168.5.35) with Microsoft SMTP Server (TLS) id
 14.3.195.1; Tue, 18 Aug 2015 13:56:06 +0100
Received: from localhost (192.168.154.168) by LEMAIL01.le.imgtec.org
 (192.168.152.62) with Microsoft SMTP Server (TLS) id 14.3.210.2; Tue, 18 Aug
 2015 13:56:05 +0100
Date:   Tue, 18 Aug 2015 13:56:05 +0100
From:   Markos Chandras <markos.chandras@imgtec.com>
To:     Nikola Veljkovic <Nikola.Veljkovic@imgtec.com>
CC:     "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>,
        "ralf@linux-mips.org" <ralf@linux-mips.org>,
        Chris Dearman <Chris.Dearman@imgtec.com>,
        Raghu Gandham <Raghu.Gandham@imgtec.com>,
        "Miodrag Dinic" <Miodrag.Dinic@imgtec.com>,
        Petar Jovanovic <Petar.Jovanovic@imgtec.com>,
        Lazar Trsic <Lazar.Trsic@imgtec.com>
Subject: Re: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
Message-ID: <20150818125605.GA25978@mchandras-linux.le.imgtec.org>
References: <19CDB9880DBFC241860D925D8FAA94A00342EAB8@BADAG02.ba.imgtec.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <19CDB9880DBFC241860D925D8FAA94A00342EAB8@BADAG02.ba.imgtec.org>
User-Agent: Mutt/1.5.23 (2014-03-12)
X-Originating-IP: [192.168.154.168]
Return-Path: <Markos.Chandras@imgtec.com>
X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0)
X-Orcpt: rfc822;linux-mips@linux-mips.org
Original-Recipient: rfc822;linux-mips@linux-mips.org
X-archive-position: 48938
X-ecartis-version: Ecartis v1.0.0
Sender: linux-mips-bounce@linux-mips.org
Errors-to: linux-mips-bounce@linux-mips.org
X-original-sender: markos.chandras@imgtec.com
Precedence: bulk
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
List-software: Ecartis version 1.0.0
List-Id: linux-mips <linux-mips.eddie.linux-mips.org>
X-List-ID: linux-mips <linux-mips.eddie.linux-mips.org>
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
X-list: linux-mips
Content-Length: 2753
Lines: 59

On Tue, Aug 18, 2015 at 10:25:31AM +0000, Nikola Veljkovic wrote:
> There is a discrepancy in the personality() syscall on mips64.
> 
> For a 32bit process (o32 or n32) sys_32_personality() [1] is called, which
> converts PER_LINUX32 -> PER_LINUX:
> 
> ret = sys_personality(p);
> if (ret != -1 && (ret & PER_MASK) == PER_LINUX32)  // personality macro applied
>    ret = (ret & ~PER_MASK) | PER_LINUX;            // to make it clearer
> 
> This is different than what Intel and ARM are doing. They directly invoke
> sys_personality(). On Android, this causes a bionic test to fail for mips, as it
> expects PER_LINUX32 but receives PER_LINUX, when ran on mips64 core.
> 
> The code itself comes from older kernel versions (2.4), where it was also used
> by other architectures [2], [3], [4]. However, since kernel 2.6 other archs are
> invoking sys_personality() directly, for all ABIs.
> 
> Is there any reason why we should do PER_LINUX32 -> PER_LINUX on mips? Also, if
> we do change this for o32, should we change it for n32, as well? There is no n32
> support for Android, so it would have to be tested elsewhere.
> 
> Tested changing only o32 on Android, there were no regressions:
> 
> diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
> index ab93427..b94ee4e 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -358,7 +358,7 @@ sys_call_table:
>         sys     sys_fchdir              1
>         sys     sys_bdflush             2
>         sys     sys_sysfs               3       /* 4135 */
> -       sys     sys_32_personality      1
> +       sys     sys_personality         1
>         sys     sys_ni_syscall          0       /* for afs_syscall */
>         sys     sys_setfsuid            1
>         sys     sys_setfsgid            1
> --
> 
> [1] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/mips/kernel/linux32.c#n122
> [2] http://git.linux-mips.org/cgit/ralf/linux.git/tree/include/asm-mips64/elf.h?h=linux-2.4#n124
> [3] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/x86_64/ia32/sys_ia32.c?h=linux-2.4#n1974
> [4] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/ia64/ia32/sys_ia32.c?h=linux-2.4#n4005
> --
> Nikola Veljkovic

Hi,

I am not sure what the implication will be to userland with that change...

I presume you also need to fix the n32 case and then completely remove the sys_32_personality
syscall from linux32.c. Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)

If you are going to submit a new patch, can you please create a proper git patch
(using git format-patch) and include your Signed-off-by line as well? Thank you

-- 
markos

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

* [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
@ 2015-08-18 10:25 ` Nikola Veljkovic
  0 siblings, 0 replies; 8+ messages in thread
From: Nikola Veljkovic @ 2015-08-18 10:25 UTC (permalink / raw)
  To: linux-mips
  Cc: ralf, Chris Dearman, Raghu Gandham, Miodrag Dinic,
	Petar Jovanovic, Lazar Trsic

There is a discrepancy in the personality() syscall on mips64.

For a 32bit process (o32 or n32) sys_32_personality() [1] is called, which
converts PER_LINUX32 -> PER_LINUX:

ret = sys_personality(p);
if (ret != -1 && (ret & PER_MASK) == PER_LINUX32)  // personality macro applied
   ret = (ret & ~PER_MASK) | PER_LINUX;            // to make it clearer

This is different than what Intel and ARM are doing. They directly invoke
sys_personality(). On Android, this causes a bionic test to fail for mips, as it
expects PER_LINUX32 but receives PER_LINUX, when ran on mips64 core.

The code itself comes from older kernel versions (2.4), where it was also used
by other architectures [2], [3], [4]. However, since kernel 2.6 other archs are
invoking sys_personality() directly, for all ABIs.

Is there any reason why we should do PER_LINUX32 -> PER_LINUX on mips? Also, if
we do change this for o32, should we change it for n32, as well? There is no n32
support for Android, so it would have to be tested elsewhere.

Tested changing only o32 on Android, there were no regressions:

diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index ab93427..b94ee4e 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -358,7 +358,7 @@ sys_call_table:
        sys     sys_fchdir              1
        sys     sys_bdflush             2
        sys     sys_sysfs               3       /* 4135 */
-       sys     sys_32_personality      1
+       sys     sys_personality         1
        sys     sys_ni_syscall          0       /* for afs_syscall */
        sys     sys_setfsuid            1
        sys     sys_setfsgid            1
--

[1] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/mips/kernel/linux32.c#n122
[2] http://git.linux-mips.org/cgit/ralf/linux.git/tree/include/asm-mips64/elf.h?h=linux-2.4#n124
[3] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/x86_64/ia32/sys_ia32.c?h=linux-2.4#n1974
[4] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/ia64/ia32/sys_ia32.c?h=linux-2.4#n4005
--
Nikola Veljkovic

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

* Re: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
  2015-08-18 10:25 ` Nikola Veljkovic
  (?)
@ 2015-08-18 12:56 ` Markos Chandras
  2015-08-20 14:46     ` Nikola Veljkovic
  -1 siblings, 1 reply; 8+ messages in thread
From: Markos Chandras @ 2015-08-18 12:56 UTC (permalink / raw)
  To: Nikola Veljkovic
  Cc: linux-mips, ralf, Chris Dearman, Raghu Gandham, Miodrag Dinic,
	Petar Jovanovic, Lazar Trsic

On Tue, Aug 18, 2015 at 10:25:31AM +0000, Nikola Veljkovic wrote:
> There is a discrepancy in the personality() syscall on mips64.
> 
> For a 32bit process (o32 or n32) sys_32_personality() [1] is called, which
> converts PER_LINUX32 -> PER_LINUX:
> 
> ret = sys_personality(p);
> if (ret != -1 && (ret & PER_MASK) == PER_LINUX32)  // personality macro applied
>    ret = (ret & ~PER_MASK) | PER_LINUX;            // to make it clearer
> 
> This is different than what Intel and ARM are doing. They directly invoke
> sys_personality(). On Android, this causes a bionic test to fail for mips, as it
> expects PER_LINUX32 but receives PER_LINUX, when ran on mips64 core.
> 
> The code itself comes from older kernel versions (2.4), where it was also used
> by other architectures [2], [3], [4]. However, since kernel 2.6 other archs are
> invoking sys_personality() directly, for all ABIs.
> 
> Is there any reason why we should do PER_LINUX32 -> PER_LINUX on mips? Also, if
> we do change this for o32, should we change it for n32, as well? There is no n32
> support for Android, so it would have to be tested elsewhere.
> 
> Tested changing only o32 on Android, there were no regressions:
> 
> diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
> index ab93427..b94ee4e 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -358,7 +358,7 @@ sys_call_table:
>         sys     sys_fchdir              1
>         sys     sys_bdflush             2
>         sys     sys_sysfs               3       /* 4135 */
> -       sys     sys_32_personality      1
> +       sys     sys_personality         1
>         sys     sys_ni_syscall          0       /* for afs_syscall */
>         sys     sys_setfsuid            1
>         sys     sys_setfsgid            1
> --
> 
> [1] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/mips/kernel/linux32.c#n122
> [2] http://git.linux-mips.org/cgit/ralf/linux.git/tree/include/asm-mips64/elf.h?h=linux-2.4#n124
> [3] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/x86_64/ia32/sys_ia32.c?h=linux-2.4#n1974
> [4] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/ia64/ia32/sys_ia32.c?h=linux-2.4#n4005
> --
> Nikola Veljkovic

Hi,

I am not sure what the implication will be to userland with that change...

I presume you also need to fix the n32 case and then completely remove the sys_32_personality
syscall from linux32.c. Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)

If you are going to submit a new patch, can you please create a proper git patch
(using git format-patch) and include your Signed-off-by line as well? Thank you

-- 
markos

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

* RE: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
@ 2015-08-20 14:46     ` Nikola Veljkovic
  0 siblings, 0 replies; 8+ messages in thread
From: Nikola Veljkovic @ 2015-08-20 14:46 UTC (permalink / raw)
  To: Markos Chandras
  Cc: linux-mips, ralf, Chris Dearman, Raghu Gandham, Miodrag Dinic,
	Petar Jovanovic, Lazar Trsic

Hi,

 > I am not sure what the implication will be to userland with that change...
I could not find any code that explicitly checks for PER_LINUX[32]. Of course,
I might be missing something. Any suspects?

 > I presume you also need to fix the n32 case and then completely remove the 
 > sys_32_personality syscall from linux32.c. 
New patch below removes the syscall.

 > Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
 > code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)
Other archs (arm, intel) do not do that, so I think we should follow, and set 
PER_LINUX, but let userspace change this if it wants to. On the kernel side this
only affects uname. From the discussion [1] it seems like the right way to go.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/114506.html

 > If you are going to submit a new patch, can you please create a proper git patch
 > (using git format-patch) and include your Signed-off-by line as well? Thank you
If not obvious, my first post here, thanks for the tips.
------------------
From 6ce4addd17a333ad9935c1d6dc9fa9114e02b3fd Mon Sep 17 00:00:00 2001
From: Nikola Veljkovic <Nikola.Veljkovic@imgtec.com>
Date: Thu, 30 Jul 2015 13:54:26 +0200
Subject: [PATCH] Fix personality syscall for mips64-o32/n32
Content-Length: 5435
Lines: 139

Remove unnecessary sys_32_personality wrapper, which replaces
PER_LINUX32 with PER_LINUX.

Most other archs removed the wrapper even before 2.6.

Now that exec domains are gone, there is almost no kernel code using
personality flags - so set PER_LINUX and let userspace change it.

This fixes bionic test on Android, by making mips more like other
architectures. Change has been tested on mips64 (n64 and o32), there
were no regressions.

Signed-off-by: Nikola Veljkovic <Nikola.Veljkovic@imgtec.com>
---
 arch/mips/kernel/linux32.c     | 14 --------------
 arch/mips/kernel/scall64-n32.S |  2 +-
 arch/mips/kernel/scall64-o32.S |  2 +-
 3 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/mips/kernel/linux32.c b/arch/mips/kernel/linux32.c
index 0b29646..0513e19 100644
--- a/arch/mips/kernel/linux32.c
+++ b/arch/mips/kernel/linux32.c
@@ -119,20 +119,6 @@ SYSCALL_DEFINE6(32_pwrite, unsigned int, fd, const char __user *, buf,
 	return sys_pwrite64(fd, buf, count, merge_64(a4, a5));
 }
 
-SYSCALL_DEFINE1(32_personality, unsigned long, personality)
-{
-	unsigned int p = personality & 0xffffffff;
-	int ret;
-
-	if (personality(current->personality) == PER_LINUX32 &&
-	    personality(p) == PER_LINUX)
-		p = (p & ~PER_MASK) | PER_LINUX32;
-	ret = sys_personality(p);
-	if (ret != -1 && personality(ret) == PER_LINUX32)
-		ret = (ret & ~PER_MASK) | PER_LINUX;
-	return ret;
-}
-
 asmlinkage ssize_t sys32_readahead(int fd, u32 pad0, u64 a2, u64 a3,
 				   size_t count)
 {
diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
index 4b20106..e4c036c 100644
--- a/arch/mips/kernel/scall64-n32.S
+++ b/arch/mips/kernel/scall64-n32.S
@@ -240,7 +240,7 @@ EXPORT(sysn32_call_table)
 	PTR	compat_sys_sigaltstack
 	PTR	compat_sys_utime		/* 6130 */
 	PTR	sys_mknod
-	PTR	sys_32_personality
+	PTR	sys_personality
 	PTR	compat_sys_ustat
 	PTR	compat_sys_statfs
 	PTR	compat_sys_fstatfs		/* 6135 */
diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index f543ff4..9b3e881 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -363,7 +363,7 @@ EXPORT(sys32_call_table)
 	PTR	sys_fchdir
 	PTR	sys_bdflush
 	PTR	sys_sysfs			/* 4135 */
-	PTR	sys_32_personality
+	PTR	sys_personality
 	PTR	sys_ni_syscall			/* for afs_syscall */
 	PTR	sys_setfsuid
 	PTR	sys_setfsgid
-- 
2.4.0

________________________________________
From: Markos Chandras
Sent: Tuesday, August 18, 2015 2:56 PM
To: Nikola Veljkovic
Cc: linux-mips@linux-mips.org; ralf@linux-mips.org; Chris Dearman; Raghu Gandham; Miodrag Dinic; Petar Jovanovic; Lazar Trsic
Subject: Re: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32

On Tue, Aug 18, 2015 at 10:25:31AM +0000, Nikola Veljkovic wrote:
> There is a discrepancy in the personality() syscall on mips64.
>
> For a 32bit process (o32 or n32) sys_32_personality() [1] is called, which
> converts PER_LINUX32 -> PER_LINUX:
>
> ret = sys_personality(p);
> if (ret != -1 && (ret & PER_MASK) == PER_LINUX32)  // personality macro applied
>    ret = (ret & ~PER_MASK) | PER_LINUX;            // to make it clearer
>
> This is different than what Intel and ARM are doing. They directly invoke
> sys_personality(). On Android, this causes a bionic test to fail for mips, as it
> expects PER_LINUX32 but receives PER_LINUX, when ran on mips64 core.
>
> The code itself comes from older kernel versions (2.4), where it was also used
> by other architectures [2], [3], [4]. However, since kernel 2.6 other archs are
> invoking sys_personality() directly, for all ABIs.
>
> Is there any reason why we should do PER_LINUX32 -> PER_LINUX on mips? Also, if
> we do change this for o32, should we change it for n32, as well? There is no n32
> support for Android, so it would have to be tested elsewhere.
>
> Tested changing only o32 on Android, there were no regressions:
>
> diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
> index ab93427..b94ee4e 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -358,7 +358,7 @@ sys_call_table:
>         sys     sys_fchdir              1
>         sys     sys_bdflush             2
>         sys     sys_sysfs               3       /* 4135 */
> -       sys     sys_32_personality      1
> +       sys     sys_personality         1
>         sys     sys_ni_syscall          0       /* for afs_syscall */
>         sys     sys_setfsuid            1
>         sys     sys_setfsgid            1
> --
>
> [1] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/mips/kernel/linux32.c#n122
> [2] http://git.linux-mips.org/cgit/ralf/linux.git/tree/include/asm-mips64/elf.h?h=linux-2.4#n124
> [3] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/x86_64/ia32/sys_ia32.c?h=linux-2.4#n1974
> [4] http://git.linux-mips.org/cgit/ralf/linux.git/tree/arch/ia64/ia32/sys_ia32.c?h=linux-2.4#n4005
> --
> Nikola Veljkovic

Hi,

I am not sure what the implication will be to userland with that change...

I presume you also need to fix the n32 case and then completely remove the sys_32_personality
syscall from linux32.c. Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)

If you are going to submit a new patch, can you please create a proper git patch
(using git format-patch) and include your Signed-off-by line as well? Thank you

--
markos

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

* RE: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
@ 2015-08-20 14:46     ` Nikola Veljkovic
  0 siblings, 0 replies; 8+ messages in thread
From: Nikola Veljkovic @ 2015-08-20 14:46 UTC (permalink / raw)
  To: Markos Chandras
  Cc: linux-mips, ralf, Chris Dearman, Raghu Gandham, Miodrag Dinic,
	Petar Jovanovic, Lazar Trsic

Hi,

 > I am not sure what the implication will be to userland with that change...
I could not find any code that explicitly checks for PER_LINUX[32]. Of course,
I might be missing something. Any suspects?

 > I presume you also need to fix the n32 case and then completely remove the 
 > sys_32_personality syscall from linux32.c. 
New patch below removes the syscall.

 > Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
 > code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)
Other archs (arm, intel) do not do that, so I think we should follow, and set 
PER_LINUX, but let userspace change this if it wants to. On the kernel side this
only affects uname. From the discussion [1] it seems like the right way to go.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/114506.html

 > If you are going to submit a new patch, can you please create a proper git patch
 > (using git format-patch) and include your Signed-off-by line as well? Thank you
If not obvious, my first post here, thanks for the tips.
------------------

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

* Re: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
  2015-08-20 14:46     ` Nikola Veljkovic
  (?)
@ 2015-11-12 16:25     ` Ralf Baechle
  2015-11-12 19:50         ` Nikola Veljkovic
  -1 siblings, 1 reply; 8+ messages in thread
From: Ralf Baechle @ 2015-11-12 16:25 UTC (permalink / raw)
  To: Nikola Veljkovic
  Cc: Markos Chandras, linux-mips, Chris Dearman, Raghu Gandham,
	Miodrag Dinic, Petar Jovanovic, Lazar Trsic

On Thu, Aug 20, 2015 at 02:46:26PM +0000, Nikola Veljkovic wrote:

>  > I am not sure what the implication will be to userland with that change...
> I could not find any code that explicitly checks for PER_LINUX[32]. Of course,
> I might be missing something. Any suspects?

The one thing where it really matters is the architecture return value
for uname, try "uname -m".  On a 32 bit kernel this will return "mips",
on a 64 bit kernel "mips64".  This will break some software that expects
to identify MIPS by "mips".  There's a small tool to set this personality
flag, see ftp://ftp.linux-mips.org/pub/linux/mips/mips32/ for a year 2000
vintage source and binary rpm.  Or also the more modern setarch(1) utility.

>  > I presume you also need to fix the n32 case and then completely remove the 
>  > sys_32_personality syscall from linux32.c. 
> New patch below removes the syscall.
> 
>  > Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
>  > code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)
> Other archs (arm, intel) do not do that, so I think we should follow, and set 
> PER_LINUX, but let userspace change this if it wants to. On the kernel side this
> only affects uname. From the discussion [1] it seems like the right way to go.
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/114506.html

x86 and ARM maybe but not SPARC64 which uses the exactly same code,
see arch/sparc/kernel/sys_sparc_64.c function sparc64_personality() and
from which this function and the mips32 utility which was named sparc32
in a former live, were taken.

The PER_LINUX32 -> PER_LINUX translation of the return value is for antique
programs that expect to see PER_LINUX as the return value.

> Tested changing only o32 on Android, there were no regressions:

For anything that changes an established ABI - in this case for about 15 or
16 years I'm going to be conservative at changing such an ABI.  Also Sparc64
and PowerPC are using the exactly same wrapper.

Anyway, can you tell me more about the Android issue?

  Ralf

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

* RE: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
@ 2015-11-12 19:50         ` Nikola Veljkovic
  0 siblings, 0 replies; 8+ messages in thread
From: Nikola Veljkovic @ 2015-11-12 19:50 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Markos Chandras, linux-mips, Chris Dearman, Raghu Gandham,
	Miodrag Dinic, Petar Jovanovic, Lazar Trsic

>For anything that changes an established ABI - in this case for about 15 or
>16 years I'm going to be conservative at changing such an ABI.  Also Sparc64
>and PowerPC are using the exactly same wrapper.

>Anyway, can you tell me more about the Android issue?

I started the discussion after Google updated personality test in bionic [1].
Test expects the PER_LINUX32 to be returned for a 32-bit binary, so it fails
on mips64 for 32bit binaries.
https://android-review.googlesource.com/#/c/157455/1/tests/sys_personality_test.cpp

My guess was that the changes in test were going to be followed with the code
dependent on the personality syscall elsewhere, but so far that did not happen.
Only user of the personality syscall in Android is strace, and there are no 
issues with it as far as I know.

Also, my understanding of the risks the change in kernel would create was wrong.
From this perspective I believe your arguments against the kernel change to be
sufficient for me to propose to Google a change in the personality test.

Thank you for the additional info,
Nikola
________________________________________
From: Ralf Baechle [ralf@linux-mips.org]
Sent: Thursday, November 12, 2015 5:25 PM
To: Nikola Veljkovic
Cc: Markos Chandras; linux-mips@linux-mips.org; Chris Dearman; Raghu Gandham; Miodrag Dinic; Petar Jovanovic; Lazar Trsic
Subject: Re: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32

On Thu, Aug 20, 2015 at 02:46:26PM +0000, Nikola Veljkovic wrote:

>  > I am not sure what the implication will be to userland with that change...
> I could not find any code that explicitly checks for PER_LINUX[32]. Of course,
> I might be missing something. Any suspects?

The one thing where it really matters is the architecture return value
for uname, try "uname -m".  On a 32 bit kernel this will return "mips",
on a 64 bit kernel "mips64".  This will break some software that expects
to identify MIPS by "mips".  There's a small tool to set this personality
flag, see ftp://ftp.linux-mips.org/pub/linux/mips/mips32/ for a year 2000
vintage source and binary rpm.  Or also the more modern setarch(1) utility.

>  > I presume you also need to fix the n32 case and then completely remove the
>  > sys_32_personality syscall from linux32.c.
> New patch below removes the syscall.
>
>  > Moreover, I think you also need to fix the arch/mips/include/asm/elf.h
>  > code to set LINUX32 for O32 and n32 (for both 32b and 64b kernels.)
> Other archs (arm, intel) do not do that, so I think we should follow, and set
> PER_LINUX, but let userspace change this if it wants to. On the kernel side this
> only affects uname. From the discussion [1] it seems like the right way to go.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/114506.html

x86 and ARM maybe but not SPARC64 which uses the exactly same code,
see arch/sparc/kernel/sys_sparc_64.c function sparc64_personality() and
from which this function and the mips32 utility which was named sparc32
in a former live, were taken.

The PER_LINUX32 -> PER_LINUX translation of the return value is for antique
programs that expect to see PER_LINUX as the return value.

> Tested changing only o32 on Android, there were no regressions:

For anything that changes an established ABI - in this case for about 15 or
16 years I'm going to be conservative at changing such an ABI.  Also Sparc64
and PowerPC are using the exactly same wrapper.

Anyway, can you tell me more about the Android issue?

  Ralf

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

* RE: [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32
@ 2015-11-12 19:50         ` Nikola Veljkovic
  0 siblings, 0 replies; 8+ messages in thread
From: Nikola Veljkovic @ 2015-11-12 19:50 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Markos Chandras, linux-mips, Chris Dearman, Raghu Gandham,
	Miodrag Dinic, Petar Jovanovic, Lazar Trsic

>For anything that changes an established ABI - in this case for about 15 or
>16 years I'm going to be conservative at changing such an ABI.  Also Sparc64
>and PowerPC are using the exactly same wrapper.

>Anyway, can you tell me more about the Android issue?

I started the discussion after Google updated personality test in bionic [1].
Test expects the PER_LINUX32 to be returned for a 32-bit binary, so it fails
on mips64 for 32bit binaries.
https://android-review.googlesource.com/#/c/157455/1/tests/sys_personality_test.cpp

My guess was that the changes in test were going to be followed with the code
dependent on the personality syscall elsewhere, but so far that did not happen.
Only user of the personality syscall in Android is strace, and there are no 
issues with it as far as I know.

Also, my understanding of the risks the change in kernel would create was wrong.

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

end of thread, other threads:[~2015-11-12 19:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-18 10:25 [PATCH] MIPS: personality syscall discrepancy on mips64-o32/n32 Nikola Veljkovic
2015-08-18 10:25 ` Nikola Veljkovic
2015-08-18 12:56 ` Markos Chandras
2015-08-20 14:46   ` Nikola Veljkovic
2015-08-20 14:46     ` Nikola Veljkovic
2015-11-12 16:25     ` Ralf Baechle
2015-11-12 19:50       ` Nikola Veljkovic
2015-11-12 19:50         ` Nikola Veljkovic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.