Skip to content

Commit d02f6b7

Browse files
npigginmpe
authored andcommitted
powerpc/uaccess: Evaluate macro arguments once, before user access is allowed
get/put_user() can be called with nontrivial arguments. fs/proc/page.c has a good example: if (put_user(stable_page_flags(ppage), out)) { stable_page_flags() is quite a lot of code, including spin locks in the page allocator. Ensure these arguments are evaluated before user access is allowed. This improves security by reducing code with access to userspace, but it also fixes a PREEMPT bug with KUAP on powerpc/64s: stable_page_flags() is currently called with AMR set to allow writes, it ends up calling spin_unlock(), which can call preempt_schedule. But the task switch code can not be called with AMR set (it relies on interrupts saving the register), so this blows up. It's fine if the code inside allow_user_access() is preemptible, because a timer or IPI will save the AMR, but it's not okay to explicitly cause a reschedule. Fixes: de78a9c ("powerpc: Add a framework for Kernel Userspace Access Protection") Signed-off-by: Nicholas Piggin <[email protected]> Signed-off-by: Michael Ellerman <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 8f3d9f3 commit d02f6b7

File tree

1 file changed

+35
-14
lines changed

1 file changed

+35
-14
lines changed

arch/powerpc/include/asm/uaccess.h

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -166,32 +166,44 @@ do { \
166166
({ \
167167
long __pu_err; \
168168
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
169+
__typeof__(*(ptr)) __pu_val = (x); \
170+
__typeof__(size) __pu_size = (size); \
171+
\
169172
if (!is_kernel_addr((unsigned long)__pu_addr)) \
170173
might_fault(); \
171-
__chk_user_ptr(ptr); \
174+
__chk_user_ptr(__pu_addr); \
172175
if (do_allow) \
173-
__put_user_size((x), __pu_addr, (size), __pu_err); \
176+
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
174177
else \
175-
__put_user_size_allowed((x), __pu_addr, (size), __pu_err); \
178+
__put_user_size_allowed(__pu_val, __pu_addr, __pu_size, __pu_err); \
179+
\
176180
__pu_err; \
177181
})
178182

179183
#define __put_user_check(x, ptr, size) \
180184
({ \
181185
long __pu_err = -EFAULT; \
182186
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
187+
__typeof__(*(ptr)) __pu_val = (x); \
188+
__typeof__(size) __pu_size = (size); \
189+
\
183190
might_fault(); \
184-
if (access_ok(__pu_addr, size)) \
185-
__put_user_size((x), __pu_addr, (size), __pu_err); \
191+
if (access_ok(__pu_addr, __pu_size)) \
192+
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
193+
\
186194
__pu_err; \
187195
})
188196

189197
#define __put_user_nosleep(x, ptr, size) \
190198
({ \
191199
long __pu_err; \
192200
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
193-
__chk_user_ptr(ptr); \
194-
__put_user_size((x), __pu_addr, (size), __pu_err); \
201+
__typeof__(*(ptr)) __pu_val = (x); \
202+
__typeof__(size) __pu_size = (size); \
203+
\
204+
__chk_user_ptr(__pu_addr); \
205+
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
206+
\
195207
__pu_err; \
196208
})
197209

@@ -283,15 +295,18 @@ do { \
283295
long __gu_err; \
284296
__long_type(*(ptr)) __gu_val; \
285297
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
286-
__chk_user_ptr(ptr); \
298+
__typeof__(size) __gu_size = (size); \
299+
\
300+
__chk_user_ptr(__gu_addr); \
287301
if (!is_kernel_addr((unsigned long)__gu_addr)) \
288302
might_fault(); \
289303
barrier_nospec(); \
290304
if (do_allow) \
291-
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
305+
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
292306
else \
293-
__get_user_size_allowed(__gu_val, __gu_addr, (size), __gu_err); \
307+
__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
294308
(x) = (__typeof__(*(ptr)))__gu_val; \
309+
\
295310
__gu_err; \
296311
})
297312

@@ -300,12 +315,15 @@ do { \
300315
long __gu_err = -EFAULT; \
301316
__long_type(*(ptr)) __gu_val = 0; \
302317
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
318+
__typeof__(size) __gu_size = (size); \
319+
\
303320
might_fault(); \
304-
if (access_ok(__gu_addr, (size))) { \
321+
if (access_ok(__gu_addr, __gu_size)) { \
305322
barrier_nospec(); \
306-
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
323+
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
307324
} \
308325
(x) = (__force __typeof__(*(ptr)))__gu_val; \
326+
\
309327
__gu_err; \
310328
})
311329

@@ -314,10 +332,13 @@ do { \
314332
long __gu_err; \
315333
__long_type(*(ptr)) __gu_val; \
316334
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
317-
__chk_user_ptr(ptr); \
335+
__typeof__(size) __gu_size = (size); \
336+
\
337+
__chk_user_ptr(__gu_addr); \
318338
barrier_nospec(); \
319-
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
339+
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
320340
(x) = (__force __typeof__(*(ptr)))__gu_val; \
341+
\
321342
__gu_err; \
322343
})
323344

0 commit comments

Comments
 (0)