| Message ID | 1617011360-102531-1-git-send-email-yang.lee@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/mce/dev-mcelog: Fix potential memory access error | expand |
- set_bit(MCE_OVERFLOW, (unsigned long *)&mcelog->flags);
+ mcelog->flags |= BIT(MCE_OVERFLOW);
set_bit() is an atomic operation because it might race with the code to
get and clear this bit:
do {
flags = mcelog->flags;
} while (cmpxchg(&mcelog->flags, flags, 0) != flags);
Originally this was needed because mcelog could be called from #MC
context.
That's all changed now and the machine check notifier chain routines are
called in a kernel thread. So some other locking (mutex?) could be used
to protect access to mcelog->flags.
-Tony
diff --git a/arch/x86/kernel/cpu/mce/dev-mcelog.c b/arch/x86/kernel/cpu/mce/dev-mcelog.c index 100fbee..fd7b1b4 100644 --- a/arch/x86/kernel/cpu/mce/dev-mcelog.c +++ b/arch/x86/kernel/cpu/mce/dev-mcelog.c @@ -51,7 +51,7 @@ static int dev_mce_log(struct notifier_block *nb, unsigned long val, * earlier errors are the more interesting ones: */ if (entry >= mcelog->len) { - set_bit(MCE_OVERFLOW, (unsigned long *)&mcelog->flags); + mcelog->flags |= BIT(MCE_OVERFLOW); goto unlock; }
Using set_bit() to set a bit in an integer is not a good idea, since the function expects an unsigned long as argument, which can be 64bit wide. Coverity reports this problem as High:Out-of-bounds access(INCOMPATIBLE_CAST) CWE119: Out-of-bounds access to a scalar Pointer "&mcelog->flags" points to an object whose effective type is "unsigned int" (32 bits, unsigned) but is dereferenced as a wider "unsigned long" (64 bits, unsigned). This may lead to memory corruption. /home/heyuan.shy/git-repo/linux/arch/x86/kernel/cpu/mce/dev-mcelog.c: dev_mce_log Just use BIT instead. Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> --- arch/x86/kernel/cpu/mce/dev-mcelog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)