Skip to content

Commit 177a981

Browse files
Jiri Slabygregkh
Jiri Slaby
authored and
gregkh
committed
futex: Remove duplicated code and fix undefined behaviour
commit 30d6e0a upstream. There is code duplicated over all architecture's headers for futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr, and comparison of the result. Remove this duplication and leave up to the arches only the needed assembly which is now in arch_futex_atomic_op_inuser. This effectively distributes the Will Deacon's arm64 fix for undefined behaviour reported by UBSAN to all architectures. The fix was done in commit 5f16a04 (arm64: futex: Fix undefined behaviour with FUTEX_OP_OPARG_SHIFT usage). Look there for an example dump. And as suggested by Thomas, check for negative oparg too, because it was also reported to cause undefined behaviour report. Note that s390 removed access_ok check in d12a297 ("s390/uaccess: remove pointless access_ok() checks") as access_ok there returns true. We introduce it back to the helper for the sake of simplicity (it gets optimized away anyway). Signed-off-by: Jiri Slaby <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Acked-by: Russell King <[email protected]> Acked-by: Michael Ellerman <[email protected]> (powerpc) Acked-by: Heiko Carstens <[email protected]> [s390] Acked-by: Chris Metcalf <[email protected]> [for tile] Reviewed-by: Darren Hart (VMware) <[email protected]> Reviewed-by: Will Deacon <[email protected]> [core/arm64] Cc: [email protected] Cc: Rich Felker <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: Benjamin Herrenschmidt <[email protected]> Cc: Max Filippov <[email protected]> Cc: Paul Mackerras <[email protected]> Cc: [email protected] Cc: Jonas Bonn <[email protected]> Cc: [email protected] Cc: [email protected] Cc: Yoshinori Sato <[email protected]> Cc: [email protected] Cc: Helge Deller <[email protected]> Cc: "James E.J. Bottomley" <[email protected]> Cc: Catalin Marinas <[email protected]> Cc: Matt Turner <[email protected]> Cc: [email protected] Cc: Fenghua Yu <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: [email protected] Cc: Stefan Kristiansson <[email protected]> Cc: [email protected] Cc: Ivan Kokshaysky <[email protected]> Cc: Stafford Horne <[email protected]> Cc: [email protected] Cc: Richard Henderson <[email protected]> Cc: Chris Zankel <[email protected]> Cc: Michal Simek <[email protected]> Cc: Tony Luck <[email protected]> Cc: [email protected] Cc: Vineet Gupta <[email protected]> Cc: Ralf Baechle <[email protected]> Cc: Richard Kuo <[email protected]> Cc: [email protected] Cc: Martin Schwidefsky <[email protected]> Cc: [email protected] Cc: "David S. Miller" <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Cc: Ben Hutchings <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 2fc1277 commit 177a981

File tree

20 files changed

+126
-470
lines changed

20 files changed

+126
-470
lines changed

‎arch/alpha/include/asm/futex.h‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,10 @@
2929
: "r" (uaddr), "r"(oparg) \
3030
: "memory")
3131

32-
static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
32+
static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
33+
u32 __user *uaddr)
3334
{
34-
int op = (encoded_op >> 28) & 7;
35-
int cmp = (encoded_op >> 24) & 15;
36-
int oparg = (encoded_op << 8) >> 20;
37-
int cmparg = (encoded_op << 20) >> 20;
3835
int oldval = 0, ret;
39-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
40-
oparg = 1 << oparg;
41-
42-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
43-
return -EFAULT;
4436

4537
pagefault_disable();
4638

@@ -66,17 +58,9 @@ static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
6658

6759
pagefault_enable();
6860

69-
if (!ret) {
70-
switch (cmp) {
71-
case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
72-
case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
73-
case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
74-
case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
75-
case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
76-
case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
77-
default: ret = -ENOSYS;
78-
}
79-
}
61+
if (!ret)
62+
*oval = oldval;
63+
8064
return ret;
8165
}
8266

‎arch/arc/include/asm/futex.h‎

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,11 @@
7373

7474
#endif
7575

