July 2024 - Linux-stable-mirror

[v6.9-stable PATCH] mm: page_ref: remove folio_try_get_rcu()

by Yang Shi

commit fa2690af573dfefb47ba6eef888797a64b6b5f3c upstream The below bug was reported on a non-SMP kernel: [ 275.267158][ T4335] ------------[ cut here ]------------ [ 275.267949][ T4335] kernel BUG at include/linux/page_ref.h:275! [ 275.268526][ T4335] invalid opcode: 0000 [#1] KASAN PTI [ 275.269001][ T4335] CPU: 0 PID: 4335 Comm: trinity-c3 Not tainted 6.7.0-rc4-00061-gefa7df3e3bb5 #1 [ 275.269787][ T4335] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 [ 275.270679][ T4335] RIP: 0010:try_get_folio (include/linux/page_ref.h:275 (discriminator 3) mm/gup.c:79 (discriminator 3)) [ 275.272813][ T4335] RSP: 0018:ffffc90005dcf650 EFLAGS: 00010202 [ 275.273346][ T4335] RAX: 0000000000000246 RBX: ffffea00066e0000 RCX: 0000000000000000 [ 275.274032][ T4335] RDX: fffff94000cdc007 RSI: 0000000000000004 RDI: ffffea00066e0034 [ 275.274719][ T4335] RBP: ffffea00066e0000 R08: 0000000000000000 R09: fffff94000cdc006 [ 275.275404][ T4335] R10: ffffea00066e0037 R11: 0000000000000000 R12: 0000000000000136 [ 275.276106][ T4335] R13: ffffea00066e0034 R14: dffffc0000000000 R15: ffffea00066e0008 [ 275.276790][ T4335] FS: 00007fa2f9b61740(0000) GS:ffffffff89d0d000(0000) knlGS:0000000000000000 [ 275.277570][ T4335] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 275.278143][ T4335] CR2: 00007fa2f6c00000 CR3: 0000000134b04000 CR4: 00000000000406f0 [ 275.278833][ T4335] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 275.279521][ T4335] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 275.280201][ T4335] Call Trace: [ 275.280499][ T4335] <TASK> [ 275.280751][ T4335] ? die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434 arch/x86/kernel/dumpstack.c:447) [ 275.281087][ T4335] ? do_trap (arch/x86/kernel/traps.c:112 arch/x86/kernel/traps.c:153) [ 275.281463][ T4335] ? try_get_folio (include/linux/page_ref.h:275 (discriminator 3) mm/gup.c:79 (discriminator 3)) [ 275.281884][ T4335] ? try_get_folio (include/linux/page_ref.h:275 (discriminator 3) mm/gup.c:79 (discriminator 3)) [ 275.282300][ T4335] ? do_error_trap (arch/x86/kernel/traps.c:174) [ 275.282711][ T4335] ? try_get_folio (include/linux/page_ref.h:275 (discriminator 3) mm/gup.c:79 (discriminator 3)) [ 275.283129][ T4335] ? handle_invalid_op (arch/x86/kernel/traps.c:212) [ 275.283561][ T4335] ? try_get_folio (include/linux/page_ref.h:275 (discriminator 3) mm/gup.c:79 (discriminator 3)) [ 275.283990][ T4335] ? exc_invalid_op (arch/x86/kernel/traps.c:264) [ 275.284415][ T4335] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:568) [ 275.284859][ T4335] ? try_get_folio (include/linux/page_ref.h:275 (discriminator 3) mm/gup.c:79 (discriminator 3)) [ 275.285278][ T4335] try_grab_folio (mm/gup.c:148) [ 275.285684][ T4335] __get_user_pages (mm/gup.c:1297 (discriminator 1)) [ 275.286111][ T4335] ? __pfx___get_user_pages (mm/gup.c:1188) [ 275.286579][ T4335] ? __pfx_validate_chain (kernel/locking/lockdep.c:3825) [ 275.287034][ T4335] ? mark_lock (kernel/locking/lockdep.c:4656 (discriminator 1)) [ 275.287416][ T4335] __gup_longterm_locked (mm/gup.c:1509 mm/gup.c:2209) [ 275.288192][ T4335] ? __pfx___gup_longterm_locked (mm/gup.c:2204) [ 275.288697][ T4335] ? __pfx_lock_acquire (kernel/locking/lockdep.c:5722) [ 275.289135][ T4335] ? __pfx___might_resched (kernel/sched/core.c:10106) [ 275.289595][ T4335] pin_user_pages_remote (mm/gup.c:3350) [ 275.290041][ T4335] ? __pfx_pin_user_pages_remote (mm/gup.c:3350) [ 275.290545][ T4335] ? find_held_lock (kernel/locking/lockdep.c:5244 (discriminator 1)) [ 275.290961][ T4335] ? mm_access (kernel/fork.c:1573) [ 275.291353][ T4335] process_vm_rw_single_vec+0x142/0x360 [ 275.291900][ T4335] ? __pfx_process_vm_rw_single_vec+0x10/0x10 [ 275.292471][ T4335] ? mm_access (kernel/fork.c:1573) [ 275.292859][ T4335] process_vm_rw_core+0x272/0x4e0 [ 275.293384][ T4335] ? hlock_class (arch/x86/include/asm/bitops.h:227 arch/x86/include/asm/bitops.h:239 include/asm-generic/bitops/instrumented-non-atomic.h:142 kernel/locking/lockdep.c:228) [ 275.293780][ T4335] ? __pfx_process_vm_rw_core+0x10/0x10 [ 275.294350][ T4335] process_vm_rw (mm/process_vm_access.c:284) [ 275.294748][ T4335] ? __pfx_process_vm_rw (mm/process_vm_access.c:259) [ 275.295197][ T4335] ? __task_pid_nr_ns (include/linux/rcupdate.h:306 (discriminator 1) include/linux/rcupdate.h:780 (discriminator 1) kernel/pid.c:504 (discriminator 1)) [ 275.295634][ T4335] __x64_sys_process_vm_readv (mm/process_vm_access.c:291) [ 275.296139][ T4335] ? syscall_enter_from_user_mode (kernel/entry/common.c:94 kernel/entry/common.c:112) [ 275.296642][ T4335] do_syscall_64 (arch/x86/entry/common.c:51 (discriminator 1) arch/x86/entry/common.c:82 (discriminator 1)) [ 275.297032][ T4335] ? __task_pid_nr_ns (include/linux/rcupdate.h:306 (discriminator 1) include/linux/rcupdate.h:780 (discriminator 1) kernel/pid.c:504 (discriminator 1)) [ 275.297470][ T4335] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4300 kernel/locking/lockdep.c:4359) [ 275.297988][ T4335] ? do_syscall_64 (arch/x86/include/asm/cpufeature.h:171 arch/x86/entry/common.c:97) [ 275.298389][ T4335] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4300 kernel/locking/lockdep.c:4359) [ 275.298906][ T4335] ? do_syscall_64 (arch/x86/include/asm/cpufeature.h:171 arch/x86/entry/common.c:97) [ 275.299304][ T4335] ? do_syscall_64 (arch/x86/include/asm/cpufeature.h:171 arch/x86/entry/common.c:97) [ 275.299703][ T4335] ? do_syscall_64 (arch/x86/include/asm/cpufeature.h:171 arch/x86/entry/common.c:97) [ 275.300115][ T4335] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:129) This BUG is the VM_BUG_ON(!in_atomic() && !irqs_disabled()) assertion in folio_ref_try_add_rcu() for non-SMP kernel. The process_vm_readv() calls GUP to pin the THP. An optimization for pinning THP instroduced by commit 57edfcfd3419 ("mm/gup: accelerate thp gup even for "pages != NULL"") calls try_grab_folio() to pin the THP, but try_grab_folio() is supposed to be called in atomic context for non-SMP kernel, for example, irq disabled or preemption disabled, due to the optimization introduced by commit e286781d5f2e ("mm: speculative page references"). The commit efa7df3e3bb5 ("mm: align larger anonymous mappings on THP boundaries") is not actually the root cause although it was bisected to. It just makes the problem exposed more likely. The follow up discussion suggested the optimization for non-SMP kernel may be out-dated and not worth it anymore [1]. So removing the optimization to silence the BUG. However calling try_grab_folio() in GUP slow path actually is unnecessary, so the following patch will clean this up. [1] https://lore.kernel.org/linux-mm/821cf1d6-92b9-4ac4-bacc-d8f2364ac14f@paulm… Link: https://lkml.kernel.org/r/20240625205350.1777481-1-yang@os.amperecomputing.… Fixes: 57edfcfd3419 ("mm/gup: accelerate thp gup even for "pages != NULL"") Signed-off-by: Yang Shi <yang(a)os.amperecomputing.com> Reported-by: kernel test robot <oliver.sang(a)intel.com> Tested-by: Oliver Sang <oliver.sang(a)intel.com> Acked-by: Peter Xu <peterx(a)redhat.com> Acked-by: David Hildenbrand <david(a)redhat.com> Cc: Christoph Lameter <cl(a)linux.com> Cc: Matthew Wilcox (Oracle) <willy(a)infradead.org> Cc: Paul E. McKenney <paulmck(a)kernel.org> Cc: Rik van Riel <riel(a)surriel.com> Cc: Vivek Kasireddy <vivek.kasireddy(a)intel.com> Cc: <stable(a)vger.kernel.org> [6.6+] Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> --- fs/netfs/buffered_write.c | 4 ++-- fs/smb/client/file.c | 4 ++-- include/linux/page_ref.h | 49 ++------------------------------------- mm/filemap.c | 10 ++++---- mm/gup.c | 2 +- 5 files changed, 12 insertions(+), 57 deletions(-) diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c index d2ce0849bb53..e6066ddbb3ac 100644 --- a/fs/netfs/buffered_write.c +++ b/fs/netfs/buffered_write.c @@ -811,7 +811,7 @@ static void netfs_extend_writeback(struct address_space *mapping, break; } - if (!folio_try_get_rcu(folio)) { + if (!folio_try_get(folio)) { xas_reset(xas); continue; } @@ -1028,7 +1028,7 @@ static ssize_t netfs_writepages_begin(struct address_space *mapping, if (!folio) break; - if (!folio_try_get_rcu(folio)) { + if (!folio_try_get(folio)) { xas_reset(xas); continue; } diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 9be37d0fe724..4784fece4d99 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -2753,7 +2753,7 @@ static void cifs_extend_writeback(struct address_space *mapping, break; } - if (!folio_try_get_rcu(folio)) { + if (!folio_try_get(folio)) { xas_reset(xas); continue; } @@ -2989,7 +2989,7 @@ static ssize_t cifs_writepages_begin(struct address_space *mapping, if (!folio) break; - if (!folio_try_get_rcu(folio)) { + if (!folio_try_get(folio)) { xas_reset(xas); continue; } diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h index d7c2d33baa7f..fdd2a75adb03 100644 --- a/include/linux/page_ref.h +++ b/include/linux/page_ref.h @@ -263,54 +263,9 @@ static inline bool folio_try_get(struct folio *folio) return folio_ref_add_unless(folio, 1, 0); } -static inline bool folio_ref_try_add_rcu(struct folio *folio, int count) -{ -#ifdef CONFIG_TINY_RCU - /* - * The caller guarantees the folio will not be freed from interrupt - * context, so (on !SMP) we only need preemption to be disabled - * and TINY_RCU does that for us. - */ -# ifdef CONFIG_PREEMPT_COUNT - VM_BUG_ON(!in_atomic() && !irqs_disabled()); -# endif - VM_BUG_ON_FOLIO(folio_ref_count(folio) == 0, folio); - folio_ref_add(folio, count); -#else - if (unlikely(!folio_ref_add_unless(folio, count, 0))) { - /* Either the folio has been freed, or will be freed. */ - return false; - } -#endif - return true; -} - -/** - * folio_try_get_rcu - Attempt to increase the refcount on a folio. - * @folio: The folio. - * - * This is a version of folio_try_get() optimised for non-SMP kernels. - * If you are still holding the rcu_read_lock() after looking up the - * page and know that the page cannot have its refcount decreased to - * zero in interrupt context, you can use this instead of folio_try_get(). - * - * Example users include get_user_pages_fast() (as pages are not unmapped - * from interrupt context) and the page cache lookups (as pages are not - * truncated from interrupt context). We also know that pages are not - * frozen in interrupt context for the purposes of splitting or migration. - * - * You can also use this function if you're holding a lock that prevents - * pages being frozen & removed; eg the i_pages lock for the page cache - * or the mmap_lock or page table lock for page tables. In this case, - * it will always succeed, and you could have used a plain folio_get(), - * but it's sometimes more convenient to have a common function called - * from both locked and RCU-protected contexts. - * - * Return: True if the reference count was successfully incremented. - */ -static inline bool folio_try_get_rcu(struct folio *folio) +static inline bool folio_ref_try_add(struct folio *folio, int count) { - return folio_ref_try_add_rcu(folio, 1); + return folio_ref_add_unless(folio, count, 0); } static inline int page_ref_freeze(struct page *page, int count) diff --git a/mm/filemap.c b/mm/filemap.c index 30de18c4fd28..367ea201468d 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1823,7 +1823,7 @@ void *filemap_get_entry(struct address_space *mapping, pgoff_t index) if (!folio || xa_is_value(folio)) goto out; - if (!folio_try_get_rcu(folio)) + if (!folio_try_get(folio)) goto repeat; if (unlikely(folio != xas_reload(&xas))) { @@ -1977,7 +1977,7 @@ static inline struct folio *find_get_entry(struct xa_state *xas, pgoff_t max, if (!folio || xa_is_value(folio)) return folio; - if (!folio_try_get_rcu(folio)) + if (!folio_try_get(folio)) goto reset; if (unlikely(folio != xas_reload(xas))) { @@ -2157,7 +2157,7 @@ unsigned filemap_get_folios_contig(struct address_space *mapping, if (xa_is_value(folio)) goto update_start; - if (!folio_try_get_rcu(folio)) + if (!folio_try_get(folio)) goto retry; if (unlikely(folio != xas_reload(&xas))) @@ -2289,7 +2289,7 @@ static void filemap_get_read_batch(struct address_space *mapping, break; if (xa_is_sibling(folio)) break; - if (!folio_try_get_rcu(folio)) + if (!folio_try_get(folio)) goto retry; if (unlikely(folio != xas_reload(&xas))) @@ -3448,7 +3448,7 @@ static struct folio *next_uptodate_folio(struct xa_state *xas, continue; if (folio_test_locked(folio)) continue; - if (!folio_try_get_rcu(folio)) + if (!folio_try_get(folio)) continue; /* Has the page moved or been split? */ if (unlikely(folio != xas_reload(xas))) diff --git a/mm/gup.c b/mm/gup.c index 1611e73b1121..ec8570d25a6c 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -76,7 +76,7 @@ static inline struct folio *try_get_folio(struct page *page, int refs) folio = page_folio(page); if (WARN_ON_ONCE(folio_ref_count(folio) < 0)) return NULL; - if (unlikely(!folio_ref_try_add_rcu(folio, refs))) + if (unlikely(!folio_ref_try_add(folio, refs))) return NULL; /* -- 2.41.0

9 months

2
1
0 0

FAILED: patch "[PATCH] ACPI: processor_idle: Fix invalid comparison with insertion" failed to apply to 5.15-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y git checkout FETCH_HEAD git cherry-pick -x 233323f9b9f828cd7cd5145ad811c1990b692542 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024071528-foothill-overdraft-d69a@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^.. Possible dependencies: 233323f9b9f8 ("ACPI: processor_idle: Fix invalid comparison with insertion sort for latency") 0e6078c3c673 ("ACPI: processor idle: Use swap() instead of open coding it") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 233323f9b9f828cd7cd5145ad811c1990b692542 Mon Sep 17 00:00:00 2001 From: Kuan-Wei Chiu <visitorckw(a)gmail.com> Date: Tue, 2 Jul 2024 04:56:39 +0800 Subject: [PATCH] ACPI: processor_idle: Fix invalid comparison with insertion sort for latency The acpi_cst_latency_cmp() comparison function currently used for sorting C-state latencies does not satisfy transitivity, causing incorrect sorting results. Specifically, if there are two valid acpi_processor_cx elements A and B and one invalid element C, it may occur that A < B, A = C, and B = C. Sorting algorithms assume that if A < B and A = C, then C < B, leading to incorrect ordering. Given the small size of the array (<=8), we replace the library sort function with a simple insertion sort that properly ignores invalid elements and sorts valid ones based on latency. This change ensures correct ordering of the C-state latencies. Fixes: 65ea8f2c6e23 ("ACPI: processor idle: Fix up C-state latency if not ordered") Reported-by: Julian Sikorski <belegdol(a)gmail.com> Closes: https://lore.kernel.org/lkml/70674dc7-5586-4183-8953-8095567e73df@gmail.com Signed-off-by: Kuan-Wei Chiu <visitorckw(a)gmail.com> Tested-by: Julian Sikorski <belegdol(a)gmail.com> Cc: All applicable <stable(a)vger.kernel.org> Link: https://patch.msgid.link/20240701205639.117194-1-visitorckw@gmail.com Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index bd6a7857ce05..831fa4a12159 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -16,7 +16,6 @@ #include <linux/acpi.h> #include <linux/dmi.h> #include <linux/sched.h> /* need_resched() */ -#include <linux/sort.h> #include <linux/tick.h> #include <linux/cpuidle.h> #include <linux/cpu.h> @@ -386,25 +385,24 @@ static void acpi_processor_power_verify_c3(struct acpi_processor *pr, acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, 1); } -static int acpi_cst_latency_cmp(const void *a, const void *b) +static void acpi_cst_latency_sort(struct acpi_processor_cx *states, size_t length) { - const struct acpi_processor_cx *x = a, *y = b; + int i, j, k; - if (!(x->valid && y->valid)) - return 0; - if (x->latency > y->latency) - return 1; - if (x->latency < y->latency) - return -1; - return 0; -} -static void acpi_cst_latency_swap(void *a, void *b, int n) -{ - struct acpi_processor_cx *x = a, *y = b; + for (i = 1; i < length; i++) { + if (!states[i].valid) + continue; - if (!(x->valid && y->valid)) - return; - swap(x->latency, y->latency); + for (j = i - 1, k = i; j >= 0; j--) { + if (!states[j].valid) + continue; + + if (states[j].latency > states[k].latency) + swap(states[j].latency, states[k].latency); + + k = j; + } + } } static int acpi_processor_power_verify(struct acpi_processor *pr) @@ -449,10 +447,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) if (buggy_latency) { pr_notice("FW issue: working around C-state latencies out of order\n"); - sort(&pr->power.states[1], max_cstate, - sizeof(struct acpi_processor_cx), - acpi_cst_latency_cmp, - acpi_cst_latency_swap); + acpi_cst_latency_sort(&pr->power.states[1], max_cstate); } lapic_timer_propagate_broadcast(pr);

9 months

3
2
0 0

Missing writer side af_unix annotations in 4.19.317

by syphyr

Several commits were made in v4.19.317 for af_unix to fix race conditions. Most of the READ_ONCE commits were added to v4.19.317, but the WRITE_ONCE on the writer side did not get added. For example, https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=…

9 months

2
1
0 0

Re: [PATCH] ARM: fix get_user() broken with veneer

by John Stultz

On Tue, Sep 26, 2023 at 9:09 AM Masahiro Yamada <masahiroy(a)kernel.org> wrote: > > The 32-bit ARM kernel stops working if the kernel grows to the point > where veneers for __get_user_* are created. > > AAPCS32 [1] states, "Register r12 (IP) may be used by a linker as a > scratch register between a routine and any subroutine it calls. It > can also be used within a routine to hold intermediate values between > subroutine calls." > > However, bl instructions buried within the inline asm are unpredictable > for compilers; hence, "ip" must be added to the clobber list. > > This becomes critical when veneers for __get_user_* are created because > veneers use the ip register since commit 02e541db0540 ("ARM: 8323/1: > force linker to use PIC veneers"). > > [1]: https://github.com/ARM-software/abi-aa/blob/2023Q1/aapcs32/aapcs32.rst > > Signed-off-by: Masahiro Yamada <masahiroy(a)kernel.org> > Reviewed-by: Ard Biesheuvel <ardb(a)kernel.org> + stable(a)vger.kernel.org It seems like this (commit 24d3ba0a7b44c1617c27f5045eecc4f34752ab03 upstream) would be a good candidate for -stable? The issue it fixes can manifest in lots of very strange ways, so it would be good to avoid others getting tripped up by it on -stable branches. (Apologies for being a bit verbose in the following, I've included a lot of details and breadcrumbs so others might find this if they run into the same issues.) I was recently looking into an arm32 issue, and found getting a custom built kernel consistently working in qemu-system-arm to bisect issues in the range of 5.15-6.6 was a bit difficult, as I would hit a couple different odd errors. For 5.15 I was seeing systemd fail to start in a fairly opaque way: starting systemd-udevd.service - Rule-based Manager for Device Events and Files. systemd-udevd.service: Main process exited, code=exited, status=1/FAILURE systemd-udevd.service: Failed with result 'exit-code'. Failed to start systemd-udevd.service - Rule-based Manager for Device Events and Files. But further looking through the logs I found: systemd[1]: Failed to open netlink: Operation not permitted Despite lots of digging to try to understand what was going wrong, the one thing that worked was switching to CONFIG_CC_OPTIMIZE_FOR_SIZE (which I only tried as I came across this old thread: https://lists.yoctoproject.org/g/linux-yocto/message/8035 ), this seemed very suspicious, but I didn't have a lot of time to dig further. That resolved things until ~6.1, where I started seeing crashes at init: [ 16.982562] Run /init as init process [ 16.989311] Failed to execute /init (error -22) [ 16.990017] Run /sbin/init as init process [ 16.994737] Starting init: /sbin/init exists but couldn't execute it (error -22) That I bisected that failure down to being supposedly caused by commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?… And searching around that commit luckily led me to this change, which finally seems to resolve the different issues I saw for 6.6, 6.1 and 5.15! Now, In my rush to get something booting with qemu, I started with the debian config but disabled modules, and didn't put much time into getting rid of config options or drivers I wouldn't need. So the kernel is pretty large. So maybe not super common, but I definitely wouldn't want others to have to go down this debugging rabbit hole. thanks -john

9 months

3
2
0 0

[PATCH 2/2] serial: sc16is7xx: fix invalid FIFO access with special register set

by Hugo Villeneuve

From: Hugo Villeneuve <hvilleneuve(a)dimonoff.com> When enabling access to the special register set, Receiver time-out and RHR interrupts can happen. In this case, the IRQ handler will try to read from the FIFO thru the RHR register at address 0x00, but address 0x00 is mapped to DLL register, resulting in erroneous FIFO reading. Call graph example: sc16is7xx_startup(): entry sc16is7xx_ms_proc(): entry sc16is7xx_set_termios(): entry sc16is7xx_set_baud(): DLH/DLL = $009C --> access special register set sc16is7xx_port_irq() entry --> IIR is 0x0C sc16is7xx_handle_rx() entry sc16is7xx_fifo_read(): --> unable to access FIFO (RHR) because it is mapped to DLL (LCR=LCR_CONF_MODE_A) sc16is7xx_set_baud(): exit --> Restore access to general register set Fix the problem by claiming the efr_lock mutex when accessing the Special register set. Fixes: dfeae619d781 ("serial: sc16is7xx") Cc: <stable(a)vger.kernel.org> Signed-off-by: Hugo Villeneuve <hvilleneuve(a)dimonoff.com> --- drivers/tty/serial/sc16is7xx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 58696e05492c..b4c1798a1df2 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -592,6 +592,8 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) SC16IS7XX_MCR_CLKSEL_BIT, prescaler == 1 ? 0 : SC16IS7XX_MCR_CLKSEL_BIT); + mutex_lock(&one->efr_lock); + /* Backup LCR and access special register set (DLL/DLH) */ lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG); sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, @@ -606,6 +608,8 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) /* Restore LCR and access to general register set */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr); + mutex_unlock(&one->efr_lock); + return DIV_ROUND_CLOSEST((clk / prescaler) / 16, div); } -- 2.39.2

