* [PATCH] powerpc: fix personality handling in ppc64_personality()
@ 2012-08-01 20:03 Jiri Kosina
2012-08-01 22:19 ` Andreas Schwab
0 siblings, 1 reply; 7+ messages in thread
From: Jiri Kosina @ 2012-08-01 20:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev, linux-kernel
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
Use personality() macro to compare only PER_MASK bytes and make sure that
we are setting only the bits that should be set, instead of
overwriting the whole value.
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
Found accidentally. Untested, I don't have the hardware.
arch/powerpc/kernel/syscalls.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index f2496f2..4dcc7c6 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -107,11 +107,11 @@ long ppc64_personality(unsigned long personality)
long ret;
if (personality(current->personality) == PER_LINUX32
- && personality == PER_LINUX)
- personality = PER_LINUX32;
+ && personality(personality) == PER_LINUX)
+ personality &= ~PER_LINUX | PER_LINUX32;
ret = sys_personality(personality);
- if (ret == PER_LINUX32)
- ret = PER_LINUX;
+ if (personality(ret) == PER_LINUX32)
+ ret &= ~PER_LINUX32 | PER_LINUX;
return ret;
}
#endif
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc: fix personality handling in ppc64_personality()
2012-08-01 20:03 [PATCH] powerpc: fix personality handling in ppc64_personality() Jiri Kosina
@ 2012-08-01 22:19 ` Andreas Schwab
2012-08-02 7:10 ` [PATCH v2] " Jiri Kosina
0 siblings, 1 reply; 7+ messages in thread
From: Andreas Schwab @ 2012-08-01 22:19 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, linux-kernel
Jiri Kosina <jkosina@suse.cz> writes:
> if (personality(current->personality) == PER_LINUX32
> - && personality == PER_LINUX)
> - personality = PER_LINUX32;
> + && personality(personality) == PER_LINUX)
> + personality &= ~PER_LINUX | PER_LINUX32;
That doesn't work. ~PER_LINUX is -1, so this is a no-op.
> ret = sys_personality(personality);
> - if (ret == PER_LINUX32)
> - ret = PER_LINUX;
> + if (personality(ret) == PER_LINUX32)
> + ret &= ~PER_LINUX32 | PER_LINUX;
That only "works" because PER_LINUX is 0.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] powerpc: fix personality handling in ppc64_personality()
2012-08-01 22:19 ` Andreas Schwab
@ 2012-08-02 7:10 ` Jiri Kosina
2012-09-05 2:54 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 7+ messages in thread
From: Jiri Kosina @ 2012-08-02 7:10 UTC (permalink / raw)
To: Andreas Schwab
Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, linux-kernel
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
Use personality() macro to compare only PER_MASK bytes and make sure that
we are 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/powerpc/kernel/syscalls.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index f2496f2..dc1558e 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -107,11 +107,11 @@ long ppc64_personality(unsigned long personality)
long ret;
if (personality(current->personality) == PER_LINUX32
- && personality == PER_LINUX)
- 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;
}
#endif
--
1.7.3.1
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc: fix personality handling in ppc64_personality()
2012-08-02 7:10 ` [PATCH v2] " Jiri Kosina
@ 2012-09-05 2:54 ` Benjamin Herrenschmidt
2012-09-05 8:56 ` Jiri Kosina
0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-05 2:54 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Andreas Schwab, Paul Mackerras, linuxppc-dev, linux-kernel
On Thu, 2012-08-02 at 09:10 +0200, Jiri Kosina wrote:
> 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
>
> Use personality() macro to compare only PER_MASK bytes and make sure that
> we are 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
Had already merged v1 (oops.. didn't spot the issue with PER_LINUX being
0). Can you send an incremental fixup ?
Cheers,
Ben.
> arch/powerpc/kernel/syscalls.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
> index f2496f2..dc1558e 100644
> --- a/arch/powerpc/kernel/syscalls.c
> +++ b/arch/powerpc/kernel/syscalls.c
> @@ -107,11 +107,11 @@ long ppc64_personality(unsigned long personality)
> long ret;
>
> if (personality(current->personality) == PER_LINUX32
> - && personality == PER_LINUX)
> - 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;
> }
> #endif
> --
> 1.7.3.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc: fix personality handling in ppc64_personality()
2012-09-05 2:54 ` Benjamin Herrenschmidt
@ 2012-09-05 8:56 ` Jiri Kosina
2012-09-05 21:13 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 7+ messages in thread
From: Jiri Kosina @ 2012-09-05 8:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Andreas Schwab, Paul Mackerras, linuxppc-dev, linux-kernel
On Wed, 5 Sep 2012, Benjamin Herrenschmidt wrote:
> > 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
> >
> > Use personality() macro to compare only PER_MASK bytes and make sure that
> > we are 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
>
> Had already merged v1 (oops.. didn't spot the issue with PER_LINUX being
> 0). Can you send an incremental fixup ?
Hi Benjamin,
actually commit 7256a5d2da56 seems to contain the correct PER_LINUX
handling, so seems like you picked the right one :)
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc: fix personality handling in ppc64_personality()
2012-09-05 8:56 ` Jiri Kosina
@ 2012-09-05 21:13 ` Benjamin Herrenschmidt
2012-09-06 8:46 ` Jiri Kosina
0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-05 21:13 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Andreas Schwab, Paul Mackerras, linuxppc-dev, linux-kernel
On Wed, 2012-09-05 at 10:56 +0200, Jiri Kosina wrote:
> Hi Benjamin,
>
> actually commit 7256a5d2da56 seems to contain the correct PER_LINUX
> handling, so seems like you picked the right one :)
>
Odd, they looked different around the use of PER_MASK when I looked but
I was tired & jet lagged, so I might have just had a brain fail...
Cheers,
Ben.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc: fix personality handling in ppc64_personality()
2012-09-05 21:13 ` Benjamin Herrenschmidt
@ 2012-09-06 8:46 ` Jiri Kosina
0 siblings, 0 replies; 7+ messages in thread
From: Jiri Kosina @ 2012-09-06 8:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Andreas Schwab, Paul Mackerras, linuxppc-dev, linux-kernel
On Thu, 6 Sep 2012, Benjamin Herrenschmidt wrote:
> > actually commit 7256a5d2da56 seems to contain the correct PER_LINUX
> > handling, so seems like you picked the right one :)
> >
>
> Odd, they looked different around the use of PER_MASK when I looked but
The original patch had
personality &= ~PER_LINUX | PER_LINUX32;
Which is bogus, exactly because ~PER_LINUX is -1.
I then used
personality = (personality & ~PER_MASK) | PER_LINUX32;
which is correct and perhaps a little bit more descriptive, and that is
what you have merged, so all is fine.
> I was tired & jet lagged, so I might have just had a brain fail...
Probably just missed that the first patch used PER_LINUX and the second
one PER_MASK, or whatever.
Anyway, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-09-06 8:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-01 20:03 [PATCH] powerpc: fix personality handling in ppc64_personality() Jiri Kosina
2012-08-01 22:19 ` Andreas Schwab
2012-08-02 7:10 ` [PATCH v2] " Jiri Kosina
2012-09-05 2:54 ` Benjamin Herrenschmidt
2012-09-05 8:56 ` Jiri Kosina
2012-09-05 21:13 ` Benjamin Herrenschmidt
2012-09-06 8:46 ` Jiri Kosina
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).