Re: CVE-2024-40938: landlock: Fix d_parent walk
On Mon, Jul 15, 2024 at 01:17:10PM -0700, Kees Cook wrote: > On Mon, Jul 15, 2024 at 08:04:21PM +0200, Mickaël Salaün wrote: > > Yes, that's why we use WARN_ON_ONCE() to check cases that should never > > happen (at the time of writting), but in practice it's useful to check > > (with fuzzing) that this assertion is true. However, if a > > WARN_ON_ONCE() is reached, this doesn't mean that this is a security > > issue, but just an unexpected case that kernel maintainers should be > > notified with to fix it. > > I leave CVE determinations to the CNA. :) I think the difficulty here is > with having no way to trivially see which WARN is security sensitive and > which isn't, and since WARNs may panic, all WARNs could be a DoS, and > therefore may be a CVE for some deployment somewhere. That is exactly correct, and why we must mark any way that userspace can hit a WARN as needing a CVE. thanks, greg k-h
Re: [PATCH v4 00/11] Add per-core RAPL energy counter support for AMD CPUs
Hello Ian, On 7/15/2024 8:52 PM, Ian Rogers wrote: > On Mon, Jul 15, 2024 at 2:36 AM Dhananjay Ugwekar > wrote: >> >> Hello Ian, >> >> On 7/12/2024 3:53 AM, Ian Rogers wrote: >>> On Thu, Jul 11, 2024 at 3:25 AM Dhananjay Ugwekar >>> wrote: Currently the energy-cores event in the power PMU aggregates energy consumption data at a package level. On the other hand the core energy RAPL counter in AMD CPUs has a core scope (which means the energy consumption is recorded separately for each core). Earlier efforts to add the core event in the power PMU had failed [1], due to the difference in the scope of these two events. Hence, there is a need for a new core scope PMU. This patchset adds a new "power_per_core" PMU alongside the existing "power" PMU, which will be responsible for collecting the new "energy-per-core" event. >>> >>> Sorry for being naive, is the only reason for adding the new PMU for >>> the sake of the cpumask? Perhaps we can add per event cpumasks like >>> say `/sys/devices/power/events/energy-per-core.cpumask` which contains >>> the CPUs of the different cores in this case. There's supporting >>> hotplugging CPUs as an issue. Adding the tool support for this >>> wouldn't be hard and it may be less messy (although old perf tools on >>> new kernels wouldn't know about these files). >> >> I went over the two approaches and below are my thoughts, >> >> New PMU approach: >> Pros >> * It will work with older perf tools, hence these patches can be backported >> to an older kernel and the new per-core event will work there as well. >> Cons >> * More code changes in rapl.c >> >> Event specific cpumask approach: >> Pros >> * It might be easier to add diff scope events within the same PMU in >> future(although currently I'm not able to find such a usecase, apart from >> the RAPL pkg and core energy counters) >> Cons >> * Both new kernel and perf tool will be required to use the new per-core >> event. >> >> I feel that while the event-specific cpumask is a viable alternative to the >> new PMU addition approach, I dont see any clear pros to select that over the >> current approach. Please let me know if you have any design related concerns >> to the addition of new PMU or your concern is mostly about the amount of >> code changes in this approach. > > Thanks Dhananjay, and thanks for taking the time for an objective > discussion on the mailing list. I'm very supportive of seeing the work > you are enabling land. > > My concern comes from the tool side. If every PMU starts to have > variants for the sake of the cpumask what does this mean for > aggregation in the perf tool? There is another issue around event > grouping, you can't group events across PMUs, but my feeling is that > event grouping needs to be rethought. By default the power_per_core > events are going to be aggregated together by the perf tool, which > then loses their per_core-ness. Yea right, maybe we need to fix this behavior. > > I was trying to think of the same problem but in other PMUs. One > thought I had was the difference between hyperthread and core events. > At least on Intel, some events can only count for the whole core not > per hyperthread. The events don't have a cpu_per_core PMU, they just > use the regular cpu one, and so the cpumask is set to all online > hyperthreads. When a per-core event is programmed it will get > programmed on every hyperthread and so counted twice for the core. > This at the least wastes a counter, but it probably also yields twice > the expected count as every event is counted twice then aggregated. So > this is just wrong and the user is expected to be smart and fix it > (checking the x86 events there is a convention to use a ".ALL" or > ".ANY" suffix for core wide events iirc). If we had a cpumask for > these events then we could avoid the double setting, free up a counter > and avoid double counting. Were we to fix things the way it is done in > this patch series we'd add another PMU. Yes, this seems like a valid usecase for event-specific cpumasks. > > My feeling is that in the longer term a per event cpumask looks > cleaner. I think either way you need a new kernel for the new RAPL > events. The problem with an old perf tool and a new kernel, this > doesn't normally happen with distributions as they match the perf tool > to the kernel version needlessly losing features and fixes along the > way. If the new PMU is going to get backported through fixes.. then we > can do similar for reading the per event cpumask. I'd be tempted not > to do this and focus on the next LTS kernel, getting the kernel and > tool fixes in as necessary. Makes sense, even though this approach will require more effort but it seems to be worthwhile as it would help things down the line (make it easier to have heterogenous-scope events within a PMU). I'll need to go through the perf tool to see how we can design this. I'll get back with an RFC
Re: [PATCH v4 02/11] perf/x86/rapl: Fix the energy-pkg event for AMD CPUs
Hello Peter, Rui, After Ian's comments on the series, I have decided to rethink the approach of adding a new PMU for the per-core RAPL counters. However this patch is still needed as a fix for the commit ("x86/cpu/topology: Add support for the AMD 0x8026 leaf"), I will be sending this separately along with a similar fix for powercap/intel_rapl_common. Thanks, Dhananjay On 7/11/2024 3:54 PM, Dhananjay Ugwekar wrote: > After commit ("x86/cpu/topology: Add support for the AMD 0x8026 leaf"), > on AMD processors that support extended CPUID leaf 0x8026, the > topology_die_cpumask() and topology_logical_die_id() macros, no longer > return the package cpumask and package id, instead they return the CCD > (Core Complex Die) mask and id respectively. This leads to the energy-pkg > event scope to be modified to CCD instead of package. > > Replacing these macros with their package counterparts fixes the > energy-pkg event for AMD CPUs. > > However due to the difference between the scope of energy-pkg event for > Intel and AMD CPUs, we have to replace these macros conditionally only for > AMD CPUs. > > On a 12 CCD 1 Package AMD Zen4 Genoa machine: > > Before: > $ cat /sys/devices/power/cpumask > 0,8,16,24,32,40,48,56,64,72,80,88. > > The expected cpumask here is supposed to be just "0", as it is a package > scope event, only one CPU will be collecting the event for all the CPUs in > the package. > > After: > $ cat /sys/devices/power/cpumask > 0 > > Signed-off-by: Dhananjay Ugwekar > Fixes: 63edbaa48a57 ("x86/cpu/topology: Add support for the AMD 0x8026 > leaf") > --- > Changes in v4: > * Invert the pkg scope check in init_rapl_pmus() (Peter) > * Add comments to explain the pkg scope check (Peter) > > PS: Scope check logic is still kept the same (i.e., all Intel systems being > considered as die scope), Rui will be modifying it to limit the die-scope > only to Cascadelake-AP in a future patch on top of this patchset. > --- > arch/x86/events/rapl.c | 39 ++- > 1 file changed, 34 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c > index 0c5e7a7c43ac..df71f38ad98d 100644 > --- a/arch/x86/events/rapl.c > +++ b/arch/x86/events/rapl.c > @@ -103,6 +103,13 @@ static struct perf_pmu_events_attr event_attr_##v = { > \ > .event_str = str, > \ > }; > > +/* > + * RAPL PMU scope for AMD is package whereas for Intel it is die. > + */ > +#define rapl_pmu_is_pkg_scope() \ > + (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || \ > + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) > + > struct rapl_pmu { > raw_spinlock_t lock; > int n_active; > @@ -140,9 +147,25 @@ static unsigned int rapl_cntr_mask; > static u64 rapl_timer_ms; > static struct perf_msr *rapl_msrs; > > +/* > + * Helper functions to get the correct topology macros according to the > + * RAPL PMU scope. > + */ > +static inline unsigned int get_rapl_pmu_idx(int cpu) > +{ > + return rapl_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) : > + topology_logical_die_id(cpu); > +} > + > +static inline const struct cpumask *get_rapl_pmu_cpumask(int cpu) > +{ > + return rapl_pmu_is_pkg_scope() ? topology_core_cpumask(cpu) : > + topology_die_cpumask(cpu); > +} > + > static inline struct rapl_pmu *cpu_to_rapl_pmu(unsigned int cpu) > { > - unsigned int rapl_pmu_idx = topology_logical_die_id(cpu); > + unsigned int rapl_pmu_idx = get_rapl_pmu_idx(cpu); > > /* >* The unsigned check also catches the '-1' return value for non > @@ -543,6 +566,7 @@ static struct perf_msr amd_rapl_msrs[] = { > > static int rapl_cpu_offline(unsigned int cpu) > { > + const struct cpumask *rapl_pmu_cpumask = get_rapl_pmu_cpumask(cpu); > struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu); > int target; > > @@ -552,7 +576,7 @@ static int rapl_cpu_offline(unsigned int cpu) > > pmu->cpu = -1; > /* Find a new cpu to collect rapl events */ > - target = cpumask_any_but(topology_die_cpumask(cpu), cpu); > + target = cpumask_any_but(rapl_pmu_cpumask, cpu); > > /* Migrate rapl events to the new target */ > if (target < nr_cpu_ids) { > @@ -565,6 +589,8 @@ static int rapl_cpu_offline(unsigned int cpu) > > static int rapl_cpu_online(unsigned int cpu) > { > + unsigned int rapl_pmu_idx = get_rapl_pmu_idx(cpu); > + const struct cpumask *rapl_pmu_cpumask = get_rapl_pmu_cpumask(cpu); > struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu); > int target; > > @@ -579,14 +605,14 @@ static int rapl_cpu_online(unsigned int cpu) > pmu->timer_interval = ms_to_ktime(rapl_timer_ms); > rapl_hrtimer_init(pmu); > > - rapl_pmus->pmus[topology_lo
Re: [PATCH v2] net/ipv4/tcp_cong: Replace strncpy() with strscpy()
On 7/15/24 11:41, Simon Horman wrote: On Sat, Jul 13, 2024 at 09:11:15PM -0700, Kees Cook wrote: Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() and tcp_get_default_congestion_control(). The callers use the results as standard C strings (via nla_put_string() and proc handlers respectively), so trailing padding is not needed. Since passing the destination buffer arguments decays it to a pointer, the size can't be trivially determined by the compiler. ca->name is the same length in both cases, so strscpy() won't fail (when ca->name is NUL-terminated). Include the length explicitly instead of using the 2-argument strscpy(). Link: https://github.com/KSPP/linux/issues/90 [1] Signed-off-by: Kees Cook nit: Looking at git history, the subject prefix should probably be 'tcp'. And it would be best to explicitly target the patch against net-next. Subject: [PATCH net-next v2] tcp: ... That notwithstanding, this looks good to me. Reviewed-by: Simon Horman @Eric: I can fix the prefix when applying the patch. Please LMK if you prefer otherwise. Thanks, Paolo
[PATCH AUTOSEL 6.9 03/22] powerpc/pseries: Whitelist dtl slub object for copying to userspace
From: Anjali K [ Upstream commit 1a14150e1656f7a332a943154fc486504db4d586 ] Reading the dispatch trace log from /sys/kernel/debug/powerpc/dtl/cpu-* results in a BUG() when the config CONFIG_HARDENED_USERCOPY is enabled as shown below. kernel BUG at mm/usercopy.c:102! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries Modules linked in: xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth pseries_wdt dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 27 PID: 1815 Comm: python3 Not tainted 6.10.0-rc3 #85 Hardware name: IBM,9040-MRX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1060.00 (NM1060_042) hv:phyp pSeries NIP: c05d23d4 LR: c05d23d0 CTR: 006ee6f8 REGS: c00120c078c0 TRAP: 0700 Not tainted (6.10.0-rc3) MSR: 80029033 CR: 2828220f XER: 000e CFAR: c01fdc80 IRQMASK: 0 [ ... GPRs omitted ... ] NIP [c05d23d4] usercopy_abort+0x78/0xb0 LR [c05d23d0] usercopy_abort+0x74/0xb0 Call Trace: usercopy_abort+0x74/0xb0 (unreliable) __check_heap_object+0xf8/0x120 check_heap_object+0x218/0x240 __check_object_size+0x84/0x1a4 dtl_file_read+0x17c/0x2c4 full_proxy_read+0x8c/0x110 vfs_read+0xdc/0x3a0 ksys_read+0x84/0x144 system_call_exception+0x124/0x330 system_call_vectored_common+0x15c/0x2ec --- interrupt: 3000 at 0x7fff81f3ab34 Commit 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0") requires that only whitelisted areas in slab/slub objects can be copied to userspace when usercopy hardening is enabled using CONFIG_HARDENED_USERCOPY. Dtl contains hypervisor dispatch events which are expected to be read by privileged users. Hence mark this safe for user access. Specify useroffset=0 and usersize=DISPATCH_LOG_BYTES to whitelist the entire object. Co-developed-by: Vishal Chourasia Signed-off-by: Vishal Chourasia Signed-off-by: Anjali K Reviewed-by: Srikar Dronamraju Signed-off-by: Michael Ellerman Link: https://msgid.link/20240614173844.746818-1-anja...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/platforms/pseries/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 284a6fa04b0c2..cba40d9d12848 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -343,8 +343,8 @@ static int alloc_dispatch_log_kmem_cache(void) { void (*ctor)(void *) = get_dtl_cache_ctor(); - dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES, - DISPATCH_LOG_BYTES, 0, ctor); + dtl_cache = kmem_cache_create_usercopy("dtl", DISPATCH_LOG_BYTES, + DISPATCH_LOG_BYTES, 0, 0, DISPATCH_LOG_BYTES, ctor); if (!dtl_cache) { pr_warn("Failed to create dispatch trace log buffer cache\n"); pr_warn("Stolen time statistics will be unreliable\n"); -- 2.43.0
[PATCH AUTOSEL 6.6 02/18] powerpc/pseries: Whitelist dtl slub object for copying to userspace
From: Anjali K [ Upstream commit 1a14150e1656f7a332a943154fc486504db4d586 ] Reading the dispatch trace log from /sys/kernel/debug/powerpc/dtl/cpu-* results in a BUG() when the config CONFIG_HARDENED_USERCOPY is enabled as shown below. kernel BUG at mm/usercopy.c:102! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries Modules linked in: xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth pseries_wdt dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 27 PID: 1815 Comm: python3 Not tainted 6.10.0-rc3 #85 Hardware name: IBM,9040-MRX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1060.00 (NM1060_042) hv:phyp pSeries NIP: c05d23d4 LR: c05d23d0 CTR: 006ee6f8 REGS: c00120c078c0 TRAP: 0700 Not tainted (6.10.0-rc3) MSR: 80029033 CR: 2828220f XER: 000e CFAR: c01fdc80 IRQMASK: 0 [ ... GPRs omitted ... ] NIP [c05d23d4] usercopy_abort+0x78/0xb0 LR [c05d23d0] usercopy_abort+0x74/0xb0 Call Trace: usercopy_abort+0x74/0xb0 (unreliable) __check_heap_object+0xf8/0x120 check_heap_object+0x218/0x240 __check_object_size+0x84/0x1a4 dtl_file_read+0x17c/0x2c4 full_proxy_read+0x8c/0x110 vfs_read+0xdc/0x3a0 ksys_read+0x84/0x144 system_call_exception+0x124/0x330 system_call_vectored_common+0x15c/0x2ec --- interrupt: 3000 at 0x7fff81f3ab34 Commit 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0") requires that only whitelisted areas in slab/slub objects can be copied to userspace when usercopy hardening is enabled using CONFIG_HARDENED_USERCOPY. Dtl contains hypervisor dispatch events which are expected to be read by privileged users. Hence mark this safe for user access. Specify useroffset=0 and usersize=DISPATCH_LOG_BYTES to whitelist the entire object. Co-developed-by: Vishal Chourasia Signed-off-by: Vishal Chourasia Signed-off-by: Anjali K Reviewed-by: Srikar Dronamraju Signed-off-by: Michael Ellerman Link: https://msgid.link/20240614173844.746818-1-anja...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/platforms/pseries/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index ecea85c74c43f..abc086b53e017 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -343,8 +343,8 @@ static int alloc_dispatch_log_kmem_cache(void) { void (*ctor)(void *) = get_dtl_cache_ctor(); - dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES, - DISPATCH_LOG_BYTES, 0, ctor); + dtl_cache = kmem_cache_create_usercopy("dtl", DISPATCH_LOG_BYTES, + DISPATCH_LOG_BYTES, 0, 0, DISPATCH_LOG_BYTES, ctor); if (!dtl_cache) { pr_warn("Failed to create dispatch trace log buffer cache\n"); pr_warn("Stolen time statistics will be unreliable\n"); -- 2.43.0
[PATCH AUTOSEL 6.1 02/15] powerpc/pseries: Whitelist dtl slub object for copying to userspace
From: Anjali K [ Upstream commit 1a14150e1656f7a332a943154fc486504db4d586 ] Reading the dispatch trace log from /sys/kernel/debug/powerpc/dtl/cpu-* results in a BUG() when the config CONFIG_HARDENED_USERCOPY is enabled as shown below. kernel BUG at mm/usercopy.c:102! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries Modules linked in: xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth pseries_wdt dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 27 PID: 1815 Comm: python3 Not tainted 6.10.0-rc3 #85 Hardware name: IBM,9040-MRX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1060.00 (NM1060_042) hv:phyp pSeries NIP: c05d23d4 LR: c05d23d0 CTR: 006ee6f8 REGS: c00120c078c0 TRAP: 0700 Not tainted (6.10.0-rc3) MSR: 80029033 CR: 2828220f XER: 000e CFAR: c01fdc80 IRQMASK: 0 [ ... GPRs omitted ... ] NIP [c05d23d4] usercopy_abort+0x78/0xb0 LR [c05d23d0] usercopy_abort+0x74/0xb0 Call Trace: usercopy_abort+0x74/0xb0 (unreliable) __check_heap_object+0xf8/0x120 check_heap_object+0x218/0x240 __check_object_size+0x84/0x1a4 dtl_file_read+0x17c/0x2c4 full_proxy_read+0x8c/0x110 vfs_read+0xdc/0x3a0 ksys_read+0x84/0x144 system_call_exception+0x124/0x330 system_call_vectored_common+0x15c/0x2ec --- interrupt: 3000 at 0x7fff81f3ab34 Commit 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0") requires that only whitelisted areas in slab/slub objects can be copied to userspace when usercopy hardening is enabled using CONFIG_HARDENED_USERCOPY. Dtl contains hypervisor dispatch events which are expected to be read by privileged users. Hence mark this safe for user access. Specify useroffset=0 and usersize=DISPATCH_LOG_BYTES to whitelist the entire object. Co-developed-by: Vishal Chourasia Signed-off-by: Vishal Chourasia Signed-off-by: Anjali K Reviewed-by: Srikar Dronamraju Signed-off-by: Michael Ellerman Link: https://msgid.link/20240614173844.746818-1-anja...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/platforms/pseries/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index a0701dbdb1348..09372361f1080 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -342,8 +342,8 @@ static int alloc_dispatch_log_kmem_cache(void) { void (*ctor)(void *) = get_dtl_cache_ctor(); - dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES, - DISPATCH_LOG_BYTES, 0, ctor); + dtl_cache = kmem_cache_create_usercopy("dtl", DISPATCH_LOG_BYTES, + DISPATCH_LOG_BYTES, 0, 0, DISPATCH_LOG_BYTES, ctor); if (!dtl_cache) { pr_warn("Failed to create dispatch trace log buffer cache\n"); pr_warn("Stolen time statistics will be unreliable\n"); -- 2.43.0
[PATCH AUTOSEL 5.15 2/9] powerpc/pseries: Whitelist dtl slub object for copying to userspace
From: Anjali K [ Upstream commit 1a14150e1656f7a332a943154fc486504db4d586 ] Reading the dispatch trace log from /sys/kernel/debug/powerpc/dtl/cpu-* results in a BUG() when the config CONFIG_HARDENED_USERCOPY is enabled as shown below. kernel BUG at mm/usercopy.c:102! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries Modules linked in: xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth pseries_wdt dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 27 PID: 1815 Comm: python3 Not tainted 6.10.0-rc3 #85 Hardware name: IBM,9040-MRX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1060.00 (NM1060_042) hv:phyp pSeries NIP: c05d23d4 LR: c05d23d0 CTR: 006ee6f8 REGS: c00120c078c0 TRAP: 0700 Not tainted (6.10.0-rc3) MSR: 80029033 CR: 2828220f XER: 000e CFAR: c01fdc80 IRQMASK: 0 [ ... GPRs omitted ... ] NIP [c05d23d4] usercopy_abort+0x78/0xb0 LR [c05d23d0] usercopy_abort+0x74/0xb0 Call Trace: usercopy_abort+0x74/0xb0 (unreliable) __check_heap_object+0xf8/0x120 check_heap_object+0x218/0x240 __check_object_size+0x84/0x1a4 dtl_file_read+0x17c/0x2c4 full_proxy_read+0x8c/0x110 vfs_read+0xdc/0x3a0 ksys_read+0x84/0x144 system_call_exception+0x124/0x330 system_call_vectored_common+0x15c/0x2ec --- interrupt: 3000 at 0x7fff81f3ab34 Commit 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0") requires that only whitelisted areas in slab/slub objects can be copied to userspace when usercopy hardening is enabled using CONFIG_HARDENED_USERCOPY. Dtl contains hypervisor dispatch events which are expected to be read by privileged users. Hence mark this safe for user access. Specify useroffset=0 and usersize=DISPATCH_LOG_BYTES to whitelist the entire object. Co-developed-by: Vishal Chourasia Signed-off-by: Vishal Chourasia Signed-off-by: Anjali K Reviewed-by: Srikar Dronamraju Signed-off-by: Michael Ellerman Link: https://msgid.link/20240614173844.746818-1-anja...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/platforms/pseries/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index d25053755c8b8..309a72518ecc3 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -314,8 +314,8 @@ static int alloc_dispatch_log_kmem_cache(void) { void (*ctor)(void *) = get_dtl_cache_ctor(); - dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES, - DISPATCH_LOG_BYTES, 0, ctor); + dtl_cache = kmem_cache_create_usercopy("dtl", DISPATCH_LOG_BYTES, + DISPATCH_LOG_BYTES, 0, 0, DISPATCH_LOG_BYTES, ctor); if (!dtl_cache) { pr_warn("Failed to create dispatch trace log buffer cache\n"); pr_warn("Stolen time statistics will be unreliable\n"); -- 2.43.0
[PATCH AUTOSEL 5.10 2/7] powerpc/pseries: Whitelist dtl slub object for copying to userspace
From: Anjali K [ Upstream commit 1a14150e1656f7a332a943154fc486504db4d586 ] Reading the dispatch trace log from /sys/kernel/debug/powerpc/dtl/cpu-* results in a BUG() when the config CONFIG_HARDENED_USERCOPY is enabled as shown below. kernel BUG at mm/usercopy.c:102! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries Modules linked in: xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth pseries_wdt dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 27 PID: 1815 Comm: python3 Not tainted 6.10.0-rc3 #85 Hardware name: IBM,9040-MRX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1060.00 (NM1060_042) hv:phyp pSeries NIP: c05d23d4 LR: c05d23d0 CTR: 006ee6f8 REGS: c00120c078c0 TRAP: 0700 Not tainted (6.10.0-rc3) MSR: 80029033 CR: 2828220f XER: 000e CFAR: c01fdc80 IRQMASK: 0 [ ... GPRs omitted ... ] NIP [c05d23d4] usercopy_abort+0x78/0xb0 LR [c05d23d0] usercopy_abort+0x74/0xb0 Call Trace: usercopy_abort+0x74/0xb0 (unreliable) __check_heap_object+0xf8/0x120 check_heap_object+0x218/0x240 __check_object_size+0x84/0x1a4 dtl_file_read+0x17c/0x2c4 full_proxy_read+0x8c/0x110 vfs_read+0xdc/0x3a0 ksys_read+0x84/0x144 system_call_exception+0x124/0x330 system_call_vectored_common+0x15c/0x2ec --- interrupt: 3000 at 0x7fff81f3ab34 Commit 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0") requires that only whitelisted areas in slab/slub objects can be copied to userspace when usercopy hardening is enabled using CONFIG_HARDENED_USERCOPY. Dtl contains hypervisor dispatch events which are expected to be read by privileged users. Hence mark this safe for user access. Specify useroffset=0 and usersize=DISPATCH_LOG_BYTES to whitelist the entire object. Co-developed-by: Vishal Chourasia Signed-off-by: Vishal Chourasia Signed-off-by: Anjali K Reviewed-by: Srikar Dronamraju Signed-off-by: Michael Ellerman Link: https://msgid.link/20240614173844.746818-1-anja...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/platforms/pseries/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 822be2680b792..8e4a2e8aee114 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -312,8 +312,8 @@ static int alloc_dispatch_log_kmem_cache(void) { void (*ctor)(void *) = get_dtl_cache_ctor(); - dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES, - DISPATCH_LOG_BYTES, 0, ctor); + dtl_cache = kmem_cache_create_usercopy("dtl", DISPATCH_LOG_BYTES, + DISPATCH_LOG_BYTES, 0, 0, DISPATCH_LOG_BYTES, ctor); if (!dtl_cache) { pr_warn("Failed to create dispatch trace log buffer cache\n"); pr_warn("Stolen time statistics will be unreliable\n"); -- 2.43.0
[PATCH AUTOSEL 5.4 2/7] powerpc/pseries: Whitelist dtl slub object for copying to userspace
From: Anjali K [ Upstream commit 1a14150e1656f7a332a943154fc486504db4d586 ] Reading the dispatch trace log from /sys/kernel/debug/powerpc/dtl/cpu-* results in a BUG() when the config CONFIG_HARDENED_USERCOPY is enabled as shown below. kernel BUG at mm/usercopy.c:102! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries Modules linked in: xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth pseries_wdt dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 27 PID: 1815 Comm: python3 Not tainted 6.10.0-rc3 #85 Hardware name: IBM,9040-MRX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1060.00 (NM1060_042) hv:phyp pSeries NIP: c05d23d4 LR: c05d23d0 CTR: 006ee6f8 REGS: c00120c078c0 TRAP: 0700 Not tainted (6.10.0-rc3) MSR: 80029033 CR: 2828220f XER: 000e CFAR: c01fdc80 IRQMASK: 0 [ ... GPRs omitted ... ] NIP [c05d23d4] usercopy_abort+0x78/0xb0 LR [c05d23d0] usercopy_abort+0x74/0xb0 Call Trace: usercopy_abort+0x74/0xb0 (unreliable) __check_heap_object+0xf8/0x120 check_heap_object+0x218/0x240 __check_object_size+0x84/0x1a4 dtl_file_read+0x17c/0x2c4 full_proxy_read+0x8c/0x110 vfs_read+0xdc/0x3a0 ksys_read+0x84/0x144 system_call_exception+0x124/0x330 system_call_vectored_common+0x15c/0x2ec --- interrupt: 3000 at 0x7fff81f3ab34 Commit 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0") requires that only whitelisted areas in slab/slub objects can be copied to userspace when usercopy hardening is enabled using CONFIG_HARDENED_USERCOPY. Dtl contains hypervisor dispatch events which are expected to be read by privileged users. Hence mark this safe for user access. Specify useroffset=0 and usersize=DISPATCH_LOG_BYTES to whitelist the entire object. Co-developed-by: Vishal Chourasia Signed-off-by: Vishal Chourasia Signed-off-by: Anjali K Reviewed-by: Srikar Dronamraju Signed-off-by: Michael Ellerman Link: https://msgid.link/20240614173844.746818-1-anja...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/platforms/pseries/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index d5abb25865e36..bc1a4e024529b 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -305,8 +305,8 @@ static int alloc_dispatch_log_kmem_cache(void) { void (*ctor)(void *) = get_dtl_cache_ctor(); - dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES, - DISPATCH_LOG_BYTES, 0, ctor); + dtl_cache = kmem_cache_create_usercopy("dtl", DISPATCH_LOG_BYTES, + DISPATCH_LOG_BYTES, 0, 0, DISPATCH_LOG_BYTES, ctor); if (!dtl_cache) { pr_warn("Failed to create dispatch trace log buffer cache\n"); pr_warn("Stolen time statistics will be unreliable\n"); -- 2.43.0
Re: [PATCH v2] net/ipv4/tcp_cong: Replace strncpy() with strscpy()
On Tue, Jul 16, 2024 at 4:32 AM Paolo Abeni wrote: > > On 7/15/24 11:41, Simon Horman wrote: > > On Sat, Jul 13, 2024 at 09:11:15PM -0700, Kees Cook wrote: > >> Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() > >> and tcp_get_default_congestion_control(). The callers use the results as > >> standard C strings (via nla_put_string() and proc handlers respectively), > >> so trailing padding is not needed. > >> > >> Since passing the destination buffer arguments decays it to a pointer, > >> the size can't be trivially determined by the compiler. ca->name is > >> the same length in both cases, so strscpy() won't fail (when ca->name > >> is NUL-terminated). Include the length explicitly instead of using the > >> 2-argument strscpy(). > >> > >> Link: https://github.com/KSPP/linux/issues/90 [1] > >> Signed-off-by: Kees Cook > > > > nit: Looking at git history, the subject prefix should probably be 'tcp'. > > And it would be best to explicitly target the patch against net-next. > > > > Subject: [PATCH net-next v2] tcp: ... > > > > That notwithstanding, this looks good to me. > > > > Reviewed-by: Simon Horman > > @Eric: I can fix the prefix when applying the patch. Please LMK if you > prefer otherwise. Sure thing, thanks for taking care of this Paolo, Simon, and Kees. Reviewed-by: Eric Dumazet
Re: [PATCH v2] net/ipv4/tcp_cong: Replace strncpy() with strscpy()
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski : On Sat, 13 Jul 2024 21:11:15 -0700 you wrote: > Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() > and tcp_get_default_congestion_control(). The callers use the results as > standard C strings (via nla_put_string() and proc handlers respectively), > so trailing padding is not needed. > > Since passing the destination buffer arguments decays it to a pointer, > the size can't be trivially determined by the compiler. ca->name is > the same length in both cases, so strscpy() won't fail (when ca->name > is NUL-terminated). Include the length explicitly instead of using the > 2-argument strscpy(). > > [...] Here is the summary with links: - [v2] net/ipv4/tcp_cong: Replace strncpy() with strscpy() https://git.kernel.org/netdev/net-next/c/a3bfc095060b You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH] wifi: wireless: fix more UBSAN noise in cfg80211_conn_scan()
Looking at https://syzkaller.appspot.com/bug?extid=d5dc2801166df6d34774 and trying to reproduce it with CONFIG_UBSAN enabled, I've noticed the following: UBSAN: array-index-out-of-bounds in net/wireless/sme.c:95:3 index 0 is out of range for type 'struct ieee80211_channel *[]' CPU: 3 PID: 4993 Comm: repro Not tainted 6.10.0-01155-gd67978318827 #5 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS <...> Call Trace: dump_stack_lvl+0x1c2/0x2a0 ? __pfx_dump_stack_lvl+0x10/0x10 ? __pfx__printk+0x10/0x10 ? __local_bh_enable_ip+0x12e/0x1c0 __ubsan_handle_out_of_bounds+0x127/0x150 cfg80211_conn_scan+0xd8e/0xf30 cfg80211_connect+0x1400/0x1c30 nl80211_connect+0x1549/0x1a70 ... This is very similar to 92ecbb3ac6f3 ("wifi: mac80211: fix UBSAN noise in ieee80211_prep_hw_scan()"), so just fix it in the same way by setting 'request->n_channels' early to help '__counted_by()' work as expected. And the same 'kmalloc()' math adjustment is also applicable. Signed-off-by: Dmitry Antipov --- net/wireless/sme.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/net/wireless/sme.c b/net/wireless/sme.c index a8ad55f11133..f5da45331847 100644 --- a/net/wireless/sme.c +++ b/net/wireless/sme.c @@ -77,12 +77,16 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev) else n_channels = ieee80211_get_num_supported_channels(wdev->wiphy); - request = kzalloc(sizeof(*request) + sizeof(request->ssids[0]) + - sizeof(request->channels[0]) * n_channels, - GFP_KERNEL); + request = kzalloc(struct_size(request, channels, n_channels) + + sizeof(request->ssids[0]), GFP_KERNEL); if (!request) return -ENOMEM; + /* None of the channels are actually set +* up but let UBSAN know the boundaries. +*/ + request->n_channels = n_channels; + if (wdev->conn->params.channel) { enum nl80211_band band = wdev->conn->params.channel->band; struct ieee80211_supported_band *sband = @@ -112,9 +116,9 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev) } request->rates[band] = (1 << bands->n_bitrates) - 1; } - n_channels = i; + request->n_channels = i; } - request->n_channels = n_channels; + request->ssids = (void *)&request->channels[n_channels]; request->n_ssids = 1; -- 2.45.2
Re: [PATCH 1/3] fortify: use if_changed_dep to record header dependency in *.cmd files
Hi Masahiro, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.10 next-20240716] [cannot apply to akpm-mm/mm-nonmm-unstable kees/for-next/hardening kees/for-next/pstore kees/for-next/kspp] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/fortify-use-if_changed_dep-to-record-header-dependency-in-cmd-files/20240715-224820 base: linus/master patch link: https://lore.kernel.org/r/20240715144529.101634-2-masahiroy%40kernel.org patch subject: [PATCH 1/3] fortify: use if_changed_dep to record header dependency in *.cmd files config: i386-randconfig-004-20240716 (https://download.01.org/0day-ci/archive/20240717/202407170104.dce5mksa-...@intel.com/config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240717/202407170104.dce5mksa-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202407170104.dce5mksa-...@intel.com/ All errors (new ones prefixed by >>): >> fixdep: error opening file: lib/test_fortify/.write_overflow-memcpy.log.d: >> No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.read_overflow2-memcmp.log.d: >> No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.read_overflow-memchr.log.d: No >> such file or directory -- >> fixdep: error opening file: >> lib/test_fortify/.write_overflow-strcpy-lit.log.d: No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.read_overflow2-memmove.log.d: >> No such file or directory -- >> fixdep: error opening file: >> lib/test_fortify/.write_overflow-strncpy-src.log.d: No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.read_overflow-memcmp.log.d: No >> such file or directory -- >> fixdep: error opening file: lib/test_fortify/.read_overflow-memscan.log.d: >> No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.write_overflow-strcpy.log.d: >> No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.write_overflow-memmove.log.d: >> No such file or directory -- >> fixdep: error opening file: lib/test_fortify/.write_overflow-memset.log.d: >> No such file or directory .. -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH] wifi: wireless: fix more UBSAN noise in cfg80211_conn_scan()
On Tue, Jul 16, 2024 at 08:40:11PM +0300, Dmitry Antipov wrote: > Looking at https://syzkaller.appspot.com/bug?extid=d5dc2801166df6d34774 > and trying to reproduce it with CONFIG_UBSAN enabled, I've noticed the > following: > > UBSAN: array-index-out-of-bounds in net/wireless/sme.c:95:3 > index 0 is out of range for type 'struct ieee80211_channel *[]' > CPU: 3 PID: 4993 Comm: repro Not tainted 6.10.0-01155-gd67978318827 #5 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS <...> > Call Trace: > > dump_stack_lvl+0x1c2/0x2a0 > ? __pfx_dump_stack_lvl+0x10/0x10 > ? __pfx__printk+0x10/0x10 > ? __local_bh_enable_ip+0x12e/0x1c0 > __ubsan_handle_out_of_bounds+0x127/0x150 > cfg80211_conn_scan+0xd8e/0xf30 > cfg80211_connect+0x1400/0x1c30 > nl80211_connect+0x1549/0x1a70 > ... > > This is very similar to 92ecbb3ac6f3 ("wifi: mac80211: fix UBSAN noise > in ieee80211_prep_hw_scan()"), so just fix it in the same way by setting > 'request->n_channels' early to help '__counted_by()' work as expected. > And the same 'kmalloc()' math adjustment is also applicable. > > Signed-off-by: Dmitry Antipov Nice catch! Yes, this looks correct. Reviewed-by: Kees Cook -- Kees Cook
Re: [GIT PULL] pstore updates for v6.11-rc1
On Mon, 15 Jul 2024 at 09:29, Kees Cook wrote: > > Please pull these small pstore updates for v6.11-rc1. (Note that since there > were no pstore updates for v6.10, this is based on v6.9-rc2. I forgot > to do my traditional merge-on-rc2 for this tree when v6.10-rc2 happened.) Note that this is exactly what should happen. No need to do any back-merges if some sub-area has been quiet. Sure, if some branch is *really* old, you might want to do a test-merge just to see if there are conflicts (either textual or just due to infrastructure changes elsewhere that cause old changes to not work in a modern context), but honestly, even that is just a "nice to have". But "missed one release" is not very old in the big picture. We do releases relatively often, after all. So then merge conflicts like that are on me as a top-level maintainer, and if they end up being very complex I might come back to you and ask for guidance. But honestly, that's quite rare. IOW, I'd much rather developers concentrate on their own branch and on making it very solid, rather than worry too much about what has happened elsewhere meantime. (Of course, that's all for the simple and good case where you have clearly separate trees - which pstore largely is. Some other areas get a lot more "this is affected by other changes and in turn affects other areas", and then people there have to inevitably be more involved with what has been going on). Linus
[PATCH] leds: gpio: Set num_leds after allocation
With the new __counted_by annotation, the "num_leds" variable needs to valid for accesses to the "leds" array. This requirement is not met in gpio_leds_create(), since "num_leds" starts at "0", so "leds" index "0" will not be considered valid (num_leds would need to be "1" to access index "0"). Fix this by setting the allocation size after allocation, and then update the final count based on how many were actually added to the array. Fixes: 52cd75108a42 ("leds: gpio: Annotate struct gpio_leds_priv with __counted_by") Signed-off-by: Kees Cook --- Cc: Lee Jones Cc: Pavel Machek Cc: linux-l...@vger.kernel.org --- drivers/leds/leds-gpio.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c index 83fcd7b6afff..4d1612d557c8 100644 --- a/drivers/leds/leds-gpio.c +++ b/drivers/leds/leds-gpio.c @@ -150,7 +150,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev) { struct fwnode_handle *child; struct gpio_leds_priv *priv; - int count, ret; + int count, used, ret; count = device_get_child_node_count(dev); if (!count) @@ -159,9 +159,11 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev) priv = devm_kzalloc(dev, struct_size(priv, leds, count), GFP_KERNEL); if (!priv) return ERR_PTR(-ENOMEM); + priv->num_leds = count; + used = 0; device_for_each_child_node(dev, child) { - struct gpio_led_data *led_dat = &priv->leds[priv->num_leds]; + struct gpio_led_data *led_dat = &priv->leds[used]; struct gpio_led led = {}; /* @@ -197,8 +199,9 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev) /* Set gpiod label to match the corresponding LED name. */ gpiod_set_consumer_name(led_dat->gpiod, led_dat->cdev.dev->kobj.name); - priv->num_leds++; + used++; } + priv->num_leds = used; return priv; } -- 2.34.1
[PATCH] dmaengine: stm32-dma3: Set lli_size after allocation
With the new __counted_by annotation, the "lli_size" variable needs to valid for accesses to the "lli" array. This requirement is not met in stm32_dma3_chan_desc_alloc(), since "lli_size" starts at "0", so "lli" index "0" will not be considered valid during the initialization for loop. Fix this by setting lli_size immediately after allocation (similar to how this is handled in stm32_mdma_alloc_desc() for the node/count relationship). Fixes: f561ec8b2b33 ("dmaengine: Add STM32 DMA3 support") Signed-off-by: Kees Cook --- Cc: "Amélie Delaunay" Cc: Vinod Koul Cc: Maxime Coquelin Cc: Alexandre Torgue Cc: dmaeng...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org --- drivers/dma/stm32/stm32-dma3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c index 4087e0263a48..0be6e944df6f 100644 --- a/drivers/dma/stm32/stm32-dma3.c +++ b/drivers/dma/stm32/stm32-dma3.c @@ -403,6 +403,7 @@ static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_ch swdesc = kzalloc(struct_size(swdesc, lli, count), GFP_NOWAIT); if (!swdesc) return NULL; + swdesc->lli_size = count; for (i = 0; i < count; i++) { swdesc->lli[i].hwdesc = dma_pool_zalloc(chan->lli_pool, GFP_NOWAIT, @@ -410,7 +411,6 @@ static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_ch if (!swdesc->lli[i].hwdesc) goto err_pool_free; } - swdesc->lli_size = count; swdesc->ccr = 0; /* Set LL base address */ -- 2.34.1
Re: [GIT PULL] pstore updates for v6.11-rc1
The pull request you sent on Mon, 15 Jul 2024 09:29:29 -0700: > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git > tags/pstore-v6.11-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/8050258bd1eed0f77dd7e3fa15feb23bbcc38e63 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
[PATCH] interconnect: icc-clk: Add missed num_nodes initialization
With the new __counted_by annotation, the "num_nodes" struct member must be set before accessing the "nodes" array. This initialization was done in other places where a new struct icc_onecell_data is allocated, but this case in icc_clk_register() was missed. Set "num_nodes" after allocation. Fixes: dd4904f3b924 ("interconnect: qcom: Annotate struct icc_onecell_data with __counted_by") Signed-off-by: Kees Cook --- Cc: Georgi Djakov Cc: linux...@vger.kernel.org --- drivers/interconnect/icc-clk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c index f788db15cd76..b956e4050f38 100644 --- a/drivers/interconnect/icc-clk.c +++ b/drivers/interconnect/icc-clk.c @@ -87,6 +87,7 @@ struct icc_provider *icc_clk_register(struct device *dev, onecell = devm_kzalloc(dev, struct_size(onecell, nodes, 2 * num_clocks), GFP_KERNEL); if (!onecell) return ERR_PTR(-ENOMEM); + onecell->num_nodes = 2 * num_clocks; qp = devm_kzalloc(dev, struct_size(qp, clocks, num_clocks), GFP_KERNEL); if (!qp) @@ -133,8 +134,6 @@ struct icc_provider *icc_clk_register(struct device *dev, onecell->nodes[j++] = node; } - onecell->num_nodes = j; - ret = icc_provider_register(provider); if (ret) goto err; -- 2.34.1
Re: [PATCH] leds: gpio: Set num_leds after allocation
On 16/07/24 15:24, Kees Cook wrote: With the new __counted_by annotation, the "num_leds" variable needs to valid for accesses to the "leds" array. This requirement is not met in gpio_leds_create(), since "num_leds" starts at "0", so "leds" index "0" will not be considered valid (num_leds would need to be "1" to access index "0"). Fix this by setting the allocation size after allocation, and then update the final count based on how many were actually added to the array. Fixes: 52cd75108a42 ("leds: gpio: Annotate struct gpio_leds_priv with __counted_by") Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Thanks -- Gustavo --- Cc: Lee Jones Cc: Pavel Machek Cc: linux-l...@vger.kernel.org --- drivers/leds/leds-gpio.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c index 83fcd7b6afff..4d1612d557c8 100644 --- a/drivers/leds/leds-gpio.c +++ b/drivers/leds/leds-gpio.c @@ -150,7 +150,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev) { struct fwnode_handle *child; struct gpio_leds_priv *priv; - int count, ret; + int count, used, ret; count = device_get_child_node_count(dev); if (!count) @@ -159,9 +159,11 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev) priv = devm_kzalloc(dev, struct_size(priv, leds, count), GFP_KERNEL); if (!priv) return ERR_PTR(-ENOMEM); + priv->num_leds = count; + used = 0; device_for_each_child_node(dev, child) { - struct gpio_led_data *led_dat = &priv->leds[priv->num_leds]; + struct gpio_led_data *led_dat = &priv->leds[used]; struct gpio_led led = {}; /* @@ -197,8 +199,9 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev) /* Set gpiod label to match the corresponding LED name. */ gpiod_set_consumer_name(led_dat->gpiod, led_dat->cdev.dev->kobj.name); - priv->num_leds++; + used++; } + priv->num_leds = used; return priv; }
Re: [PATCH] dmaengine: stm32-dma3: Set lli_size after allocation
On 16/07/24 15:38, Kees Cook wrote: With the new __counted_by annotation, the "lli_size" variable needs to valid for accesses to the "lli" array. This requirement is not met in stm32_dma3_chan_desc_alloc(), since "lli_size" starts at "0", so "lli" index "0" will not be considered valid during the initialization for loop. Fix this by setting lli_size immediately after allocation (similar to how this is handled in stm32_mdma_alloc_desc() for the node/count relationship). Fixes: f561ec8b2b33 ("dmaengine: Add STM32 DMA3 support") Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Thanks -- Gustavo --- Cc: "Amélie Delaunay" Cc: Vinod Koul Cc: Maxime Coquelin Cc: Alexandre Torgue Cc: dmaeng...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org --- drivers/dma/stm32/stm32-dma3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c index 4087e0263a48..0be6e944df6f 100644 --- a/drivers/dma/stm32/stm32-dma3.c +++ b/drivers/dma/stm32/stm32-dma3.c @@ -403,6 +403,7 @@ static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_ch swdesc = kzalloc(struct_size(swdesc, lli, count), GFP_NOWAIT); if (!swdesc) return NULL; + swdesc->lli_size = count; for (i = 0; i < count; i++) { swdesc->lli[i].hwdesc = dma_pool_zalloc(chan->lli_pool, GFP_NOWAIT, @@ -410,7 +411,6 @@ static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_ch if (!swdesc->lli[i].hwdesc) goto err_pool_free; } - swdesc->lli_size = count; swdesc->ccr = 0; /* Set LL base address */
Re: [PATCH] interconnect: icc-clk: Add missed num_nodes initialization
On 16/07/24 15:48, Kees Cook wrote: With the new __counted_by annotation, the "num_nodes" struct member must be set before accessing the "nodes" array. This initialization was done in other places where a new struct icc_onecell_data is allocated, but this case in icc_clk_register() was missed. Set "num_nodes" after allocation. Fixes: dd4904f3b924 ("interconnect: qcom: Annotate struct icc_onecell_data with __counted_by") Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Thanks -- Gustavo --- Cc: Georgi Djakov Cc: linux...@vger.kernel.org --- drivers/interconnect/icc-clk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c index f788db15cd76..b956e4050f38 100644 --- a/drivers/interconnect/icc-clk.c +++ b/drivers/interconnect/icc-clk.c @@ -87,6 +87,7 @@ struct icc_provider *icc_clk_register(struct device *dev, onecell = devm_kzalloc(dev, struct_size(onecell, nodes, 2 * num_clocks), GFP_KERNEL); if (!onecell) return ERR_PTR(-ENOMEM); + onecell->num_nodes = 2 * num_clocks; qp = devm_kzalloc(dev, struct_size(qp, clocks, num_clocks), GFP_KERNEL); if (!qp) @@ -133,8 +134,6 @@ struct icc_provider *icc_clk_register(struct device *dev, onecell->nodes[j++] = node; } - onecell->num_nodes = j; - ret = icc_provider_register(provider); if (ret) goto err;
[PATCH] dmaengine: ti: omap-dma: Initialize sglen after allocation
With the new __counted_by annocation, the "sglen" struct member must be set before accessing the "sg" array. This initialization was done in other places where a new struct omap_desc is allocated, but these cases were missed. Set "sglen" after allocation. Fixes: b85178611c11 ("dmaengine: ti: omap-dma: Annotate struct omap_desc with __counted_by") Signed-off-by: Kees Cook --- Cc: Peter Ujfalusi Cc: Vinod Koul Cc: dmaeng...@vger.kernel.org --- drivers/dma/ti/omap-dma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c index 7e6c04afbe89..6ab9bfbdc480 100644 --- a/drivers/dma/ti/omap-dma.c +++ b/drivers/dma/ti/omap-dma.c @@ -1186,10 +1186,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic( d->dev_addr = dev_addr; d->fi = burst; d->es = es; + d->sglen = 1; d->sg[0].addr = buf_addr; d->sg[0].en = period_len / es_bytes[es]; d->sg[0].fn = buf_len / period_len; - d->sglen = 1; d->ccr = c->ccr; if (dir == DMA_DEV_TO_MEM) @@ -1258,10 +1258,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_memcpy( d->dev_addr = src; d->fi = 0; d->es = data_type; + d->sglen = 1; d->sg[0].en = len / BIT(data_type); d->sg[0].fn = 1; d->sg[0].addr = dest; - d->sglen = 1; d->ccr = c->ccr; d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_POSTINC; @@ -1309,6 +1309,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved( if (data_type > CSDP_DATA_TYPE_32) data_type = CSDP_DATA_TYPE_32; + d->sglen = 1; sg = &d->sg[0]; d->dir = DMA_MEM_TO_MEM; d->dev_addr = xt->src_start; @@ -1316,7 +1317,6 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved( sg->en = xt->sgl[0].size / BIT(data_type); sg->fn = xt->numf; sg->addr = xt->dst_start; - d->sglen = 1; d->ccr = c->ccr; src_icg = dmaengine_get_src_icg(xt, &xt->sgl[0]); -- 2.34.1
Re: [PATCH v4 00/11] Add per-core RAPL energy counter support for AMD CPUs
On Tue, Jul 16, 2024 at 1:42 AM Dhananjay Ugwekar wrote: > > Hello Ian, > > On 7/15/2024 8:52 PM, Ian Rogers wrote: > > On Mon, Jul 15, 2024 at 2:36 AM Dhananjay Ugwekar > > wrote: > >> > >> Hello Ian, > >> > >> On 7/12/2024 3:53 AM, Ian Rogers wrote: > >>> On Thu, Jul 11, 2024 at 3:25 AM Dhananjay Ugwekar > >>> wrote: > > Currently the energy-cores event in the power PMU aggregates energy > consumption data at a package level. On the other hand the core energy > RAPL counter in AMD CPUs has a core scope (which means the energy > consumption is recorded separately for each core). Earlier efforts to add > the core event in the power PMU had failed [1], due to the difference in > the scope of these two events. Hence, there is a need for a new core > scope > PMU. > > This patchset adds a new "power_per_core" PMU alongside the existing > "power" PMU, which will be responsible for collecting the new > "energy-per-core" event. > >>> > >>> Sorry for being naive, is the only reason for adding the new PMU for > >>> the sake of the cpumask? Perhaps we can add per event cpumasks like > >>> say `/sys/devices/power/events/energy-per-core.cpumask` which contains > >>> the CPUs of the different cores in this case. There's supporting > >>> hotplugging CPUs as an issue. Adding the tool support for this > >>> wouldn't be hard and it may be less messy (although old perf tools on > >>> new kernels wouldn't know about these files). > >> > >> I went over the two approaches and below are my thoughts, > >> > >> New PMU approach: > >> Pros > >> * It will work with older perf tools, hence these patches can be > >> backported to an older kernel and the new per-core event will work there > >> as well. > >> Cons > >> * More code changes in rapl.c > >> > >> Event specific cpumask approach: > >> Pros > >> * It might be easier to add diff scope events within the same PMU in > >> future(although currently I'm not able to find such a usecase, apart from > >> the RAPL pkg and core energy counters) > >> Cons > >> * Both new kernel and perf tool will be required to use the new per-core > >> event. > >> > >> I feel that while the event-specific cpumask is a viable alternative to > >> the new PMU addition approach, I dont see any clear pros to select that > >> over the current approach. Please let me know if you have any design > >> related concerns to the addition of new PMU or your concern is mostly > >> about the amount of code changes in this approach. > > > > Thanks Dhananjay, and thanks for taking the time for an objective > > discussion on the mailing list. I'm very supportive of seeing the work > > you are enabling land. > > > > My concern comes from the tool side. If every PMU starts to have > > variants for the sake of the cpumask what does this mean for > > aggregation in the perf tool? There is another issue around event > > grouping, you can't group events across PMUs, but my feeling is that > > event grouping needs to be rethought. By default the power_per_core > > events are going to be aggregated together by the perf tool, which > > then loses their per_core-ness. > > Yea right, maybe we need to fix this behavior. > > > > > I was trying to think of the same problem but in other PMUs. One > > thought I had was the difference between hyperthread and core events. > > At least on Intel, some events can only count for the whole core not > > per hyperthread. The events don't have a cpu_per_core PMU, they just > > use the regular cpu one, and so the cpumask is set to all online > > hyperthreads. When a per-core event is programmed it will get > > programmed on every hyperthread and so counted twice for the core. > > This at the least wastes a counter, but it probably also yields twice > > the expected count as every event is counted twice then aggregated. So > > this is just wrong and the user is expected to be smart and fix it > > (checking the x86 events there is a convention to use a ".ALL" or > > ".ANY" suffix for core wide events iirc). If we had a cpumask for > > these events then we could avoid the double setting, free up a counter > > and avoid double counting. Were we to fix things the way it is done in > > this patch series we'd add another PMU. > > Yes, this seems like a valid usecase for event-specific cpumasks. > > > > > My feeling is that in the longer term a per event cpumask looks > > cleaner. I think either way you need a new kernel for the new RAPL > > events. The problem with an old perf tool and a new kernel, this > > doesn't normally happen with distributions as they match the perf tool > > to the kernel version needlessly losing features and fixes along the > > way. If the new PMU is going to get backported through fixes.. then we > > can do similar for reading the per event cpumask. I'd be tempted not > > to do this and focus on the next LTS kernel, getting the kernel and > > tool fixes in as necessary. > > Makes sense, even though this
Re: [PATCH] dmaengine: ti: omap-dma: Initialize sglen after allocation
On 16/07/24 15:57, Kees Cook wrote: With the new __counted_by annocation, the "sglen" struct member must be set before accessing the "sg" array. This initialization was done in other places where a new struct omap_desc is allocated, but these cases were missed. Set "sglen" after allocation. Fixes: b85178611c11 ("dmaengine: ti: omap-dma: Annotate struct omap_desc with __counted_by") Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Thanks -- Gustavo --- Cc: Peter Ujfalusi Cc: Vinod Koul Cc: dmaeng...@vger.kernel.org --- drivers/dma/ti/omap-dma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c index 7e6c04afbe89..6ab9bfbdc480 100644 --- a/drivers/dma/ti/omap-dma.c +++ b/drivers/dma/ti/omap-dma.c @@ -1186,10 +1186,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic( d->dev_addr = dev_addr; d->fi = burst; d->es = es; + d->sglen = 1; d->sg[0].addr = buf_addr; d->sg[0].en = period_len / es_bytes[es]; d->sg[0].fn = buf_len / period_len; - d->sglen = 1; d->ccr = c->ccr; if (dir == DMA_DEV_TO_MEM) @@ -1258,10 +1258,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_memcpy( d->dev_addr = src; d->fi = 0; d->es = data_type; + d->sglen = 1; d->sg[0].en = len / BIT(data_type); d->sg[0].fn = 1; d->sg[0].addr = dest; - d->sglen = 1; d->ccr = c->ccr; d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_POSTINC; @@ -1309,6 +1309,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved( if (data_type > CSDP_DATA_TYPE_32) data_type = CSDP_DATA_TYPE_32; + d->sglen = 1; sg = &d->sg[0]; d->dir = DMA_MEM_TO_MEM; d->dev_addr = xt->src_start; @@ -1316,7 +1317,6 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved( sg->en = xt->sgl[0].size / BIT(data_type); sg->fn = xt->numf; sg->addr = xt->dst_start; - d->sglen = 1; d->ccr = c->ccr; src_icg = dmaengine_get_src_icg(xt, &xt->sgl[0]);
Re: [PATCH 1/3] fortify: use if_changed_dep to record header dependency in *.cmd files
On Wed, Jul 17, 2024 at 2:51 AM kernel test robot wrote: > > Hi Masahiro, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on linus/master] > [also build test ERROR on v6.10 next-20240716] > [cannot apply to akpm-mm/mm-nonmm-unstable kees/for-next/hardening > kees/for-next/pstore kees/for-next/kspp] > [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#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/fortify-use-if_changed_dep-to-record-header-dependency-in-cmd-files/20240715-224820 > base: linus/master > patch link: > https://lore.kernel.org/r/20240715144529.101634-2-masahiroy%40kernel.org > patch subject: [PATCH 1/3] fortify: use if_changed_dep to record header > dependency in *.cmd files > config: i386-randconfig-004-20240716 > (https://download.01.org/0day-ci/archive/20240717/202407170104.dce5mksa-...@intel.com/config) > compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 > reproduce (this is a W=1 build): > (https://download.01.org/0day-ci/archive/20240717/202407170104.dce5mksa-...@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version > of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot > | Closes: > https://lore.kernel.org/oe-kbuild-all/202407170104.dce5mksa-...@intel.com/ > > All errors (new ones prefixed by >>): > > >> fixdep: error opening file: lib/test_fortify/.write_overflow-memcpy.log.d: > >> No such file or directory > -- > >> fixdep: error opening file: lib/test_fortify/.read_overflow2-memcmp.log.d: > >> No such file or directory > -- > >> fixdep: error opening file: lib/test_fortify/.read_overflow-memchr.log.d: > >> No such file or directory > -- > >> fixdep: error opening file: > >> lib/test_fortify/.write_overflow-strcpy-lit.log.d: No such file or > >> directory > -- > >> fixdep: error opening file: > >> lib/test_fortify/.read_overflow2-memmove.log.d: No such file or directory > -- > >> fixdep: error opening file: > >> lib/test_fortify/.write_overflow-strncpy-src.log.d: No such file or > >> directory > -- > >> fixdep: error opening file: lib/test_fortify/.read_overflow-memcmp.log.d: > >> No such file or directory > -- > >> fixdep: error opening file: lib/test_fortify/.read_overflow-memscan.log.d: > >> No such file or directory > -- > >> fixdep: error opening file: lib/test_fortify/.write_overflow-strcpy.log.d: > >> No such file or directory > -- > >> fixdep: error opening file: > >> lib/test_fortify/.write_overflow-memmove.log.d: No such file or directory > -- > >> fixdep: error opening file: lib/test_fortify/.write_overflow-memset.log.d: > >> No such file or directory > .. This issue seems to occur with GCC <=7 $ echo 'void b(void) __attribute__((__error__(""))); void a(void) { b(); }' | gcc -Wp,-MMD,test.d -c -o /dev/null -x c - did not create *.d with GCC <= 7. I do not see the issue with GCC >= 8 or Clang. One quick solution is to skip the test for GCC <= 7. > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki > -- Best Regards Masahiro Yamada