x86: polish __{get,put}_user_{,no}check() The primary purpose is correcting a latent bug in __get_user_check() (the macro has no active user at present): The access_ok() check should be before the actual access, or else any PV guest could initiate MMIO reads with side effects. Clean up all four macros at once: - all arguments evaluated exactly once - build the "check" flavor using the "nocheck" ones, instead of open coding them - "int" is wide enough for error codes - name local variables without using underscores as prefixes - avoid pointless parentheses - add blanks after commas separating parameters or arguments - consistently use tabs for indentation Signed-off-by: Jan Beulich --- This corrects the code which would have resulted in an XSA on Xen 4.2 and older, if those were still security supported. For that reason I at least want to explore whether this is a change we want to take for 4.9. --- a/xen/include/asm-x86/uaccess.h +++ b/xen/include/asm-x86/uaccess.h @@ -104,37 +104,35 @@ extern void __put_user_bad(void); #define __put_user(x,ptr) \ __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr))) -#define __put_user_nocheck(x,ptr,size) \ -({ \ - long __pu_err; \ - __put_user_size((x),(ptr),(size),__pu_err,-EFAULT); \ - __pu_err; \ +#define __put_user_nocheck(x, ptr, size) \ +({ \ + int err_; \ + __put_user_size(x, ptr, size, err_, -EFAULT); \ + err_; \ }) -#define __put_user_check(x,ptr,size) \ +#define __put_user_check(x, ptr, size) \ ({ \ - long __pu_err = -EFAULT; \ - __typeof__(*(ptr)) __user *__pu_addr = (ptr); \ - if (access_ok(__pu_addr,size)) \ - __put_user_size((x),__pu_addr,(size),__pu_err,-EFAULT); \ - __pu_err; \ + __typeof__(*(ptr)) __user *ptr_ = (ptr); \ + __typeof__(size) size_ = (size); \ + access_ok(ptr_, size_) ? __put_user_nocheck(x, ptr_, size_) \ + : -EFAULT; \ }) -#define __get_user_nocheck(x,ptr,size) \ -({ \ - long __gu_err; \ - __get_user_size((x),(ptr),(size),__gu_err,-EFAULT); \ - __gu_err; \ +#define __get_user_nocheck(x, ptr, size) \ +({ \ + int err_; \ + __get_user_size(x, ptr, size, err_, -EFAULT); \ + err_; \ }) -#define __get_user_check(x,ptr,size) \ -({ \ - long __gu_err; \ - __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ - __get_user_size((x),__gu_addr,(size),__gu_err,-EFAULT); \ - if (!access_ok(__gu_addr,size)) __gu_err = -EFAULT; \ - __gu_err; \ -}) +#define __get_user_check(x, ptr, size) \ +({ \ + __typeof__(*(ptr)) __user *ptr_ = (ptr); \ + __typeof__(size) size_ = (size); \ + access_ok(ptr_, size_) ? __get_user_nocheck(x, ptr_, size_) \ + : -EFAULT; \ +}) struct __large_struct { unsigned long buf[100]; }; #define __m(x) (*(const struct __large_struct *)(x))