9 months

1
0
0 0

[PATCH 1/2] serial: sc16is7xx: fix TX fifo corruption

by Hugo Villeneuve

From: Hugo Villeneuve <hvilleneuve(a)dimonoff.com> Sometimes, when a packet is received on channel A at almost the same time as a packet is about to be transmitted on channel B, we observe with a logic analyzer that the received packet on channel A is transmitted on channel B. In other words, the Tx buffer data on channel B is corrupted with data from channel A. The problem appeared since commit 4409df5866b7 ("serial: sc16is7xx: change EFR lock to operate on each channels"), which changed the EFR locking to operate on each channel instead of chip-wise. This commit has introduced a regression, because the EFR lock is used not only to protect the EFR registers access, but also, in a very obscure and undocumented way, to protect access to the data buffer, which is shared by the Tx and Rx handlers, but also by each channel of the IC. Fix this regression first by switching to kfifo_out_linear_ptr() in sc16is7xx_handle_tx() to eliminate the need for a shared Rx/Tx buffer. Secondly, replace the chip-wise Rx buffer with a separate Rx buffer for each channel. Fixes: 4409df5866b7 ("serial: sc16is7xx: change EFR lock to operate on each channels") Cc: <stable(a)vger.kernel.org> Signed-off-by: Hugo Villeneuve <hvilleneuve(a)dimonoff.com> --- drivers/tty/serial/sc16is7xx.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index c79dcd7c8d1a..58696e05492c 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -327,6 +327,7 @@ struct sc16is7xx_one { struct kthread_work reg_work; struct kthread_delayed_work ms_work; struct sc16is7xx_one_config config; + unsigned char buf[SC16IS7XX_FIFO_SIZE]; /* Rx buffer. */ unsigned int old_mctrl; u8 old_lcr; /* Value before EFR access. */ bool irda_mode; @@ -340,7 +341,6 @@ struct sc16is7xx_port { unsigned long gpio_valid_mask; #endif u8 mctrl_mask; - unsigned char buf[SC16IS7XX_FIFO_SIZE]; struct kthread_worker kworker; struct task_struct *kworker_task; struct sc16is7xx_one p[]; @@ -612,18 +612,18 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen, unsigned int iir) { - struct sc16is7xx_port *s = dev_get_drvdata(port->dev); + struct sc16is7xx_one *one = to_sc16is7xx_one(port, port); unsigned int lsr = 0, bytes_read, i; bool read_lsr = (iir == SC16IS7XX_IIR_RLSE_SRC) ? true : false; u8 ch, flag; - if (unlikely(rxlen >= sizeof(s->buf))) { + if (unlikely(rxlen >= sizeof(one->buf))) { dev_warn_ratelimited(port->dev, "ttySC%i: Possible RX FIFO overrun: %d\n", port->line, rxlen); port->icount.buf_overrun++; /* Ensure sanity of RX level */ - rxlen = sizeof(s->buf); + rxlen = sizeof(one->buf); } while (rxlen) { @@ -636,10 +636,10 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen, lsr = 0; if (read_lsr) { - s->buf[0] = sc16is7xx_port_read(port, SC16IS7XX_RHR_REG); + one->buf[0] = sc16is7xx_port_read(port, SC16IS7XX_RHR_REG); bytes_read = 1; } else { - sc16is7xx_fifo_read(port, s->buf, rxlen); + sc16is7xx_fifo_read(port, one->buf, rxlen); bytes_read = rxlen; } @@ -672,7 +672,7 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen, } for (i = 0; i < bytes_read; ++i) { - ch = s->buf[i]; + ch = one->buf[i]; if (uart_handle_sysrq_char(port, ch)) continue; @@ -690,10 +690,10 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen, static void sc16is7xx_handle_tx(struct uart_port *port) { - struct sc16is7xx_port *s = dev_get_drvdata(port->dev); struct tty_port *tport = &port->state->port; unsigned long flags; unsigned int txlen; + unsigned char *tail; if (unlikely(port->x_char)) { sc16is7xx_port_write(port, SC16IS7XX_THR_REG, port->x_char); @@ -718,8 +718,9 @@ static void sc16is7xx_handle_tx(struct uart_port *port) txlen = 0; } - txlen = uart_fifo_out(port, s->buf, txlen); - sc16is7xx_fifo_write(port, s->buf, txlen); + txlen = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail, txlen); + sc16is7xx_fifo_write(port, tail, txlen); + uart_xmit_advance(port, txlen); uart_port_lock_irqsave(port, &flags); if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS) -- 2.39.2

9 months

1
0
0 0

FAILED: patch "[PATCH] thermal: core: Allow thermal zones to tell the core to ignore" failed to apply to 6.10-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.10.y git checkout FETCH_HEAD git cherry-pick -x e528be3c87be953b73e7826a2d7e4b837cbad39d # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024072302-policy-spleen-3156@gregkh' --subject-prefix 'PATCH 6.10.y' HEAD^.. Possible dependencies: e528be3c87be ("thermal: core: Allow thermal zones to tell the core to ignore them") 7c8267275de6 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From e528be3c87be953b73e7826a2d7e4b837cbad39d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" <rafael.j.wysocki(a)intel.com> Date: Wed, 17 Jul 2024 21:45:02 +0200 Subject: [PATCH] thermal: core: Allow thermal zones to tell the core to ignore them The iwlwifi wireless driver registers a thermal zone that is only needed when the network interface handled by it is up and it wants that thermal zone to be effectively ignored by the core otherwise. Before commit a8a261774466 ("thermal: core: Call monitor_thermal_zone() if zone temperature is invalid") that could be achieved by returning an error code from the thermal zone's .get_temp() callback because the core did not really handle errors returned by it almost at all. However, commit a8a261774466 made the core attempt to recover from the situation in which the temperature of a thermal zone cannot be determined due to errors returned by its .get_temp() and is always invalid from the core's perspective. That was done because there are thermal zones in which .get_temp() returns errors to start with due to some difficulties related to the initialization ordering, but then it will start to produce valid temperature values at one point. Unfortunately, the simple approach taken by commit a8a261774466, which is to poll the thermal zone periodically until its .get_temp() callback starts to return valid temperature values, is at odds with the special thermal zone in iwlwifi in which .get_temp() may always return an error because its network interface may always be down. If that happens, every attempt to invoke the thermal zone's .get_temp() callback resulting in an error causes the thermal core to print a dev_warn() message to the kernel log which is super-noisy. To address this problem, make the core handle the case in which .get_temp() returns 0, but the temperature value returned by it is not actually valid, in a special way. Namely, make the core completely ignore the invalid temperature value coming from .get_temp() in that case, which requires folding in update_temperature() into its caller and a few related changes. On the iwlwifi side, modify iwl_mvm_tzone_get_temp() to return 0 and put THERMAL_TEMP_INVALID into the temperature return memory location instead of returning an error when the firmware is not running or it is not of the right type. Also, to clearly separate the handling of invalid temperature values from the thermal zone initialization, introduce a special THERMAL_TEMP_INIT value specifically for the latter purpose. Fixes: a8a261774466 ("thermal: core: Call monitor_thermal_zone() if zone temperature is invalid") Closes: https://lore.kernel.org/linux-pm/20240715044527.GA1544@sol.localdomain/ Reported-by: Eric Biggers <ebiggers(a)kernel.org> Reported-by: Stefan Lippers-Hollmann <s.l-h(a)gmx.de> Link: https://bugzilla.kernel.org/show_bug.cgi?id=201761 Tested-by: Oleksandr Natalenko <oleksandr(a)natalenko.name> Tested-by: Stefan Lippers-Hollmann <s.l-h(a)gmx.de> Cc: 6.10+ <stable(a)vger.kernel.org> # 6.10+ Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Link: https://patch.msgid.link/4950004.31r3eYUQgx@rjwysocki.net [ rjw: Rebased on top of the current mainline ] Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c index ed0796aff722..d92470960b38 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c @@ -621,8 +621,14 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device, guard(mvm)(mvm); if (!iwl_mvm_firmware_running(mvm) || - mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) - return -ENODATA; + mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { + /* + * Tell the core that there is no valid temperature value to + * return, but it need not worry about this. + */ + *temperature = THERMAL_TEMP_INVALID; + return 0; + } ret = iwl_mvm_get_temp(mvm, &temp); if (ret) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 8795187fbc52..f6e700e48aad 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -300,8 +300,6 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz) thermal_zone_device_set_polling(tz, tz->passive_delay_jiffies); else if (tz->polling_delay_jiffies) thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies); - else if (tz->temperature == THERMAL_TEMP_INVALID) - thermal_zone_device_set_polling(tz, msecs_to_jiffies(THERMAL_RECHECK_DELAY_MS)); } static struct thermal_governor *thermal_get_tz_governor(struct thermal_zone_device *tz) @@ -382,7 +380,7 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, td->threshold = trip->temperature; if (tz->last_temperature >= old_threshold && - tz->last_temperature != THERMAL_TEMP_INVALID) { + tz->last_temperature != THERMAL_TEMP_INIT) { /* * Mitigation is under way, so it needs to stop if the zone * temperature falls below the low temperature of the trip. @@ -417,27 +415,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, } } -static void update_temperature(struct thermal_zone_device *tz) -{ - int temp, ret; - - ret = __thermal_zone_get_temp(tz, &temp); - if (ret) { - if (ret != -EAGAIN) - dev_warn(&tz->device, - "failed to read out thermal zone (%d)\n", - ret); - return; - } - - tz->last_temperature = tz->temperature; - tz->temperature = temp; - - trace_thermal_temperature(tz); - - thermal_genl_sampling_temp(tz->id, temp); -} - static void thermal_zone_device_check(struct work_struct *work) { struct thermal_zone_device *tz = container_of(work, struct @@ -452,7 +429,7 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check); - tz->temperature = THERMAL_TEMP_INVALID; + tz->temperature = THERMAL_TEMP_INIT; tz->passive = 0; tz->prev_low_trip = -INT_MAX; tz->prev_high_trip = INT_MAX; @@ -504,6 +481,7 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, struct thermal_trip_desc *td; LIST_HEAD(way_down_list); LIST_HEAD(way_up_list); + int temp, ret; if (tz->suspended) return; @@ -511,10 +489,29 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, if (!thermal_zone_device_is_enabled(tz)) return; - update_temperature(tz); + ret = __thermal_zone_get_temp(tz, &temp); + if (ret) { + if (ret != -EAGAIN) + dev_info(&tz->device, "Temperature check failed (%d)\n", ret); - if (tz->temperature == THERMAL_TEMP_INVALID) + thermal_zone_device_set_polling(tz, msecs_to_jiffies(THERMAL_RECHECK_DELAY_MS)); + return; + } else if (temp <= THERMAL_TEMP_INVALID) { + /* + * Special case: No valid temperature value is available, but + * the zone owner does not want the core to do anything about + * it. Continue regular zone polling if needed, so that this + * function can be called again, but skip everything else. + */ goto monitor; + } + + tz->last_temperature = tz->temperature; + tz->temperature = temp; + + trace_thermal_temperature(tz); + + thermal_genl_sampling_temp(tz->id, temp); tz->notify_event = event; diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 30c0e78859a7..ba8e6fc807ca 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -133,6 +133,9 @@ struct thermal_zone_device { struct thermal_trip_desc trips[] __counted_by(num_trips); }; +/* Initial thermal zone temperature. */ +#define THERMAL_TEMP_INIT INT_MIN + /* * Default delay after a failing thermal zone temperature check before * attempting to check it again. diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index 81e019493557..aedb8369e2aa 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -163,6 +163,8 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp) } ret = __thermal_zone_get_temp(tz, temp); + if (!ret && *temp <= THERMAL_TEMP_INVALID) + ret = -ENODATA; unlock: mutex_unlock(&tz->lock);

9 months

2
1
0 0

[PATCH 5.15.y 0/8] Backport patches for DAMON merge regions fix

by SeongJae Park

Commit 310d6c15e910 ("mm/damon/core: merge regions aggressively when max_nr_regions") causes a build warning and a build failure [1] on 5.15.y. Those are due to 1) unnecessarily strict type check from max(), and 2) use of not-yet-introduced damon_ctx->attrs field, respectively. Fix the warning by backporting a minmax.h upstream commit that made the type check less strict for unnecessary case, and upstream commits that it depends on. Note that all patches except the fourth one ("minmax: fix header inclusions") are clean cherry-picks of upstream commit. For the fourth one, minor conflict resolving was needed. Also, the last patch, which is the backport of the DAMON fix, was cleanly cherry-picked, but added manual fix for the build failure. [1] https://lore.kernel.org/2024071532-pebble-jailhouse-48b2@gregkh Andy Shevchenko (1): minmax: fix header inclusions Bart Van Assche (1): tracing: Define the is_signed_type() macro once David Laight (3): minmax: allow min()/max()/clamp() if the arguments have the same signedness. minmax: allow comparisons of 'int' against 'unsigned char/short' minmax: relax check to allow comparison between unsigned arguments and signed constants Jason A. Donenfeld (2): minmax: sanity check constant bounds when clamping minmax: clamp more efficiently by avoiding extra comparison SeongJae Park (1): mm/damon/core: merge regions aggressively when max_nr_regions is unmet include/linux/compiler.h | 6 +++ include/linux/minmax.h | 89 ++++++++++++++++++++++++++---------- include/linux/overflow.h | 1 - include/linux/trace_events.h | 2 - mm/damon/core.c | 23 ++++++++-- 5 files changed, 90 insertions(+), 31 deletions(-) base-commit: 4d1b7f1bf3858ed48a98c004bda5fdff2cdf13c8 -- 2.39.2

