| 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
| ||
|
Message-ID: <20231124113803.165431-1-cleger@rivosinc.com>
Date: Fri, 24 Nov 2023 12:38:03 +0100
From: Clément Léger <cleger@...osinc.com>
To: Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Cc: Clément Léger <cleger@...osinc.com>,
Christoph Hellwig <hch@...radead.org>,
Ben Dooks <ben.dooks@...ethink.co.uk>,
kernel test robot <lkp@...el.com>
Subject: [PATCH v2] riscv: fix incorrect use of __user pointer
These warnings were reported by sparse and were due to lack of __user
annotation as well as dereferencing such pointer:
arch/riscv/kernel/traps_misaligned.c:361:21: warning: dereference of noderef expression
arch/riscv/kernel/traps_misaligned.c:373:21: warning: dereference of noderef expression
arch/riscv/kernel/traps_misaligned.c:381:21: warning: dereference of noderef expression
arch/riscv/kernel/traps_misaligned.c:322:24: warning: incorrect type in initializer (different address spaces)
arch/riscv/kernel/traps_misaligned.c:322:24: expected unsigned char const [noderef] __user *__gu_ptr
arch/riscv/kernel/traps_misaligned.c:322:24: got unsigned char const [usertype] *addr
arch/riscv/kernel/traps_misaligned.c:361:21: warning: dereference of noderef expression
arch/riscv/kernel/traps_misaligned.c:373:21: warning: dereference of noderef expression
arch/riscv/kernel/traps_misaligned.c:381:21: warning: dereference of noderef expression
arch/riscv/kernel/traps_misaligned.c:332:24: warning: incorrect type in initializer (different address spaces)
arch/riscv/kernel/traps_misaligned.c:332:24: expected unsigned char [noderef] __user *__gu_ptr
arch/riscv/kernel/traps_misaligned.c:332:24: got unsigned char [usertype] *addr
As suggested by Christoph Hellwig, casting pointers from an address
space to another is not a good idea and we should rather cast the
untyped unsigned long to their final address space. Fix the ones in
load_u8()/store_u8()/__read_insn() by passing a unsigned long and then
casting it to the appropriate type (__user of not) depending if used in
kernel/ user mode. Also remove unneeded else construct in store_u8()/
load_u8().
Reported-by: kernel test robot <lkp@...el.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311160606.obGOOwB3-lkp@intel.com/
Signed-off-by: Clément Léger <cleger@...osinc.com>
---
arch/riscv/kernel/traps_misaligned.c | 55 +++++++++++++---------------
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index 5eba37147caa..a92b88af855a 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -265,19 +265,19 @@ static unsigned long get_f32_rs(unsigned long insn, u8 fp_reg_offset,
#define GET_F32_RS2S(insn, regs) (get_f32_rs(RVC_RS2S(insn), 0, regs))
#ifdef CONFIG_RISCV_M_MODE
-static inline int load_u8(struct pt_regs *regs, const u8 *addr, u8 *r_val)
+static inline int load_u8(struct pt_regs *regs, const unsigned long addr, u8 *r_val)
{
u8 val;
- asm volatile("lbu %0, %1" : "=&r" (val) : "m" (*addr));
+ asm volatile("lbu %0, %1" : "=&r" (val) : "m" (*(const u8 *)addr));
*r_val = val;
return 0;
}
-static inline int store_u8(struct pt_regs *regs, u8 *addr, u8 val)
+static inline int store_u8(struct pt_regs *regs, unsigned long addr, u8 val)
{
- asm volatile ("sb %0, %1\n" : : "r" (val), "m" (*addr));
+ asm volatile ("sb %0, %1\n" : : "r" (val), "m" (*(u8 *)addr));
return 0;
}
@@ -316,34 +316,32 @@ static inline int get_insn(struct pt_regs *regs, ulong mepc, ulong *r_insn)
return 0;
}
#else
-static inline int load_u8(struct pt_regs *regs, const u8 *addr, u8 *r_val)
+static inline int load_u8(struct pt_regs *regs, const unsigned long addr, u8 *r_val)
{
- if (user_mode(regs)) {
- return __get_user(*r_val, addr);
- } else {
- *r_val = *addr;
- return 0;
- }
+ if (user_mode(regs))
+ return __get_user(*r_val, (u8 __user *)addr);
+
+ *r_val = *(const u8 *)addr;
+ return 0;
}
-static inline int store_u8(struct pt_regs *regs, u8 *addr, u8 val)
+static inline int store_u8(struct pt_regs *regs, unsigned long addr, u8 val)
{
- if (user_mode(regs)) {
- return __put_user(val, addr);
- } else {
- *addr = val;
- return 0;
- }
+ if (user_mode(regs))
+ return __put_user(val, (u8 __user *)addr);
+
+ *(u8 *)addr = val;
+ return 0;
}
-#define __read_insn(regs, insn, insn_addr) \
+#define __read_insn(regs, insn, insn_addr, type) \
({ \
int __ret; \
\
if (user_mode(regs)) { \
- __ret = __get_user(insn, insn_addr); \
+ __ret = __get_user(insn, (type __user *) insn_addr); \
} else { \
- insn = *insn_addr; \
+ insn = *(type *)insn_addr; \
__ret = 0; \
} \
\
@@ -356,9 +354,8 @@ static inline int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn)
if (epc & 0x2) {
ulong tmp = 0;
- u16 __user *insn_addr = (u16 __user *)epc;
- if (__read_insn(regs, insn, insn_addr))
+ if (__read_insn(regs, insn, epc, u16))
return -EFAULT;
/* __get_user() uses regular "lw" which sign extend the loaded
* value make sure to clear higher order bits in case we "or" it
@@ -369,16 +366,14 @@ static inline int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn)
*r_insn = insn;
return 0;
}
- insn_addr++;
- if (__read_insn(regs, tmp, insn_addr))
+ epc += sizeof(u16);
+ if (__read_insn(regs, tmp, epc, u16))
return -EFAULT;
*r_insn = (tmp << 16) | insn;
return 0;
} else {
- u32 __user *insn_addr = (u32 __user *)epc;
-
- if (__read_insn(regs, insn, insn_addr))
+ if (__read_insn(regs, insn, epc, u32))
return -EFAULT;
if ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) {
*r_insn = insn;
@@ -491,7 +486,7 @@ int handle_misaligned_load(struct pt_regs *regs)
val.data_u64 = 0;
for (i = 0; i < len; i++) {
- if (load_u8(regs, (void *)(addr + i), &val.data_bytes[i]))
+ if (load_u8(regs, addr + i, &val.data_bytes[i]))
return -1;
}
@@ -589,7 +584,7 @@ int handle_misaligned_store(struct pt_regs *regs)
return -EOPNOTSUPP;
for (i = 0; i < len; i++) {
- if (store_u8(regs, (void *)(addr + i), val.data_bytes[i]))
+ if (store_u8(regs, addr + i, val.data_bytes[i]))
return -1;
}
--
2.42.0
Powered by blists - more mailing lists