* [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report
@ 2021-03-12 4:12 qiang.zhang
2021-03-12 6:30 ` Dmitry Vyukov
2021-03-12 6:32 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: qiang.zhang @ 2021-03-12 4:12 UTC (permalink / raw)
To: linux; +Cc: akpm, linux-kernel, linux-arm-kernel, syzkaller-bugs
From: Zqiang <qiang.zhang@windriver.com>
BUG: using smp_processor_id() in preemptible [00000000] code:
syz-executor.0/15841
caller is debug_smp_processor_id+0x20/0x24
lib/smp_processor_id.c:64
The smp_processor_id() is used in a code segment when
preemption has been disabled, otherwise, when preemption
is enabled this pointer is usually no longer useful
since it may no longer point to per cpu data of the
current processor.
Reported-by: syzbot <syzbot+a7ee43e564223f195c84@syzkaller.appspotmail.com>
Fixes: f5fe12b1eaee ("ARM: spectre-v2: harden user aborts in kernel space")
Signed-off-by: Zqiang <qiang.zhang@windriver.com>
---
arch/arm/include/asm/system_misc.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
index 66f6a3ae68d2..61916dc7d361 100644
--- a/arch/arm/include/asm/system_misc.h
+++ b/arch/arm/include/asm/system_misc.h
@@ -21,8 +21,10 @@ typedef void (*harden_branch_predictor_fn_t)(void);
DECLARE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn);
static inline void harden_branch_predictor(void)
{
+ preempt_disable();
harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn,
smp_processor_id());
+ preempt_enable();
if (fn)
fn();
}
--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report
2021-03-12 4:12 [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report qiang.zhang
@ 2021-03-12 6:30 ` Dmitry Vyukov
2021-03-12 7:50 ` 回复: " Zhang, Qiang
2021-03-12 6:32 ` kernel test robot
1 sibling, 1 reply; 4+ messages in thread
From: Dmitry Vyukov @ 2021-03-12 6:30 UTC (permalink / raw)
To: Zhang, Qiang
Cc: Russell King - ARM Linux, Andrew Morton, LKML, Linux ARM, syzkaller-bugs
On Fri, Mar 12, 2021 at 5:13 AM <qiang.zhang@windriver.com> wrote:
>
> From: Zqiang <qiang.zhang@windriver.com>
>
> BUG: using smp_processor_id() in preemptible [00000000] code:
> syz-executor.0/15841
> caller is debug_smp_processor_id+0x20/0x24
> lib/smp_processor_id.c:64
>
> The smp_processor_id() is used in a code segment when
> preemption has been disabled, otherwise, when preemption
> is enabled this pointer is usually no longer useful
> since it may no longer point to per cpu data of the
> current processor.
>
> Reported-by: syzbot <syzbot+a7ee43e564223f195c84@syzkaller.appspotmail.com>
> Fixes: f5fe12b1eaee ("ARM: spectre-v2: harden user aborts in kernel space")
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
> arch/arm/include/asm/system_misc.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
> index 66f6a3ae68d2..61916dc7d361 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -21,8 +21,10 @@ typedef void (*harden_branch_predictor_fn_t)(void);
> DECLARE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn);
> static inline void harden_branch_predictor(void)
> {
> + preempt_disable();
> harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn,
> smp_processor_id());
> + preempt_enable();
> if (fn)
> fn();
> }
Hi Qiang,
If the CPU can change here, what if it changes right after preempt_enable()?
Disabling preemption just around reading the callback looks like a
no-op. Shouldn't we disable preemption at least around reading and
calling the callback?
On the second look, the fn seems to be const after init, so maybe we
need to use raw_smp_processor_id() instead with an explanatory
comment?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* 回复: [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report
2021-03-12 6:30 ` Dmitry Vyukov
@ 2021-03-12 7:50 ` Zhang, Qiang
0 siblings, 0 replies; 4+ messages in thread
From: Zhang, Qiang @ 2021-03-12 7:50 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Russell King - ARM Linux, Andrew Morton, LKML, Linux ARM, syzkaller-bugs
________________________________________
发件人: Dmitry Vyukov <dvyukov@google.com>
发送时间: 2021年3月12日 14:30
收件人: Zhang, Qiang
抄送: Russell King - ARM Linux; Andrew Morton; LKML; Linux ARM; syzkaller-bugs
主题: Re: [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report
[Please note: This e-mail is from an EXTERNAL e-mail address]
On Fri, Mar 12, 2021 at 5:13 AM <qiang.zhang@windriver.com> wrote:
>
> From: Zqiang <qiang.zhang@windriver.com>
>
> BUG: using smp_processor_id() in preemptible [00000000] code:
> syz-executor.0/15841
> caller is debug_smp_processor_id+0x20/0x24
> lib/smp_processor_id.c:64
>
> The smp_processor_id() is used in a code segment when
> preemption has been disabled, otherwise, when preemption
> is enabled this pointer is usually no longer useful
> since it may no longer point to per cpu data of the
> current processor.
>
> Reported-by: syzbot <syzbot+a7ee43e564223f195c84@syzkaller.appspotmail.com>
> Fixes: f5fe12b1eaee ("ARM: spectre-v2: harden user aborts in kernel space")
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
> arch/arm/include/asm/system_misc.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
> index 66f6a3ae68d2..61916dc7d361 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -21,8 +21,10 @@ typedef void (*harden_branch_predictor_fn_t)(void);
> DECLARE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn);
> static inline void harden_branch_predictor(void)
> {
> + preempt_disable();
> harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn,
> smp_processor_id());
> + preempt_enable();
> if (fn)
> fn();
> }
>Hi Qiang,
>
>If the CPU can change here, what if it changes right after >preempt_enable()?
>Disabling preemption just around reading the callback looks like a
>no-op. Shouldn't we disable preemption at least around reading and
>calling the callback?
Hi dvyukov
Oh, I'm confused, we should call preempt_enable after calling callback function, to make sure callback function is called on current processor . thank you for your remind.
>
>On the second look, the fn seems to be const after init, so maybe we
>need to use raw_smp_processor_id() instead with an explanatory
>comment?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report
2021-03-12 4:12 [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report qiang.zhang
2021-03-12 6:30 ` Dmitry Vyukov
@ 2021-03-12 6:32 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-03-12 6:32 UTC (permalink / raw)
To: qiang.zhang, linux
Cc: kbuild-all, akpm, linux-kernel, linux-arm-kernel, syzkaller-bugs
[-- Attachment #1: Type: text/plain, Size: 4763 bytes --]
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on linus/master hnaz-linux-mm/master v5.12-rc2 next-20210311]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/qiang-zhang-windriver-com/ARM-Fix-incorrect-use-of-smp_processor_id-by-syzbot-report/20210312-121529
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a74e6a014c9d4d4161061f770c9b4f98372ac778
config: arm-pxa168_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b31b47976c6f81426cc00c99cc1452a60e89d972
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review qiang-zhang-windriver-com/ARM-Fix-incorrect-use-of-smp_processor_id-by-syzbot-report/20210312-121529
git checkout b31b47976c6f81426cc00c99cc1452a60e89d972
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from arch/arm/mm/fault.c:21:
arch/arm/include/asm/system_misc.h: In function 'harden_branch_predictor':
>> arch/arm/include/asm/system_misc.h:25:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
25 | harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm/mm/fault.c: At top level:
arch/arm/mm/fault.c:518:1: warning: no previous prototype for 'do_DataAbort' [-Wmissing-prototypes]
518 | do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
| ^~~~~~~~~~~~
arch/arm/mm/fault.c:548:1: warning: no previous prototype for 'do_PrefetchAbort' [-Wmissing-prototypes]
548 | do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~
--
In file included from arch/arm/mm/proc-v7-bugs.c:9:
arch/arm/include/asm/system_misc.h: In function 'harden_branch_predictor':
>> arch/arm/include/asm/system_misc.h:25:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
25 | harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm/mm/proc-v7-bugs.c: At top level:
arch/arm/mm/proc-v7-bugs.c:142:6: warning: no previous prototype for 'cpu_v7_ca8_ibe' [-Wmissing-prototypes]
142 | void cpu_v7_ca8_ibe(void)
| ^~~~~~~~~~~~~~
arch/arm/mm/proc-v7-bugs.c:148:6: warning: no previous prototype for 'cpu_v7_ca15_ibe' [-Wmissing-prototypes]
148 | void cpu_v7_ca15_ibe(void)
| ^~~~~~~~~~~~~~~
arch/arm/mm/proc-v7-bugs.c:154:6: warning: no previous prototype for 'cpu_v7_bugs_init' [-Wmissing-prototypes]
154 | void cpu_v7_bugs_init(void)
| ^~~~~~~~~~~~~~~~
vim +25 arch/arm/include/asm/system_misc.h
9f97da78bf0182 David Howells 2012-03-28 18
f5fe12b1eaee22 Russell King 2018-05-14 19 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
f5fe12b1eaee22 Russell King 2018-05-14 20 typedef void (*harden_branch_predictor_fn_t)(void);
f5fe12b1eaee22 Russell King 2018-05-14 21 DECLARE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn);
f5fe12b1eaee22 Russell King 2018-05-14 22 static inline void harden_branch_predictor(void)
f5fe12b1eaee22 Russell King 2018-05-14 23 {
b31b47976c6f81 Zqiang 2021-03-12 24 preempt_disable();
f5fe12b1eaee22 Russell King 2018-05-14 @25 harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn,
f5fe12b1eaee22 Russell King 2018-05-14 26 smp_processor_id());
b31b47976c6f81 Zqiang 2021-03-12 27 preempt_enable();
f5fe12b1eaee22 Russell King 2018-05-14 28 if (fn)
f5fe12b1eaee22 Russell King 2018-05-14 29 fn();
f5fe12b1eaee22 Russell King 2018-05-14 30 }
f5fe12b1eaee22 Russell King 2018-05-14 31 #else
f5fe12b1eaee22 Russell King 2018-05-14 32 #define harden_branch_predictor() do { } while (0)
f5fe12b1eaee22 Russell King 2018-05-14 33 #endif
f5fe12b1eaee22 Russell King 2018-05-14 34
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12239 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-12 7:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 4:12 [PATCH] ARM: Fix incorrect use of smp_processor_id() by syzbot report qiang.zhang
2021-03-12 6:30 ` Dmitry Vyukov
2021-03-12 7:50 ` 回复: " Zhang, Qiang
2021-03-12 6:32 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).