9 months

2
9
0 0

[PATCH 6.1.y 0/7] Backport patches for DAMON merge regions fix

by SeongJae Park

Commit 310d6c15e910 ("mm/damon/core: merge regions aggressively when max_nr_regions") causes a build warning [1] on 6.1.y. That was due to unnecessarily strict type check from max(). Fix the warning by backporting a minmax.h upstream commit that made the type check less strict for unnecessary case, and upstream commits that it depends on. Note that all patches except the third one ("minmax: fix header inclusions") are clean cherry-picks of upstream commit. For the third one, a minor conflict fix was needed. [1] https://lore.kernel.org/2024071519-janitor-robe-779f@gregkh Andy Shevchenko (1): minmax: fix header inclusions David Laight (3): minmax: allow min()/max()/clamp() if the arguments have the same signedness. minmax: allow comparisons of 'int' against 'unsigned char/short' minmax: relax check to allow comparison between unsigned arguments and signed constants Jason A. Donenfeld (2): minmax: sanity check constant bounds when clamping minmax: clamp more efficiently by avoiding extra comparison SeongJae Park (1): mm/damon/core: merge regions aggressively when max_nr_regions is unmet include/linux/minmax.h | 89 ++++++++++++++++++++++++++++++------------ mm/damon/core.c | 21 +++++++++- 2 files changed, 83 insertions(+), 27 deletions(-) base-commit: 291e563ecab1ea89c70172ecf0d6bff7b725d3cb -- 2.39.2