76-
static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
76+
static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
77+
u32 __user *uaddr)
7778
{
78-
int op = (encoded_op >> 28) & 7;
79-
int cmp = (encoded_op >> 24) & 15;
80-
int oparg = (encoded_op << 8) >> 20;
81-
int cmparg = (encoded_op << 20) >> 20;
8279
int oldval = 0, ret;
8380

84-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
85-
oparg = 1 << oparg;
86-
87-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
88-
return -EFAULT;
89-
9081
#ifndef CONFIG_ARC_HAS_LLSC
9182
preempt_disable(); /* to guarantee atomic r-m-w of futex op */
9283
#endif
@@ -118,30 +109,9 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
118109
preempt_enable();
119110
#endif
120111

121-
if (!ret) {
122-
switch (cmp) {
123-
case FUTEX_OP_CMP_EQ:
124-
ret = (oldval == cmparg);
125-
break;
126-
case FUTEX_OP_CMP_NE:
127-
ret = (oldval != cmparg);
128-
break;
129-
case FUTEX_OP_CMP_LT:
130-
ret = (oldval < cmparg);
131-
break;
132-
case FUTEX_OP_CMP_GE:
133-
ret = (oldval >= cmparg);
134-
break;
135-
case FUTEX_OP_CMP_LE:
136-
ret = (oldval <= cmparg);
137-
break;
138-
case FUTEX_OP_CMP_GT:
139-
ret = (oldval > cmparg);
140-
break;
141-
default:
142-
ret = -ENOSYS;
143-
}
144-
}
112+
if (!ret)
113+
*oval = oldval;
114+
145115
return ret;
146116
}
147117

‎arch/arm/include/asm/futex.h‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -128,20 +128,10 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
128128
#endif /* !SMP */
129129

130130
static inline int
131-
futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
131+
arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
132132
{
133-
int op = (encoded_op >> 28) & 7;
134-
int cmp = (encoded_op >> 24) & 15;
135-
int oparg = (encoded_op << 8) >> 20;
136-
int cmparg = (encoded_op << 20) >> 20;
137133
int oldval = 0, ret, tmp;
138134

139-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
140-
oparg = 1 << oparg;
141-
142-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
143-
return -EFAULT;
144-
145135
#ifndef CONFIG_SMP
146136
preempt_disable();
147137
#endif
@@ -172,17 +162,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
172162
preempt_enable();
173163
#endif
174164

175-
if (!ret) {
176-
switch (cmp) {
177-
case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
178-
case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
179-
case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
180-
case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
181-
case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
182-
case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
183-
default: ret = -ENOSYS;
184-
}
185-
}
165+
if (!ret)
166+
*oval = oldval;
167+
186168
return ret;
187169
}
188170

‎arch/arm64/include/asm/futex.h‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,10 @@
5353
: "memory")
5454

5555
static inline int
56-
futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
56+
arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
5757
{
58-
int op = (encoded_op >> 28) & 7;
59-
int cmp = (encoded_op >> 24) & 15;
60-
int oparg = (int)(encoded_op << 8) >> 20;
61-
int cmparg = (int)(encoded_op << 20) >> 20;
6258
int oldval = 0, ret, tmp;
6359

64-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
65-
oparg = 1U << (oparg & 0x1f);
66-
67-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
68-
return -EFAULT;
69-
7060
pagefault_disable();
7161

7262
switch (op) {
@@ -96,17 +86,9 @@ futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
9686

9787
pagefault_enable();
9888

99-
if (!ret) {
100-
switch (cmp) {
101-
case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
102-
case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
103-
case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
104-
case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
105-
case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
106-
case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
107-
default: ret = -ENOSYS;
108-
}
109-
}
89+
if (!ret)
90+
*oval = oldval;
91+
11092
return ret;
11193
}
11294

‎arch/frv/include/asm/futex.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
#include <asm/errno.h>
88
#include <asm/uaccess.h>
99

10-
extern int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr);
10+
extern int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
11+
u32 __user *uaddr);
1112

1213
static inline int
1314
futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,

