May 2020 - Linux-stable-mirror

[PATCH] f2fs: avoid utf8_strncasecmp() with unstable name

by Eric Biggers

From: Eric Biggers <ebiggers(a)google.com> If the dentry name passed to ->d_compare() fits in dentry::d_iname, then it may be concurrently modified by a rename. This can cause undefined behavior (possibly out-of-bounds memory accesses or crashes) in utf8_strncasecmp(), since fs/unicode/ isn't written to handle strings that may be concurrently modified. Fix this by first copying the filename to a stack buffer if needed. This way we get a stable snapshot of the filename. Fixes: 2c2eb7a300cd ("f2fs: Support case-insensitive file name lookups") Cc: <stable(a)vger.kernel.org> # v5.4+ Cc: Al Viro <viro(a)zeniv.linux.org.uk> Cc: Daniel Rosenberg <drosen(a)google.com> Cc: Gabriel Krisman Bertazi <krisman(a)collabora.co.uk> Signed-off-by: Eric Biggers <ebiggers(a)google.com> --- fs/f2fs/dir.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c index 44bfc464df787..5c179b72eb8a8 100644 --- a/fs/f2fs/dir.c +++ b/fs/f2fs/dir.c @@ -1083,6 +1083,7 @@ static int f2fs_d_compare(const struct dentry *dentry, unsigned int len, struct qstr qstr = {.name = str, .len = len }; const struct dentry *parent = READ_ONCE(dentry->d_parent); const struct inode *inode = READ_ONCE(parent->d_inode); + char strbuf[DNAME_INLINE_LEN]; if (!inode || !IS_CASEFOLDED(inode)) { if (len != name->len) @@ -1090,6 +1091,22 @@ static int f2fs_d_compare(const struct dentry *dentry, unsigned int len, return memcmp(str, name->name, len); } + /* + * If the dentry name is stored in-line, then it may be concurrently + * modified by a rename. If this happens, the VFS will eventually retry + * the lookup, so it doesn't matter what ->d_compare() returns. + * However, it's unsafe to call utf8_strncasecmp() with an unstable + * string. Therefore, we have to copy the name into a temporary buffer. + */ + if (len <= DNAME_INLINE_LEN - 1) { + unsigned int i; + + for (i = 0; i < len; i++) + strbuf[i] = READ_ONCE(str[i]); + strbuf[len] = 0; + qstr.name = strbuf; + } + return f2fs_ci_compare(inode, name, &qstr, false); } -- 2.26.2

5 years, 1 month

2
1
0 0

Re: [PATCH v3] virtio_vsock: Fix race condition in virtio_transport_recv_pkt()

by Markus Elfring

> This fixes it by checking sk->sk_shutdown(suggested by Stefano) after > lock_sock since sk->sk_shutdown is set to SHUTDOWN_MASK under the > protection of lock_sock_nested. How do you think about a wording variant like the following? Thus check the data structure member “sk_shutdown” (suggested by Stefano) after a call of the function “lock_sock” since this field is set to “SHUTDOWN_MASK” under the protection of “lock_sock_nested”. Would you like to add the tag “Fixes” to the commit message? Regards, Markus

5 years, 1 month

2
1
0 0

[PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map()

by Anup Patel

For multiple PLIC instances, each PLIC can only target a subset of CPUs which is represented by "lmask" in the "struct plic_priv". Currently, the default irq affinity for each PLIC interrupt is all online CPUs which is illegal value for default irq affinity when we have multiple PLIC instances. To fix this, we now set "lmask" as the default irq affinity in for each interrupt in plic_irqdomain_map(). Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs") Cc: stable(a)vger.kernel.org Signed-off-by: Anup Patel <anup.patel(a)wdc.com> --- drivers/irqchip/irq-sifive-plic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 822e074c0600..9f7f8ce88c00 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -176,9 +176,12 @@ static struct irq_chip plic_chip = { static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) { + struct plic_priv *priv = d->host_data; + irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data, handle_fasteoi_irq, NULL, NULL); irq_set_noprobe(irq); + irq_set_affinity(irq, &priv->lmask); return 0; } -- 2.25.1

5 years, 1 month

4
3
0 0

[PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present

by Anup Patel

For multiple PLIC instances, the plic_init() is called once for each PLIC instance. Due to this we have two issues: 1. cpuhp_setup_state() is called multiple times 2. plic_starting_cpu() can crash for boot CPU if cpuhp_setup_state() is called before boot CPU PLIC handler is available. This patch fixes both above issues. Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs") Cc: stable(a)vger.kernel.org Signed-off-by: Anup Patel <anup.patel(a)wdc.com> --- drivers/irqchip/irq-sifive-plic.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 9f7f8ce88c00..6c54abf5cc5e 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -76,6 +76,7 @@ struct plic_handler { void __iomem *enable_base; struct plic_priv *priv; }; +static bool plic_cpuhp_setup_done; static DEFINE_PER_CPU(struct plic_handler, plic_handlers); static inline void plic_toggle(struct plic_handler *handler, @@ -285,6 +286,7 @@ static int __init plic_init(struct device_node *node, int error = 0, nr_contexts, nr_handlers = 0, i; u32 nr_irqs; struct plic_priv *priv; + struct plic_handler *handler; priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) @@ -313,7 +315,6 @@ static int __init plic_init(struct device_node *node, for (i = 0; i < nr_contexts; i++) { struct of_phandle_args parent; - struct plic_handler *handler; irq_hw_number_t hwirq; int cpu, hartid; @@ -367,9 +368,18 @@ static int __init plic_init(struct device_node *node, nr_handlers++; } - cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING, + /* + * We can have multiple PLIC instances so setup cpuhp state only + * when context handler for current/boot CPU is present. + */ + handler = this_cpu_ptr(&plic_handlers); + if (handler->present && !plic_cpuhp_setup_done) { + cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING, "irqchip/sifive/plic:starting", plic_starting_cpu, plic_dying_cpu); + plic_cpuhp_setup_done = true; + } + pr_info("mapped %d interrupts with %d handlers for %d contexts.\n", nr_irqs, nr_handlers, nr_contexts); set_handle_irq(plic_handle_irq); -- 2.25.1

5 years, 1 month

4
3
0 0

[PATCH v2] usb: dwc2: Fix shutdown callback in platform

by Minas Harutyunyan

To avoid lot of interrupts from dwc2 core, which can be asserted in specific conditions need to disable interrupts on HW level instead of disable IRQs on Kernel level, because of IRQ can be shared between drivers. Cc: stable(a)vger.kernel.org Fixes: a40a00318c7fc ("usb: dwc2: add shutdown callback to platform variant") Tested-by: Frank Mori Hess <fmh6jj(a)gmail.com> Reviewed-by: Alan Stern <stern(a)rowland.harvard.edu> Reviewed-by: Doug Anderson <dianders(a)chromium.org> Reviewed-by: Frank Mori Hess <fmh6jj(a)gmail.com> Signed-off-by: Minas Harutyunyan <hminas(a)synopsys.com> --- Changes in V2: - added synchronize_irq() drivers/usb/dwc2/platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index e571c8ae65ec..a6360f5f6cd4 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -342,7 +342,8 @@ static void dwc2_driver_shutdown(struct platform_device *dev) { struct dwc2_hsotg *hsotg = platform_get_drvdata(dev); - disable_irq(hsotg->irq); + dwc2_disable_global_interrupts(hsotg); + synchronize_irq(hsotg->irq); } /** -- 2.11.0

5 years, 1 month

1
0
0 0

Re: patch "driver core: Update device link status correctly for SYNC_STATE_ONLY" added to driver-core-next

by Saravana Kannan

On Thu, May 28, 2020 at 12:19 AM <gregkh(a)linuxfoundation.org> wrote: > > > This is a note to let you know that I've just added the patch titled > > driver core: Update device link status correctly for SYNC_STATE_ONLY > > to my driver-core git tree which can be found at > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git > in the driver-core-next branch. > > The patch will show up in the next release of the linux-next tree > (usually sometime within the next 24 hours during the week.) > > The patch will also be merged in the next major kernel release > during the merge window. > > If you have any questions about this process, please let me know. Not sure if this is already/automatically queued, but this needs to go to stable@ too. Cc-ing the list to make sure it's picked up. -Saravana > > > From 8c3e315d4296421cd26b3300ee0ac117f0877f20 Mon Sep 17 00:00:00 2001 > From: Saravana Kannan <saravanak(a)google.com> > Date: Tue, 26 May 2020 15:09:27 -0700 > Subject: driver core: Update device link status correctly for SYNC_STATE_ONLY > links > > When SYNC_STATE_ONLY support was added in commit 05ef983e0d65 ("driver > core: Add device link support for SYNC_STATE_ONLY flag"), > SYNC_STATE_ONLY links were treated similar to STATELESS links in terms > of not blocking consumer probe if the supplier hasn't probed yet. > > That caused a SYNC_STATE_ONLY device link's status to not get updated. > Since SYNC_STATE_ONLY device link is no longer useful once the > consumer probes, commit 21c27f06587d ("driver core: Fix > SYNC_STATE_ONLY device link implementation") addresses the status > update issue by deleting the SYNC_STATE_ONLY device link instead of > complicating the status update code. > > However, there are still some cases where we need to update the status > of a SYNC_STATE_ONLY device link. This is because a SYNC_STATE_ONLY > device link can later get converted into a normal MANAGED device link > when a normal MANAGED device link is created between a supplier and > consumer that already have a SYNC_STATE_ONLY device link between them. > > If a SYNC_STATE_ONLY device link's status isn't maintained correctly > till it's converted to a normal MANAGED device link, then the normal > MANAGED device link will end up with a wrong link status. This can cause > a warning stack trace[1] when the consumer device probes successfully. > > This commit fixes the SYNC_STATE_ONLY device link status update issue > where it wouldn't transition correctly from DL_STATE_DORMANT or > DL_STATE_AVAILABLE to DL_STATE_CONSUMER_PROBE. It also resets the status > back to DL_STATE_DORMANT or DL_STATE_AVAILABLE if the consumer probe > fails. > > [1] - https://lore.kernel.org/lkml/20200522204120.3b3c9ed6@apollo/ > Fixes: 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag") > Fixes: 21c27f06587d ("driver core: Fix SYNC_STATE_ONLY device link implementation") > Reported-by: Michael Walle <michael(a)walle.cc> > Tested-by: Michael Walle <michael(a)walle.cc> > Signed-off-by: Saravana Kannan <saravanak(a)google.com> > Reviewed-by: Rafael J. Wysocki <rrafael.j.wysocki(a)intel.com> > Link: https://lore.kernel.org/r/20200526220928.49939-1-saravanak@google.com > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > --- > drivers/base/core.c | 34 ++++++++++++++++++++++++++-------- > 1 file changed, 26 insertions(+), 8 deletions(-) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 791b7530599f..9a76dd44cb37 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -646,9 +646,17 @@ static void device_links_missing_supplier(struct device *dev) > { > struct device_link *link; > > - list_for_each_entry(link, &dev->links.suppliers, c_node) > - if (link->status == DL_STATE_CONSUMER_PROBE) > + list_for_each_entry(link, &dev->links.suppliers, c_node) { > + if (link->status != DL_STATE_CONSUMER_PROBE) > + continue; > + > + if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { > WRITE_ONCE(link->status, DL_STATE_AVAILABLE); > + } else { > + WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY)); > + WRITE_ONCE(link->status, DL_STATE_DORMANT); > + } > + } > } > > /** > @@ -687,11 +695,11 @@ int device_links_check_suppliers(struct device *dev) > device_links_write_lock(); > > list_for_each_entry(link, &dev->links.suppliers, c_node) { > - if (!(link->flags & DL_FLAG_MANAGED) || > - link->flags & DL_FLAG_SYNC_STATE_ONLY) > + if (!(link->flags & DL_FLAG_MANAGED)) > continue; > > - if (link->status != DL_STATE_AVAILABLE) { > + if (link->status != DL_STATE_AVAILABLE && > + !(link->flags & DL_FLAG_SYNC_STATE_ONLY)) { > device_links_missing_supplier(dev); > ret = -EPROBE_DEFER; > break; > @@ -952,11 +960,21 @@ static void __device_links_no_driver(struct device *dev) > if (!(link->flags & DL_FLAG_MANAGED)) > continue; > > - if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) > + if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) { > device_link_drop_managed(link); > - else if (link->status == DL_STATE_CONSUMER_PROBE || > - link->status == DL_STATE_ACTIVE) > + continue; > + } > + > + if (link->status != DL_STATE_CONSUMER_PROBE && > + link->status != DL_STATE_ACTIVE) > + continue; > + > + if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { > WRITE_ONCE(link->status, DL_STATE_AVAILABLE); > + } else { > + WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY)); > + WRITE_ONCE(link->status, DL_STATE_DORMANT); > + } > } > > dev->links.status = DL_DEV_NO_DRIVER; > -- > 2.26.2 > >

5 years, 1 month

2
1
0 0

Re: [PATCH] usb: dwc2: Fix shutdown callback in platform

by Frank Mori Hess

Hi Minas, On Fri, May 29, 2020 at 3:50 PM Minas Harutyunyan <Minas.Harutyunyan(a)synopsys.com> wrote: > > Can you test it on your setup and confirm (to keep "Tested-by: Frank.." > tag). > I just tested the dwc2_disable_global_interrupts(hsotg); synchronize_irq(hsotg->irq); version of dwc2 shutdown, and booting a new kernel with kexec worked fine for me. -- Frank

5 years, 1 month

1
0
0 0

[PATCH 2/4] lkdtm/heap: Avoid edge and middle of slabs

by Kees Cook

Har har, after I moved the slab freelist pointer into the middle of the slab, now it looks like the contents are getting poisoned. Adjust the test to avoid the freelist pointer again. Fixes: 3202fa62fb43 ("slub: relocate freelist pointer to middle of object") Cc: stable(a)vger.kernel.org Signed-off-by: Kees Cook <keescook(a)chromium.org> --- drivers/misc/lkdtm/heap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c index 3c5cec85edce..1323bc16f113 100644 --- a/drivers/misc/lkdtm/heap.c +++ b/drivers/misc/lkdtm/heap.c @@ -58,11 +58,12 @@ void lkdtm_READ_AFTER_FREE(void) int *base, *val, saw; size_t len = 1024; /* - * The slub allocator uses the first word to store the free - * pointer in some configurations. Use the middle of the - * allocation to avoid running into the freelist + * The slub allocator will use the either the first word or + * the middle of the allocation to store the free pointer, + * depending on configurations. Store in the second word to + * avoid running into the freelist. */ - size_t offset = (len / sizeof(*base)) / 2; + size_t offset = sizeof(*base); base = kmalloc(len, GFP_KERNEL); if (!base) { -- 2.25.1

5 years, 1 month

1
0
0 0

Re: [PATCH] usb: dwc2: Fix shutdown callback in platform

by Frank Mori Hess

On Fri, May 29, 2020 at 3:33 PM Minas Harutyunyan <Minas.Harutyunyan(a)synopsys.com> wrote: > So, can we conclude on this solution? > > dwc2_disable_global_interrupts(hsotg); > synchronize_irq(hsotg->irq) That solution is fine with me. Disabling the dwc2 interrupt sources in initialization before the handlers are registered is also worth doing, but it doesn't have to be in the same patch. -- Frank

5 years, 1 month

2
1
0 0

Re: [PATCH] usb: dwc2: Fix shutdown callback in platform

by Doug Anderson

Hi, On Fri, May 29, 2020 at 9:30 AM Minas Harutyunyan <Minas.Harutyunyan(a)synopsys.com> wrote: > > Hi Doug, > > On 5/29/2020 6:49 PM, Doug Anderson wrote: > > Hi, > > > > On Fri, May 29, 2020 at 4:51 AM Minas Harutyunyan > > <Minas.Harutyunyan(a)synopsys.com> wrote: > >> > >> To avoid lot of interrupts from dwc2 core, which can be asserted in > >> specific conditions need to disable interrupts on HW level instead of > >> disable IRQs on Kernel level, because of IRQ can be shared between > >> drivers. > >> > >> Cc: stable(a)vger.kernel.org > >> Fixes: a40a00318c7fc ("usb: dwc2: add shutdown callback to platform variant") > >> Tested-by: Frank Mori Hess <fmh6jj(a)gmail.com> > >> Signed-off-by: Minas Harutyunyan <hminas(a)synopsys.com> > >> --- > >> drivers/usb/dwc2/platform.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > >> index e571c8ae65ec..ada5b66b948e 100644 > >> --- a/drivers/usb/dwc2/platform.c > >> +++ b/drivers/usb/dwc2/platform.c > >> @@ -342,7 +342,7 @@ static void dwc2_driver_shutdown(struct platform_device *dev) > >> { > >> struct dwc2_hsotg *hsotg = platform_get_drvdata(dev); > >> > >> - disable_irq(hsotg->irq); > >> + dwc2_disable_global_interrupts(hsotg); > >> } > > > > I could be wrong, but I think it would be better to instead end up > > with both calls, like: > > > > dwc2_disable_global_interrupts(hsotg); > > disable_irq(hsotg->irq); > > > > To some extent it's slightly overkill, but the disable_irq() function > > has the nice "and wait for completion" bit. Your new call doesn't do > > this. > > > If dwc2 currently handling some interrupt then below patch can allow to > wait until interrupt will be handled: > > spin_lock(&hsotg->lock); > dwc2_disable_global_interrupts(hsotg); > spin_unlock(&hsotg->lock); Would that really work? If you've got a two core system and the interrupt is just firing on a different core but hasn't acquired the spinlock then your code might get the spinlock, disable the interrupts, and then release the spinlock. The interrupt handler will still be running on the other CPU and now will get the spinlock. > but on other hand dwc2 have 3 subsequent interrupt handlers - core, > gadget, host and not clear which of handler completed. > > > That being said, though, you still won't wait for the completion of > > the IRQ handler for the "other drivers" you reference, right. Maybe a > > better fix would be to add a shutdown callback for those other drivers > > and just keep relying on disable_irq()? > > > I have look to other drivers where used disable_irq() - no any driver > care about SHARED irq's. In that case your suggestion to use both > disabling is looks Ok. I'm not sure I understand. Are you saying that you'll just add shutdown callbacks to all the drivers using this IRQ and call disable_irq() there? That seems like the best solution to me. -Doug

5 years, 1 month

3
6
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror May 2020