Various Spectre-v1 fixes
Subsystems (3)
Peter Zijlstra
maintainercover Various Spectre-v1 fixes
These fix a number of perf, x86 and sched cases where we have user controlled array dereferences. All were found by Dan's recent Smatch update.
Peter Zijlstra
maintainerPATCH 1/7 perf: Fix possible Spectre-v1 for aux_pages
Quoted 1 line
kernel/events/ring_buffer.c:871perf_mmap_to_page()warn: potential spectre issue 'rb->aux_pages'
Userspace controls @pgoff through the fault address. Sanitize the array index before doing the array dereference.
--- kernel/events/ring_buffer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/events/ring_buffer.c +4 -2
@@ -867,8 +867,10 @@ perf_mmap_to_page(struct ring_buffer *rbreturn NULL ;/* AUX space */ -if (pgoff >= rb->aux_pgoff) -return virt_to_page (rb->aux_pages[pgoff - rb->aux_pgoff]); +if (pgoff >= rb->aux_pgoff) { +int aux_pgoff =array_index_nospec (pgoff - rb->aux_pgoff, rb->aux_nr_pages); +return virt_to_page (rb->aux_pages[aux_pgoff]); + } }return __perf_mmap_to_page (rb, pgoff);
Peter Zijlstra
maintainerPATCH 2/7 perf,x86: Fix possible Spectre-v1 for hw_perf_event
In reply to Peter ZijlstraQuoted 4 lines
arch/x86/events/core.c:319set_ext_hw_attr()warn: potential spectre issue 'hw_cache_event_ids[cache_type]' (local cap)arch/x86/events/core.c:319set_ext_hw_attr()warn: potential spectre issue 'hw_cache_event_ids' (local cap)arch/x86/events/core.c:328set_ext_hw_attr()warn: potential spectre issue 'hw_cache_extra_regs[cache_type]' (local cap)arch/x86/events/core.c:328set_ext_hw_attr()warn: potential spectre issue 'hw_cache_extra_regs' (local cap)
Userspace controls @config which contains 3 (byte) fields used for a 3 dimensional array deref.
--- arch/x86/events/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
arch/x86/events/core.c +4 -1
@@ -304,17 +304,20 @@ set_ext_hw_attr(struct hw_perf_event *hw config = attr->config; - cache_type = (config >>0 ) &0xff ; + cache_type = (config >>0 ) &0xff ;if (cache_type >=PERF_COUNT_HW_CACHE_MAX )return -EINVAL ; + cache_type =array_index_nospec (cache_type,PERF_COUNT_HW_CACHE_MAX ); cache_op = (config >>8 ) &0xff ;if (cache_op >=PERF_COUNT_HW_CACHE_OP_MAX )return -EINVAL ; + cache_op =array_index_nospec (cache_op,PERF_COUNT_HW_CACHE_OP_MAX ); cache_result = (config >>16 ) &0xff ;if (cache_result >=PERF_COUNT_HW_CACHE_RESULT_MAX )return -EINVAL ; + cache_result =array_index_nospec (cache_result,PERF_COUNT_HW_CACHE_RESULT_MAX ); val = hw_cache_event_ids[cache_type][cache_op][cache_result];
Peter Zijlstra
maintainerPATCH 3/7 perf,x86: Fix possible Spectre-v1 for x86_pmu::event_map()
In reply to Peter ZijlstraQuoted 5 lines
arch/x86/events/intel/core.c:337intel_pmu_event_map()warn: potential spectre issue 'intel_perfmon_event_map'arch/x86/events/intel/knc.c:122knc_pmu_event_map()warn: potential spectre issue 'knc_perfmon_event_map'arch/x86/events/intel/p4.c:722p4_pmu_event_map()warn: potential spectre issue 'p4_general_events'arch/x86/events/intel/p6.c:116p6_pmu_event_map()warn: potential spectre issue 'p6_perfmon_event_map'arch/x86/events/amd/core.c:132amd_pmu_event_map()warn: potential spectre issue 'amd_perfmon_event_map'
Userspace controls @attr, sanitize @attr->config before passing it on to x86_pmu::event_map().
--- arch/x86/events/core.c | 2 ++ 1 file changed, 2 insertions(+)
arch/x86/events/core.c +2 -0
@@ -424,6 +424,8 @@ int x86_setup_perfctr(struct perf_eventif (attr->config >= x86_pmu.max_events)return -EINVAL ; + attr->config =array_index_nospec (attr->config, x86_pmu.max_events); +/* * The generic map: */
Peter Zijlstra
maintainerPATCH 4/7 perf,x86/msr: Fix possible Spectre-v1 for msr
In reply to Peter ZijlstraQuoted 1 line
arch/x86/events/msr.c:178msr_event_init()warn: potential spectre issue 'msr' (local cap)
Userspace controls @attr, sanitize cfg (attr->config) before using it to index an array.
--- arch/x86/events/msr.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/events/msr.c +5 -3
@@ -158,9 +158,6 @@ static int msr_event_init(struct perf_evif (event->attr.type != event->pmu->type)return -ENOENT ; -if (cfg >=PERF_MSR_EVENT_MAX ) -return -EINVAL ; -/* unsupported modes and filters */ if (event->attr.exclude_user || event->attr.exclude_kernel ||
@@ -171,6 +168,11 @@ static int msr_event_init(struct perf_ev event->attr.sample_period) /* no sampling */return -EINVAL ; +if (cfg >=PERF_MSR_EVENT_MAX ) +return -EINVAL ; + + cfg =array_index_nospec (cfg,PERF_MSR_EVENT_MAX ); +if (!msr[cfg].attr)return -EINVAL ;
Peter Zijlstra
maintainerPATCH 5/7 perf,x86/cstate: Fix possible Spectre-v1 for pkg_msr
In reply to Peter ZijlstraQuoted 1 line
arch/x86/events/intel/cstate.c:307cstate_pmu_event_init()warn: potential spectre issue 'pkg_msr' (local cap)
Userspace controls @attr, sanitize cfg (attr->config) before using it to index an array.
--- arch/x86/events/intel/cstate.c | 1 + 1 file changed, 1 insertion(+)
arch/x86/events/intel/cstate.c +1 -0
@@ -302,6 +302,7 @@ static int cstate_pmu_event_init(struct }else if (event->pmu == &cstate_pkg_pmu) {if (cfg >=PERF_CSTATE_PKG_EVENT_MAX )return -EINVAL ; + cfg =array_index_nospec (cfg,PERF_CSTATE_PKG_EVENT_MAX );if (!pkg_msr[cfg].attr)return -EINVAL ; event->hw.event_base = pkg_msr[cfg].msr;
Peter Zijlstra
maintainerPATCH 6/7 sched: Fix possible Spectre-v1 for sched_prio_to_weight[]
In reply to Peter ZijlstraQuoted 1 line
kernel/sched/core.c:6921cpu_weight_nice_write_s64()warn: potential spectre issue 'sched_prio_to_weight'
Userspace controls @nice, so sanitize the value before using it to index an array.
--- kernel/sched/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
kernel/sched/core.c +4 -1
@@ -6928,11 +6928,14 @@ static int cpu_weight_nice_write_s64(str struct cftype *cft, s64 nice) {unsigned long weight; +int idx;if (nice <MIN_NICE || nice >MAX_NICE )return -ERANGE ; - weight = sched_prio_to_weight[NICE_TO_PRIO (nice) -MAX_RT_PRIO ]; + idx =array_index_nospec (NICE_TO_PRIO (nice) -MAX_RT_PRIO ,40 ); + weight = sched_prio_to_weight[idx]; +return sched_group_set_shares (css_tg (css),scale_load (weight)); }#endif
Peter Zijlstra
maintainerPATCH 7/7 sched,autogroup: Fix possible Spectre-v1 for sched_prio_to_weight
In reply to Peter ZijlstraQuoted 1 line
kernel/sched/autogroup.c:230proc_sched_autogroup_set_nice()warn: potential spectre issue 'sched_prio_to_weight'
Userspace controls @nice, sanitize the array index.
--- kernel/sched/autogroup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/sched/autogroup.c +4 -2
@@ -209,7 +209,7 @@ int proc_sched_autogroup_set_nice(structstatic unsigned long next =INITIAL_JIFFIES ;struct autogroup *ag;unsigned long shares; -int err; +int err, idx;if (nice <MIN_NICE || nice >MAX_NICE )return -EINVAL ;
@@ -227,7 +227,9 @@ int proc_sched_autogroup_set_nice(struct next =HZ /10 + jiffies; ag =autogroup_task_get (p); - shares =scale_load (sched_prio_to_weight[nice +20 ]); + + idx =array_index_nospec (nice +20 ,40 ); + shares =scale_load (sched_prio_to_weight[idx]);down_write (&ag->lock); err =sched_group_set_shares (ag->tg, shares);
Quoted 2 lines · Peter Zijlstra
These fix a number of perf, x86 and sched cases where we have user controlled array dereferences. All were found by Dan's recent Smatch update.
Do we want to mark all of these for stable?
Signature · 2 lines
Michal Hocko SUSE Labs
Peter Zijlstra
maintainerQuoted 5 lines · Michal Hocko
Quoted 2 lines · Peter Zijlstra
These fix a number of perf, x86 and sched cases where we have user controlled array dereferences. All were found by Dan's recent Smatch update.
Do we want to mark all of these for stable?
If we all agree that any (speculative) user-controlled array index -- irrespective of the existence of the second load/store that would complete the gadget -- needs fixing and thus all these patches are 'good', then yes, that makes sense.
Given yours is the only response so far, I suspect we're good on all that :-)
Quoted 7 lines · Peter Zijlstra
Quoted 5 lines · Michal Hocko
Quoted 2 lines · Peter Zijlstra
These fix a number of perf, x86 and sched cases where we have user controlled array dereferences. All were found by Dan's recent Smatch update.
Do we want to mark all of these for stable?
Quoted 7 lines
If we all agree that any (speculative) user-controlled array index -- irrespective of the existence of the second load/store that would complete the gadget -- needs fixing and thus all these patches are 'good', then yes, that makes sense.
Given yours is the only response so far, I suspect we're good on all that :-)
+1 from me for -stable.
Quoted 11 lines · Peter Zijlstra
Quoted 5 lines · Michal Hocko
Quoted 2 lines · Peter Zijlstra
These fix a number of perf, x86 and sched cases where we have user controlled array dereferences. All were found by Dan's recent Smatch update.
Do we want to mark all of these for stable?
If we all agree that any (speculative) user-controlled array index -- irrespective of the existence of the second load/store that would complete the gadget -- needs fixing and thus all these patches are 'good', then yes, that makes sense.
Well, I would rather be on the safe side (have I heard security by fear?). So if those patches are landing in upstream then I would vote to mark them for stable. They should be trivial to backport and shouldn't cause regressions that makes them more suitable stable candidates than many I have seen recently...
Signature · 2 lines
Michal Hocko SUSE Labs