‎arch/frv/kernel/futex.c‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -186,20 +186,10 @@ static inline int atomic_futex_op_xchg_xor(int oparg, u32 __user *uaddr, int *_o
186186
/*
187187
* do the futex operations
188188
*/
189-
int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
189+
int arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
190190
{
191-
int op = (encoded_op >> 28) & 7;
192-
int cmp = (encoded_op >> 24) & 15;
193-
int oparg = (encoded_op << 8) >> 20;
194-
int cmparg = (encoded_op << 20) >> 20;
195191
int oldval = 0, ret;
196192

197-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
198-
oparg = 1 << oparg;
199-
200-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
201-
return -EFAULT;
202-
203193
pagefault_disable();
204194

205195
switch (op) {
@@ -225,18 +215,9 @@ int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
225215

226216
pagefault_enable();
227217

228-
if (!ret) {
229-
switch (cmp) {
230-
case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
231-
case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
232-
case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
233-
case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
234-
case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
235-
case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
236-
default: ret = -ENOSYS; break;
237-
}
238-
}
218+
if (!ret)
219+
*oval = oldval;
239220

240221
return ret;
241222

242-
} /* end futex_atomic_op_inuser() */
223+
} /* end arch_futex_atomic_op_inuser() */

‎arch/hexagon/include/asm/futex.h‎

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,9 @@
3131

3232

3333
static inline int
34-
futex_atomic_op_inuser(int encoded_op, int __user *uaddr)
34+
arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
3535
{
36-
int op = (encoded_op >> 28) & 7;
37-
int cmp = (encoded_op >> 24) & 15;
38-
int oparg = (encoded_op << 8) >> 20;
39-
int cmparg = (encoded_op << 20) >> 20;
4036
int oldval = 0, ret;
41-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
42-
oparg = 1 << oparg;
43-
44-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
45-
return -EFAULT;
4637

4738
pagefault_disable();
4839

@@ -72,30 +63,9 @@ futex_atomic_op_inuser(int encoded_op, int __user *uaddr)
7263

7364
pagefault_enable();
7465

75-
if (!ret) {
76-
switch (cmp) {
77-
case FUTEX_OP_CMP_EQ:
78-
ret = (oldval == cmparg);
79-
break;
80-
case FUTEX_OP_CMP_NE:
81-
ret = (oldval != cmparg);
82-
break;
83-
case FUTEX_OP_CMP_LT:
84-
ret = (oldval < cmparg);
85-
break;
86-
case FUTEX_OP_CMP_GE:
87-
ret = (oldval >= cmparg);
88-
break;
89-
case FUTEX_OP_CMP_LE:
90-
ret = (oldval <= cmparg);
91-
break;
92-
case FUTEX_OP_CMP_GT:
93-
ret = (oldval > cmparg);
94-
break;
95-
default:
96-
ret = -ENOSYS;
97-
}
98-
}
66+
if (!ret)
67+
*oval = oldval;
68+
9969
return ret;
10070
}
10171

‎arch/ia64/include/asm/futex.h‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,9 @@ do { \
4545
} while (0)
4646

4747
static inline int
48-
futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
48+
arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
4949
{
50-
int op = (encoded_op >> 28) & 7;
51-
int cmp = (encoded_op >> 24) & 15;
52-
int oparg = (encoded_op << 8) >> 20;
53-
int cmparg = (encoded_op << 20) >> 20;
5450
int oldval = 0, ret;
55-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
56-
oparg = 1 << oparg;
57-
58-
if (! access_ok (VERIFY_WRITE, uaddr, sizeof(u32)))
59-
return -EFAULT;
6051

6152
pagefault_disable();
6253

@@ -84,17 +75,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
8475

8576
pagefault_enable();
8677

87-
if (!ret) {
88-
switch (cmp) {
89-
case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
90-
case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
91-
case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
92-
case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
93-
case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
94-
case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
95-
default: ret = -ENOSYS;
96-
}
97-
}
78+
if (!ret)
79+
*oval = oldval;
80+
9881
return ret;
9982
}
10083

‎arch/microblaze/include/asm/futex.h‎

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,9 @@
2929
})
3030

3131
static inline int
32-
futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
32+
arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
3333
{
34-
int op = (encoded_op >> 28) & 7;
35-
int cmp = (encoded_op >> 24) & 15;
36-
int oparg = (encoded_op << 8) >> 20;
37-
int cmparg = (encoded_op << 20) >> 20;
3834
int oldval = 0, ret;
39-
if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
40-
oparg = 1 << oparg;
41-
42-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
43-
return -EFAULT;
4435

4536
pagefault_disable();
4637

@@ -66,30 +57,9 @@ futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
6657

6758
pagefault_enable();
6859

69-
if (!ret) {
70-
switch (cmp) {
71-
case FUTEX_OP_CMP_EQ:
72-
ret = (oldval == cmparg);
73-
break;
74-
case FUTEX_OP_CMP_NE:
75-
ret = (oldval != cmparg);
76-
break;
77-
case FUTEX_OP_CMP_LT:
78-
ret = (oldval < cmparg);
79-
break;
80-
case FUTEX_OP_CMP_GE:
81-
ret = (oldval >= cmparg);
82-
break;
83-
case FUTEX_OP_CMP_LE:
84-
ret = (oldval <= cmparg);
85-
break;
86-
case FUTEX_OP_CMP_GT:
87-
ret = (oldval > cmparg);
88-
break;
89-
default:
90-
ret = -ENOSYS;
91-
}
92-
}
60+
if (!ret)
61+
*oval = oldval;
62+
9363
return ret;
9464
}
9565

0 commit comments

Comments
 (0)