9 months

2
9
0 0

[PATCH 6.10 0/9] 6.10.1-rc1 review

by Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.10.1 release. There are 9 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know. Responses should be made by Thu, 25 Jul 2024 11:40:39 +0000. Anything received after that time might be too late. The whole patch series can be found in one patch at: https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.10.1-rc1… or in the git tree and branch at: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.10.y and the diffstat can be found below. thanks, greg k-h ------------- Pseudo-Shortlog of commits: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Linux 6.10.1-rc1 Richard Fitzgerald <rf(a)opensource.cirrus.com> ASoC: cs35l56: Limit Speaker Volume to +12dB maximum Richard Fitzgerald <rf(a)opensource.cirrus.com> ASoC: cs35l56: Use header defines for Speaker Volume control definition Hao Ge <gehao(a)kylinos.cn> tpm: Use auth only after NULL check in tpm_buf_check_hmac_response() David Howells <dhowells(a)redhat.com> cifs: Fix setting of zero_point after DIO write David Howells <dhowells(a)redhat.com> cifs: Fix server re-repick on subrequest retry Steve French <stfrench(a)microsoft.com> cifs: fix noisy message on copy_file_range David Howells <dhowells(a)redhat.com> cifs: Fix missing fscache invalidation David Howells <dhowells(a)redhat.com> cifs: Fix missing error code set Kees Cook <kees(a)kernel.org> ext4: use memtostr_pad() for s_volume_name ------------- Diffstat: Makefile | 4 ++-- drivers/char/tpm/tpm2-sessions.c | 5 +++-- fs/ext4/ext4.h | 2 +- fs/ext4/ioctl.c | 2 +- fs/smb/client/cifsfs.c | 2 +- fs/smb/client/file.c | 21 +++++++++++++++++---- fs/smb/client/smb2pdu.c | 3 --- include/sound/cs35l56.h | 2 +- sound/soc/codecs/cs35l56.c | 6 +++++- 9 files changed, 31 insertions(+), 16 deletions(-)

9 months

1
9
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror July 2024