| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 1 Jan 2021 01:14:41 +0100
From: John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
To: Al Viro <viro@...iv.linux.org.uk>, Rich Felker <dalias@...c.org>
Cc: linux-sh@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [sh] fix trivial misannotations
On 1/1/21 12:23 AM, Al Viro wrote:
> Trivial misannotations in
> * get_user() (__gu_addr is a userland pointer there)
> * ip_fast_csum() (sum is __wsum, not unsigned int)
> * csum_and_copy_to_user() (destination is void *, not const void * - mea culpa)
> * __clear_user() (to is a userland pointer)
> * several places in kernel/traps_32.c (regs->pc is a userland pointer when regs is a
> userland pt_regs)
> * math-emu/math.c: READ() and WRITE() casts of address should be to userland pointer.
>
> No changes in code generation and those take care of the majority of noise from sparse
> on sh builds.
>
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
> ---
> diff --git a/arch/sh/include/asm/checksum_32.h b/arch/sh/include/asm/checksum_32.h
> index 1a391e3a7659..a6501b856f3e 100644
> --- a/arch/sh/include/asm/checksum_32.h
> +++ b/arch/sh/include/asm/checksum_32.h
> @@ -84,7 +84,8 @@ static inline __sum16 csum_fold(__wsum sum)
> */
> static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
> {
> - unsigned int sum, __dummy0, __dummy1;
> + __wsum sum;
> + unsigned int __dummy0, __dummy1;
>
> __asm__ __volatile__(
> "mov.l @%1+, %0\n\t"
> @@ -197,6 +198,6 @@ static inline __wsum csum_and_copy_to_user(const void *src,
> {
> if (!access_ok(dst, len))
> return 0;
> - return csum_partial_copy_generic((__force const void *)src, dst, len);
> + return csum_partial_copy_generic(src, (__force void *)dst, len);
> }
> #endif /* __ASM_SH_CHECKSUM_H */
> diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
> index 73f3b48d4a34..8867bb04b00e 100644
> --- a/arch/sh/include/asm/uaccess.h
> +++ b/arch/sh/include/asm/uaccess.h
> @@ -68,7 +68,7 @@ struct __large_struct { unsigned long buf[100]; };
> ({ \
> long __gu_err = -EFAULT; \
> unsigned long __gu_val = 0; \
> - const __typeof__(*(ptr)) *__gu_addr = (ptr); \
> + const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> if (likely(access_ok(__gu_addr, (size)))) \
> __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> (x) = (__force __typeof__(*(ptr)))__gu_val; \
> @@ -124,7 +124,7 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
> * Clear the area and return remaining number of bytes
> * (on failure. Usually it's 0.)
> */
> -__kernel_size_t __clear_user(void *addr, __kernel_size_t size);
> +__kernel_size_t __clear_user(void __user *addr, __kernel_size_t size);
>
> #define clear_user(addr,n) \
> ({ \
> diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c
> index b62ad0ba2395..b3c715bc254b 100644
> --- a/arch/sh/kernel/traps_32.c
> +++ b/arch/sh/kernel/traps_32.c
> @@ -490,7 +490,7 @@ asmlinkage void do_address_error(struct pt_regs *regs,
> inc_unaligned_user_access();
>
> oldfs = force_uaccess_begin();
> - if (copy_from_user(&instruction, (insn_size_t *)(regs->pc & ~1),
> + if (copy_from_user(&instruction, (insn_size_t __user *)(regs->pc & ~1),
> sizeof(instruction))) {
> force_uaccess_end(oldfs);
> goto uspace_segv;
> @@ -614,7 +614,7 @@ asmlinkage void do_reserved_inst(void)
> unsigned short inst = 0;
> int err;
>
> - get_user(inst, (unsigned short*)regs->pc);
> + get_user(inst, (unsigned short __user *)regs->pc);
>
> err = do_fpu_inst(inst, regs);
> if (!err) {
> @@ -699,9 +699,9 @@ asmlinkage void do_illegal_slot_inst(void)
> return;
>
> #ifdef CONFIG_SH_FPU_EMU
> - get_user(inst, (unsigned short *)regs->pc + 1);
> + get_user(inst, (unsigned short __user *)regs->pc + 1);
> if (!do_fpu_inst(inst, regs)) {
> - get_user(inst, (unsigned short *)regs->pc);
> + get_user(inst, (unsigned short __user *)regs->pc);
> if (!emulate_branch(inst, regs))
> return;
> /* fault in branch.*/
> diff --git a/arch/sh/math-emu/math.c b/arch/sh/math-emu/math.c
> index e8be0eca0444..3495a48b7713 100644
> --- a/arch/sh/math-emu/math.c
> +++ b/arch/sh/math-emu/math.c
> @@ -51,8 +51,8 @@
> #define Rn (regs->regs[n])
> #define Rm (regs->regs[m])
>
> -#define WRITE(d,a) ({if(put_user(d, (typeof (d)*)a)) return -EFAULT;})
> -#define READ(d,a) ({if(get_user(d, (typeof (d)*)a)) return -EFAULT;})
> +#define WRITE(d,a) ({if(put_user(d, (typeof (d) __user *)a)) return -EFAULT;})
> +#define READ(d,a) ({if(get_user(d, (typeof (d) __user *)a)) return -EFAULT;})
>
> #define PACK_S(r,f) FP_PACK_SP(&r,f)
> #define UNPACK_S(f,r) FP_UNPACK_SP(f,&r)
> diff --git a/arch/sh/mm/nommu.c b/arch/sh/mm/nommu.c
> index 8b4504413c5f..78c4b6e6d33b 100644
> --- a/arch/sh/mm/nommu.c
> +++ b/arch/sh/mm/nommu.c
> @@ -28,9 +28,9 @@ __kernel_size_t __copy_user(void *to, const void *from, __kernel_size_t n)
> return 0;
> }
>
> -__kernel_size_t __clear_user(void *to, __kernel_size_t n)
> +__kernel_size_t __clear_user(void __user *to, __kernel_size_t n)
> {
> - memset(to, 0, n);
> + memset((__force void *)to, 0, n);
> return 0;
> }
>
>
Verified on my SH-7785LCR board. Boots fine.
Tested-by: John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@...ian.org
`. `' Freie Universitaet Berlin - glaubitz@...sik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Powered by blists - more mailing lists