Patchwork [v2] sparc64: do not clobber personality flags in sys_sparc64_personality()

login
register
mail settings
Submitter Jiri Kosina
Date Aug. 2, 2012, 7:12 a.m.
Message ID <alpine.LNX.2.00.1208020912130.14910@pobox.suse.cz>
Download mbox | patch
Permalink /patch/174685/
State Accepted
Delegated to: David Miller
Headers show

Comments

Jiri Kosina - Aug. 2, 2012, 7:12 a.m.
From 4f000eda5917ceecb03767962026cc6a390b8216 Mon Sep 17 00:00:00 2001
From: Jiri Kosina <jkosina@suse.cz>
Date: Wed, 1 Aug 2012 21:10:51 +0200
Subject: [PATCH 4/4] sparc64: do not clobber personality flags in sys_sparc64_personality()

There are multiple errors in how sys_sparc64_personality() handles
personality flags stored in top three bytes.

- directly comparing current->personality against PER_LINUX32 doesn't work
  in cases when any of the personality flags stored in the top three bytes
  are used.
- directly forcefully setting personality to PER_LINUX32 or PER_LINUX
  discards any flags stored in the top three bytes

Fix the first one by properly using personality() macro to compare only
PER_MASK bytes.
Fix the second one by setting only the bits that should be set, instead of
overwriting the whole value.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---

changed since v1: fix the bit ops to reflect the fact that PER_LINUX is 
actually 0

 arch/sparc/kernel/sys_sparc_64.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
David Miller - Aug. 2, 2012, 7:27 a.m.
From: Jiri Kosina <jkosina@suse.cz>
Date: Thu, 2 Aug 2012 09:12:46 +0200 (CEST)

> From 4f000eda5917ceecb03767962026cc6a390b8216 Mon Sep 17 00:00:00 2001
> From: Jiri Kosina <jkosina@suse.cz>
> Date: Wed, 1 Aug 2012 21:10:51 +0200
> Subject: [PATCH 4/4] sparc64: do not clobber personality flags in sys_sparc64_personality()
> 
> There are multiple errors in how sys_sparc64_personality() handles
> personality flags stored in top three bytes.
> 
> - directly comparing current->personality against PER_LINUX32 doesn't work
>   in cases when any of the personality flags stored in the top three bytes
>   are used.
> - directly forcefully setting personality to PER_LINUX32 or PER_LINUX
>   discards any flags stored in the top three bytes
> 
> Fix the first one by properly using personality() macro to compare only
> PER_MASK bytes.
> Fix the second one by setting only the bits that should be set, instead of
> overwriting the whole value.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Applied, thanks a lot Jiri.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - Aug. 2, 2012, 4:19 p.m.
On Thu, Aug 02, 2012 at 09:12:46AM +0200, Jiri Kosina wrote:
> >From 4f000eda5917ceecb03767962026cc6a390b8216 Mon Sep 17 00:00:00 2001
> From: Jiri Kosina <jkosina@suse.cz>
> Date: Wed, 1 Aug 2012 21:10:51 +0200
> Subject: [PATCH 4/4] sparc64: do not clobber personality flags in sys_sparc64_personality()
> 
> There are multiple errors in how sys_sparc64_personality() handles
> personality flags stored in top three bytes.
> 
> - directly comparing current->personality against PER_LINUX32 doesn't work
>   in cases when any of the personality flags stored in the top three bytes
>   are used.
> - directly forcefully setting personality to PER_LINUX32 or PER_LINUX
>   discards any flags stored in the top three bytes
> 
> Fix the first one by properly using personality() macro to compare only
> PER_MASK bytes.
> Fix the second one by setting only the bits that should be set, instead of
> overwriting the whole value.

Hi Jiri.

Can you have a quick look at this too:
arch/sparc/include/asm/elf_32.h:#define SET_PERSONALITY(ex) set_personality(PER_LINUX)

From your other comments it looks wrong. But I have not digged into this.

Thanks,
	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina - Aug. 2, 2012, 10:16 p.m.
On Thu, 2 Aug 2012, Sam Ravnborg wrote:

> > >From 4f000eda5917ceecb03767962026cc6a390b8216 Mon Sep 17 00:00:00 2001
> > From: Jiri Kosina <jkosina@suse.cz>
> > Date: Wed, 1 Aug 2012 21:10:51 +0200
> > Subject: [PATCH 4/4] sparc64: do not clobber personality flags in sys_sparc64_personality()
> > 
> > There are multiple errors in how sys_sparc64_personality() handles
> > personality flags stored in top three bytes.
> > 
> > - directly comparing current->personality against PER_LINUX32 doesn't work
> >   in cases when any of the personality flags stored in the top three bytes
> >   are used.
> > - directly forcefully setting personality to PER_LINUX32 or PER_LINUX
> >   discards any flags stored in the top three bytes
> > 
> > Fix the first one by properly using personality() macro to compare only
> > PER_MASK bytes.
> > Fix the second one by setting only the bits that should be set, instead of
> > overwriting the whole value.
> 
> Hi Jiri.
> 
> Can you have a quick look at this too:
> arch/sparc/include/asm/elf_32.h:#define SET_PERSONALITY(ex) set_personality(PER_LINUX)
> 
> From your other comments it looks wrong. But I have not digged into 
> this.

It actually seems like much more architectures than just sparc are 
clobbering upper bytes of personality upon exec(). Thanks for pointing 
that out, it'd be worthwile to go over those and fix them up. I am adding 
that to my TODO.

Thanks,

Patch

diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index 0dc1f57..11c6c96 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -502,12 +502,12 @@  SYSCALL_DEFINE1(sparc64_personality, unsigned long, personality)
 {
 	int ret;
 
-	if (current->personality == PER_LINUX32 &&
-	    personality == PER_LINUX)
-		personality = PER_LINUX32;
+	if (personality(current->personality) == PER_LINUX32 &&
+	    personality(personality) == PER_LINUX)
+		personality |= PER_LINUX32;
 	ret = sys_personality(personality);
-	if (ret == PER_LINUX32)
-		ret = PER_LINUX;
+	if (personality(ret) == PER_LINUX32)
+		ret &= ~PER_LINUX32;
 
 	return ret;
 }