From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Slaby <jslaby@suse.cz>
Cc: linux-mips@linux-mips.org, Rich Felker <dalias@libc.org>,
linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
peterz@infradead.org,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Will Deacon <will.deacon@arm.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Paul Mackerras <paulus@samba.org>,
"H. Peter Anvin" <hpa@zytor.com>,
sparclinux@vger.kernel.org, Jonas Bonn <jonas@southpole.se>,
linux-s390@vger.kernel.org, linux-arch@vger.kernel.org,
Yoshinori Sato <ysato@users.sourceforge.jp>,
linux-hexagon@vger.kernel.org, Helge Deller <deller@gmx.de>,
x86@kernel.org, "James E.J. Bottomley" <jejb@parisc-linux.org>,
mingo@redhat.com, Catalin Marinas <catalin.marinas@arm.com>,
dvhart@infradead.org, Matt Turner <mattst88@gmail.com>,
linux-snps-arc@lists.infradead.org,
Fenghua Yu <fenghua.yu@intel.com>, Arnd Bergmann <arnd@arndb.de>,
linux-xtensa@linux-xtensa.org
Subject: Re: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 07:51:23 +0000 [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Slaby <jslaby@suse.cz>
Cc: linux-mips@linux-mips.org, Rich Felker <dalias@libc.org>,
linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
peterz@infradead.org,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Will Deacon <will.deacon@arm.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Paul Mackerras <paulus@samba.org>,
"H. Peter Anvin" <hpa@zytor.com>,
sparclinux@vger.kernel.org, Jonas Bonn <jonas@southpole.se>,
linux-s390@vger.kernel.org, linux-arch@vger.kernel.org,
Yoshinori Sato <ysato@users.sourceforge.jp>,
linux-hexagon@vger.kernel.org, Helge Deller <deller@gmx.de>,
x86@kernel.org, "James E.J. Bottomley" <jejb@parisc-linux.org>,
mingo@redhat.com, Catalin Marinas <catalin.marinas@arm.com>,
dvhart@infradead.org, Matt Turner <mattst88@gmail.com>,
linux-snps-arc@lists.infradead.org,
Fenghua Yu <fenghua.yu@intel.com>, Arnd Bergmann <arnd@arndb.de>,
linux-xtensa@linux-xtensa.org, Stefan Kristiansson <
Subject: Re: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 09:51:23 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Slaby <jslaby@suse.cz>
Cc: mingo@redhat.com, peterz@infradead.org, dvhart@infradead.org,
linux-kernel@vger.kernel.org, Richard Henderson <rth@twiddle.net>,
Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
Matt Turner <mattst88@gmail.com>,
Vineet Gupta <vgupta@synopsys.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Richard Kuo <rkuo@codeaurora.org>,
Tony Luck <tony.luck@intel.com>,
Fenghua Yu <fenghua.yu@intel.com>,
Michal Simek <monstr@monstr.eu>,
Ralf Baechle <ralf@linux-mips.org>,
Jonas Bonn <jonas@southpole.se>,
Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
Stafford Horne <shorne@gmail.com>,
"James E.J. Bottomley" <jejb@parisc-linux.org>,
Helge Deller <deller@gmx.de>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Martin Schwidefsky <schwidefsky@de.ibm.com>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Rich Felker <dalias@libc.org>,
"David S. Miller" <davem@davemloft.net>,
"H. Peter Anvin" <hpa@zytor.com>, Chris Zankel <chris@zankel.net>,
Max Filippov <jcmvbkbc@gmail.com>, Arnd Bergmann <arnd@arndb.de>,
x86@kernel.org, linux-alpha@vger.kernel.org,
linux-snps-arc@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-hexagon@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-mips@linux-mips.org, openrisc@lists.librecores.org,
linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-s390@vger.kernel.org, linux-sh@vger.kernel.org,
sparclinux@vger.kernel.org, linux-xtensa@linux-xtensa.org,
linux-arch@vger.kernel.org
Subject: Re: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 09:51:23 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Slaby <jslaby@suse.cz>
Cc: linux-mips@linux-mips.org, Rich Felker <dalias@libc.org>,
linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
peterz@infradead.org,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Will Deacon <will.deacon@arm.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Paul Mackerras <paulus@samba.org>,
"H. Peter Anvin" <hpa@zytor.com>,
sparclinux@vger.kernel.org, Jonas Bonn <jonas@southpole.se>,
linux-s390@vger.kernel.org, linux-arch@vger.kernel.org,
Yoshinori Sato <ysato@users.sourceforge.jp>,
linux-hexagon@vger.kernel.org, Helge Deller <deller@gmx.de>,
x86@kernel.org, "James E.J. Bottomley" <jejb@parisc-linux.org>,
mingo@redhat.com, Catalin Marinas <catalin.marinas@arm.com>,
dvhart@infradead.org, Matt Turner <mattst88@gmail.com>,
linux-snps-arc@lists.infradead.org,
Fenghua Yu <fenghua.yu@intel.com>, Arnd Bergmann <arnd@arndb.de>,
linux-xtensa@linux-xtensa.org
Subject: Re: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 09:51:23 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: tglx@linutronix.de (Thomas Gleixner)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 09:51:23 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 09:51:23 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Slaby <jslaby@suse.cz>
Cc: linux-mips@linux-mips.org, Rich Felker <dalias@libc.org>,
linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
peterz@infradead.org,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Will Deacon <will.deacon@arm.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Paul Mackerras <paulus@samba.org>,
"H. Peter Anvin" <hpa@zytor.com>,
sparclinux@vger.kernel.org, Jonas Bonn <jonas@southpole.se>,
linux-s390@vger.kernel.org, linux-arch@vger.kernel.org,
Yoshinori Sato <ysato@users.sourceforge.jp>,
linux-hexagon@vger.kernel.org, Helge Deller <deller@gmx.de>,
x86@kernel.org, "James E.J. Bottomley" <jejb@parisc-linux.org>,
mingo@redhat.com, Catalin Marinas <catalin.marinas@arm.com>,
dvhart@infradead.org, Matt Turner <mattst88@gmail.com>,
linux-snps-arc@lists.infradead.org,
Fenghua Yu <fenghua.yu@intel.com>, Arnd Bergmann <arnd@arndb.de>,
linux-xtensa@linux-xtensa.org, Stefan Kristiansson <>
Subject: Re: [PATCH 1/1] futex: remove duplicated code and fix UB
Date: Fri, 23 Jun 2017 09:51:23 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1706230017520.2221@nanos> (raw)
In-Reply-To: <20170621115318.2781-1-jslaby@suse.cz>
On Wed, 21 Jun 2017, Jiri Slaby wrote:
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f32b42e8725d..5bb2fd4674e7 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -48,20 +48,10 @@ do { \
> } while (0)
>
> static inline int
> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
That unsigned int seems to be a change from the arm64 tree in next. It's
not upstream and it'll cause a (easy to resolve) conflict.
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So this is really bad. We have implicit and explicit type casting to
int. And while we are at it can we please stop proliferating the existing
mess.
'op' and 'cmp' definitly can be unsigned int. There is no reason to cast
them to int.
oparg, cmparg and oldval are more interesting.
The logic here is "documented" in uapi/linux/futex.h
/* FUTEX_WAKE_OP will perform atomically
int oldval = *(int *)UADDR2;
*(int *)UADDR2 = oldval OP OPARG;
if (oldval CMP CMPARG)
wake UADDR2; */
Now the FUTEX_OP macro which is supposed to compose the encoded_up does:
#define FUTEX_OP(op, oparg, cmp, cmparg) \
(((op & 0xf) << 28) | ((cmp & 0xf) << 24) \
| ((oparg & 0xfff) << 12) | (cmparg & 0xfff))
Of course this all is not typed, undocumented and completely ill
defined.
> + int oparg = (int)(encoded_op << 8) >> 20;
> + int cmparg = (int)(encoded_op << 20) >> 20;
So in fact we sign expand the 12 bits of oparg and cmparg. Really
intuitive.
Yes, we probably can't change that anymore, but at least we should make it
very explicit and add a comment to that effect.
Thanks,
tglx
next prev parent reply other threads:[~2017-06-23 7:51 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-15 13:07 [PATCH 1/1] futex: remove duplicated code Jiri Slaby
2017-05-15 13:07 ` [OpenRISC] " Jiri Slaby
2017-05-15 13:07 ` Jiri Slaby
2017-05-15 13:07 ` Jiri Slaby
2017-05-15 13:07 ` Jiri Slaby
2017-05-15 13:07 ` Jiri Slaby
2017-05-15 13:07 ` Jiri Slaby
2017-05-15 13:16 ` Will Deacon
2017-05-15 13:16 ` [OpenRISC] " Will Deacon
2017-05-15 13:16 ` Will Deacon
2017-05-15 13:16 ` Will Deacon
2017-05-15 13:16 ` Will Deacon
2017-05-15 13:16 ` Will Deacon
2017-05-15 13:16 ` Will Deacon
2017-05-17 8:01 ` Jiri Slaby
2017-05-17 8:01 ` [OpenRISC] " Jiri Slaby
2017-05-17 8:01 ` Jiri Slaby
2017-05-17 8:01 ` Jiri Slaby
2017-05-17 8:01 ` Jiri Slaby
2017-05-17 8:01 ` Jiri Slaby
2017-05-17 8:01 ` Jiri Slaby
2017-05-18 17:30 ` Will Deacon
2017-05-18 17:30 ` [OpenRISC] " Will Deacon
2017-05-18 17:30 ` Will Deacon
2017-05-18 17:30 ` Will Deacon
2017-05-18 17:30 ` Will Deacon
2017-05-18 17:30 ` Will Deacon
2017-05-18 17:30 ` Will Deacon
2017-05-22 21:11 ` Thomas Gleixner
2017-05-22 21:11 ` [OpenRISC] " Thomas Gleixner
2017-05-22 21:11 ` Thomas Gleixner
2017-05-22 21:11 ` Thomas Gleixner
2017-05-22 21:11 ` Thomas Gleixner
2017-05-22 21:11 ` Thomas Gleixner
2017-05-22 21:11 ` Thomas Gleixner
2017-05-25 14:28 ` Will Deacon
2017-05-25 14:28 ` [OpenRISC] " Will Deacon
2017-05-25 14:28 ` Will Deacon
2017-05-25 14:28 ` Will Deacon
2017-05-25 14:28 ` Will Deacon
2017-05-25 14:28 ` Will Deacon
2017-05-25 14:28 ` Will Deacon
2017-05-26 6:54 ` Thomas Gleixner
2017-05-26 6:54 ` [OpenRISC] " Thomas Gleixner
2017-05-26 6:54 ` Thomas Gleixner
2017-05-26 6:54 ` Thomas Gleixner
2017-05-26 6:54 ` Thomas Gleixner
2017-05-26 6:54 ` Thomas Gleixner
2017-05-26 6:54 ` Thomas Gleixner
2017-06-21 11:53 ` [PATCH 1/1] futex: remove duplicated code and fix UB Jiri Slaby
2017-06-21 11:53 ` Jiri Slaby
2017-06-21 11:53 ` Jiri Slaby
2017-06-21 11:53 ` Jiri Slaby
2017-06-21 11:53 ` Jiri Slaby
2017-06-21 11:53 ` Jiri Slaby
2017-06-22 3:53 ` Darren Hart
2017-06-22 3:53 ` Darren Hart
2017-06-22 3:53 ` Darren Hart
2017-06-22 3:53 ` Darren Hart
2017-06-22 3:53 ` Darren Hart
2017-06-22 3:53 ` Darren Hart
2017-06-23 7:51 ` Thomas Gleixner [this message]
2017-06-23 7:51 ` Thomas Gleixner
2017-06-23 7:51 ` Thomas Gleixner
2017-06-23 7:51 ` Thomas Gleixner
2017-06-23 7:51 ` Thomas Gleixner
2017-06-23 7:51 ` Thomas Gleixner
2017-06-23 7:51 ` Thomas Gleixner
2017-06-26 12:02 ` Jiri Slaby
2017-06-26 12:02 ` Jiri Slaby
2017-06-26 12:02 ` Jiri Slaby
2017-06-26 12:02 ` Jiri Slaby
2017-06-26 12:02 ` Jiri Slaby
2017-06-26 12:08 ` Will Deacon
2017-06-26 12:08 ` Will Deacon
2017-06-26 12:08 ` Will Deacon
2017-06-26 12:08 ` Will Deacon
2017-06-26 12:08 ` Will Deacon
2017-07-03 10:18 ` Thomas Gleixner
2017-07-03 10:18 ` Thomas Gleixner
2017-07-03 10:18 ` Thomas Gleixner
2017-07-03 10:18 ` Thomas Gleixner
2017-07-03 10:18 ` Thomas Gleixner
2017-07-03 10:18 ` Thomas Gleixner
2017-07-03 10:18 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.20.1706230017520.2221@nanos \
--to=tglx@linutronix.de \
--cc=arnd@arndb.de \
--cc=benh@kernel.crashing.org \
--cc=catalin.marinas@arm.com \
--cc=dalias@libc.org \
--cc=deller@gmx.de \
--cc=dvhart@infradead.org \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=jcmvbkbc@gmail.com \
--cc=jejb@parisc-linux.org \
--cc=jonas@southpole.se \
--cc=jslaby@suse.cz \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hexagon@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux-snps-arc@lists.infradead.org \
--cc=linux-xtensa@linux-xtensa.org \
--cc=mattst88@gmail.com \
--cc=mingo@redhat.com \
--cc=paulus@samba.org \
--cc=peterz@infradead.org \
--cc=sparclinux@vger.kernel.org \
--cc=will.deacon@arm.com \
--cc=x86@kernel.org \
--cc=ysato@users.sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.