From: Randy Dunlap rdunlap@infradead.org
[ Upstream commit c7fabbc51352f50cc58242a6dc3b9c1a3599849b ]
Drop duplicated words in sound/pci/. {and, the, at}
Signed-off-by: Randy Dunlap rdunlap@infradead.org Link: https://lore.kernel.org/r/20200806021926.32418-1-rdunlap@infradead.org Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Sasha Levin sashal@kernel.org --- sound/pci/cs46xx/cs46xx_lib.c | 2 +- sound/pci/cs46xx/dsp_spos_scb_lib.c | 2 +- sound/pci/hda/hda_codec.c | 2 +- sound/pci/hda/hda_generic.c | 2 +- sound/pci/hda/patch_sigmatel.c | 2 +- sound/pci/ice1712/prodigy192.c | 2 +- sound/pci/oxygen/xonar_dg.c | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c index 146e1a3498c73..419da70cd942a 100644 --- a/sound/pci/cs46xx/cs46xx_lib.c +++ b/sound/pci/cs46xx/cs46xx_lib.c @@ -780,7 +780,7 @@ static void snd_cs46xx_set_capture_sample_rate(struct snd_cs46xx *chip, unsigned rate = 48000 / 9;
/* - * We can not capture at at rate greater than the Input Rate (48000). + * We can not capture at a rate greater than the Input Rate (48000). * Return an error if an attempt is made to stray outside that limit. */ if (rate > 48000) diff --git a/sound/pci/cs46xx/dsp_spos_scb_lib.c b/sound/pci/cs46xx/dsp_spos_scb_lib.c index 8d0a3d3573457..8ef51a29380af 100644 --- a/sound/pci/cs46xx/dsp_spos_scb_lib.c +++ b/sound/pci/cs46xx/dsp_spos_scb_lib.c @@ -1739,7 +1739,7 @@ int cs46xx_iec958_pre_open (struct snd_cs46xx *chip) struct dsp_spos_instance * ins = chip->dsp_spos_instance;
if ( ins->spdif_status_out & DSP_SPDIF_STATUS_OUTPUT_ENABLED ) { - /* remove AsynchFGTxSCB and and PCMSerialInput_II */ + /* remove AsynchFGTxSCB and PCMSerialInput_II */ cs46xx_dsp_disable_spdif_out (chip);
/* save state */ diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index f3a6b1d869d8a..dbeb62362f1c3 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3410,7 +3410,7 @@ EXPORT_SYMBOL_GPL(snd_hda_set_power_save); * @nid: NID to check / update * * Check whether the given NID is in the amp list. If it's in the list, - * check the current AMP status, and update the the power-status according + * check the current AMP status, and update the power-status according * to the mute status. * * This function is supposed to be set or called from the check_power_status diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 2609161707a41..97adb7e340f99 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -825,7 +825,7 @@ static void activate_amp_in(struct hda_codec *codec, struct nid_path *path, } }
-/* sync power of each widget in the the given path */ +/* sync power of each widget in the given path */ static hda_nid_t path_power_update(struct hda_codec *codec, struct nid_path *path, bool allow_powerdown) diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index d8168aa2cef38..85c33f528d7b3 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -845,7 +845,7 @@ static int stac_auto_create_beep_ctls(struct hda_codec *codec, static struct snd_kcontrol_new beep_vol_ctl = HDA_CODEC_VOLUME(NULL, 0, 0, 0);
- /* check for mute support for the the amp */ + /* check for mute support for the amp */ if ((caps & AC_AMPCAP_MUTE) >> AC_AMPCAP_MUTE_SHIFT) { const struct snd_kcontrol_new *temp; if (spec->anabeep_nid == nid) diff --git a/sound/pci/ice1712/prodigy192.c b/sound/pci/ice1712/prodigy192.c index 3919aed39ca03..5e52086d7b986 100644 --- a/sound/pci/ice1712/prodigy192.c +++ b/sound/pci/ice1712/prodigy192.c @@ -31,7 +31,7 @@ * Experimentally I found out that only a combination of * OCKS0=1, OCKS1=1 (128fs, 64fs output) and ice1724 - * VT1724_MT_I2S_MCLK_128X=0 (256fs input) yields correct - * sampling rate. That means the the FPGA doubles the + * sampling rate. That means that the FPGA doubles the * MCK01 rate. * * Copyright (c) 2003 Takashi Iwai tiwai@suse.de diff --git a/sound/pci/oxygen/xonar_dg.c b/sound/pci/oxygen/xonar_dg.c index 4cf3200e988b0..df44135e1b0c9 100644 --- a/sound/pci/oxygen/xonar_dg.c +++ b/sound/pci/oxygen/xonar_dg.c @@ -39,7 +39,7 @@ * GPIO 4 <- headphone detect * GPIO 5 -> enable ADC analog circuit for the left channel * GPIO 6 -> enable ADC analog circuit for the right channel - * GPIO 7 -> switch green rear output jack between CS4245 and and the first + * GPIO 7 -> switch green rear output jack between CS4245 and the first * channel of CS4361 (mechanical relay) * GPIO 8 -> enable output to speakers *
From: Qiushi Wu wu000273@umn.edu
[ Upstream commit c4c59b95b7f7d4cef5071b151be2dadb33f3287b ]
pm_runtime_get_sync() increments the runtime PM usage counter even when it returns an error code, causing incorrect ref count if pm_runtime_put_noidle() is not called in error handling paths. Thus call pm_runtime_put_noidle() if pm_runtime_get_sync() fails.
Signed-off-by: Qiushi Wu wu000273@umn.edu Link: https://lore.kernel.org/r/20200614033749.2975-1-wu000273@umn.edu Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/img/img-i2s-in.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/img/img-i2s-in.c b/sound/soc/img/img-i2s-in.c index c22880aea82a2..7e48c740bf550 100644 --- a/sound/soc/img/img-i2s-in.c +++ b/sound/soc/img/img-i2s-in.c @@ -346,8 +346,10 @@ static int img_i2s_in_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) chan_control_mask = IMG_I2S_IN_CH_CTL_CLK_TRANS_MASK;
ret = pm_runtime_get_sync(i2s->dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put_noidle(i2s->dev); return ret; + }
for (i = 0; i < i2s->active_channels; i++) img_i2s_in_ch_disable(i2s, i);
From: Qiushi Wu wu000273@umn.edu
[ Upstream commit 6b9fbb073636906eee9fe4d4c05a4f445b9e2a23 ]
pm_runtime_get_sync() increments the runtime PM usage counter even when it returns an error code, causing incorrect ref count if pm_runtime_put_noidle() is not called in error handling paths. Thus call pm_runtime_put_noidle() if pm_runtime_get_sync() fails.
Signed-off-by: Qiushi Wu wu000273@umn.edu Link: https://lore.kernel.org/r/20200614033344.1814-1-wu000273@umn.edu Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/img/img-parallel-out.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/img/img-parallel-out.c b/sound/soc/img/img-parallel-out.c index acc005217be06..f56752662b199 100644 --- a/sound/soc/img/img-parallel-out.c +++ b/sound/soc/img/img-parallel-out.c @@ -166,8 +166,10 @@ static int img_prl_out_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) }
ret = pm_runtime_get_sync(prl->dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put_noidle(prl->dev); return ret; + }
reg = img_prl_out_readl(prl, IMG_PRL_OUT_CTL); reg = (reg & ~IMG_PRL_OUT_CTL_EDGE_MASK) | control_set;
From: Qiushi Wu wu000273@umn.edu
[ Upstream commit deca195383a6085be62cb453079e03e04d618d6e ]
Calling pm_runtime_get_sync increments the counter even in case of failure, causing incorrect ref count if pm_runtime_put is not called in error handling paths. Call pm_runtime_put if pm_runtime_get_sync fails.
Signed-off-by: Qiushi Wu wu000273@umn.edu Reviewed-by: Jon Hunter jonathanh@nvidia.com Link: https://lore.kernel.org/r/20200613204422.24484-1-wu000273@umn.edu Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/tegra/tegra30_ahub.c | 4 +++- sound/soc/tegra/tegra30_i2s.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c index 43679aeeb12be..88e838ac937dc 100644 --- a/sound/soc/tegra/tegra30_ahub.c +++ b/sound/soc/tegra/tegra30_ahub.c @@ -655,8 +655,10 @@ static int tegra30_ahub_resume(struct device *dev) int ret;
ret = pm_runtime_get_sync(dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put(dev); return ret; + } ret = regcache_sync(ahub->regmap_ahub); ret |= regcache_sync(ahub->regmap_apbif); pm_runtime_put(dev); diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c index 0b176ea24914b..bf155c5092f06 100644 --- a/sound/soc/tegra/tegra30_i2s.c +++ b/sound/soc/tegra/tegra30_i2s.c @@ -551,8 +551,10 @@ static int tegra30_i2s_resume(struct device *dev) int ret;
ret = pm_runtime_get_sync(dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put(dev); return ret; + } ret = regcache_sync(i2s->regmap); pm_runtime_put(dev);
From: Andy Shevchenko andriy.shevchenko@linux.intel.com
[ Upstream commit 3ea2e4eab64cefa06055bb0541fcdedad4b48565 ]
Intel Emmitsburg PCH has the same LPSS than Intel Ice Lake. Add the new IDs to the list of supported devices.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones lee.jones@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/mfd/intel-lpss-pci.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c index 742d6c1973f4f..adea7ff63132f 100644 --- a/drivers/mfd/intel-lpss-pci.c +++ b/drivers/mfd/intel-lpss-pci.c @@ -176,6 +176,9 @@ static const struct pci_device_id intel_lpss_pci_ids[] = { { PCI_VDEVICE(INTEL, 0x1ac4), (kernel_ulong_t)&bxt_info }, { PCI_VDEVICE(INTEL, 0x1ac6), (kernel_ulong_t)&bxt_info }, { PCI_VDEVICE(INTEL, 0x1aee), (kernel_ulong_t)&bxt_uart_info }, + /* EBG */ + { PCI_VDEVICE(INTEL, 0x1bad), (kernel_ulong_t)&bxt_uart_info }, + { PCI_VDEVICE(INTEL, 0x1bae), (kernel_ulong_t)&bxt_uart_info }, /* GLK */ { PCI_VDEVICE(INTEL, 0x31ac), (kernel_ulong_t)&glk_i2c_info }, { PCI_VDEVICE(INTEL, 0x31ae), (kernel_ulong_t)&glk_i2c_info },
From: Stephan Gerhold stephan@gerhold.net
[ Upstream commit e2ee9edc282961783d519c760bbaa20fed4dec38 ]
The original qcom kernel changed the PDM GPIOs to be pull-down during sleep at some point. Reportedly this was done because there was some "leakage at PDM outputs during sleep":
https://source.codeaurora.org/quic/la/kernel/msm-3.10/commit/?id=0f87e08c1cd...
I cannot say how effective this is, but everything seems to work fine with this change so let's apply the same to mainline just to be sure.
Cc: Srinivas Kandagatla srinivas.kandagatla@linaro.org Signed-off-by: Stephan Gerhold stephan@gerhold.net Link: https://lore.kernel.org/r/20200605185916.318494-3-stephan@gerhold.net Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 60d218c5275c1..6754817658fa4 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -529,7 +529,7 @@ pinconf { pins = "gpio63", "gpio64", "gpio65", "gpio66", "gpio67", "gpio68"; drive-strength = <2>; - bias-disable; + bias-pull-down; }; }; };
From: Alexey Kardashevskiy aik@ozlabs.ru
[ Upstream commit f0993c839e95dd6c7f054a1015e693c87e33e4fb ]
xive_native_provision_pages() allocates memory and passes the pointer to OPAL so kmemleak cannot find the pointer usage in the kernel memory and produces a false positive report (below) (even if the kernel did scan OPAL memory, it is unable to deal with __pa() addresses anyway).
This silences the warning.
unreferenced object 0xc000200350c40000 (size 65536): comm "qemu-system-ppc", pid 2725, jiffies 4294946414 (age 70776.530s) hex dump (first 32 bytes): 02 00 00 00 50 00 00 00 00 00 00 00 00 00 00 00 ....P........... 01 00 08 07 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<0000000081ff046c>] xive_native_alloc_vp_block+0x120/0x250 [<00000000d555d524>] kvmppc_xive_compute_vp_id+0x248/0x350 [kvm] [<00000000d69b9c9f>] kvmppc_xive_connect_vcpu+0xc0/0x520 [kvm] [<000000006acbc81c>] kvm_arch_vcpu_ioctl+0x308/0x580 [kvm] [<0000000089c69580>] kvm_vcpu_ioctl+0x19c/0xae0 [kvm] [<00000000902ae91e>] ksys_ioctl+0x184/0x1b0 [<00000000f3e68bd7>] sys_ioctl+0x48/0xb0 [<0000000001b2c127>] system_call_exception+0x124/0x1f0 [<00000000d2b2ee40>] system_call_common+0xe8/0x214
Signed-off-by: Alexey Kardashevskiy aik@ozlabs.ru Signed-off-by: Michael Ellerman mpe@ellerman.id.au Link: https://lore.kernel.org/r/20200612043303.84894-1-aik@ozlabs.ru Signed-off-by: Sasha Levin sashal@kernel.org --- arch/powerpc/sysdev/xive/native.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c index cb1f51ad48e40..411f785cdfb51 100644 --- a/arch/powerpc/sysdev/xive/native.c +++ b/arch/powerpc/sysdev/xive/native.c @@ -22,6 +22,7 @@ #include <linux/delay.h> #include <linux/cpumask.h> #include <linux/mm.h> +#include <linux/kmemleak.h>
#include <asm/prom.h> #include <asm/io.h> @@ -627,6 +628,7 @@ static bool xive_native_provision_pages(void) pr_err("Failed to allocate provisioning page\n"); return false; } + kmemleak_ignore(p); opal_xive_donate_page(chip, __pa(p)); } return true;
From: Jia-Ju Bai baijiaju@tsinghua.edu.cn
[ Upstream commit 6499a0db9b0f1e903d52f8244eacc1d4be00eea2 ]
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
Signed-off-by: Jia-Ju Bai baijiaju@tsinghua.edu.cn Signed-off-by: Sean Young sean@mess.org Signed-off-by: Mauro Carvalho Chehab mchehab+huawei@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/media/pci/ttpci/av7110.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/pci/ttpci/av7110.c b/drivers/media/pci/ttpci/av7110.c index d6816effb8786..d02b5fd940c12 100644 --- a/drivers/media/pci/ttpci/av7110.c +++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt; + u8 data_0 = data[0];
- if ((data[0] < 2) && data[2] == 0xff) { + if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY; - av7110->ci_slot[data[0]].flags = flags; + av7110->ci_slot[data_0].flags = flags; } else ci_get_data(&av7110->ci_rbuffer, av7110->debi_virt,
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;
This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Best regards, Pavel
On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Yes, it seems quite dubious to me. If we *really* want to guard against rogue hardware here, the whole DMA buffer should be copied. I don't think it's worth it, a rogue PCI device can do much more harm.
On Sat, Aug 29, 2020 at 08:16:00PM +0300, Laurent Pinchart wrote:
On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Yes, it seems quite dubious to me. If we *really* want to guard against rogue hardware here, the whole DMA buffer should be copied. I don't think it's worth it, a rogue PCI device can do much more harm.
That is a good point. I'm not sure what the kernel could do to protect against a malicious PCI device (that can do dma) so this patch is totally pointless.
Thanks
Sean
On 2020/8/30 1:16, Laurent Pinchart wrote:
On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Yes, it seems quite dubious to me. If we *really* want to guard against rogue hardware here, the whole DMA buffer should be copied. I don't think it's worth it, a rogue PCI device can do much more harm.
From the original driver code, data[0] is considered to be bad and thus it should be checked, because the content of the DMA buffer may be problematic. Based on this consideration, data[0] can be also modified to bypass the check, and thus its value should be copied to a local variable for the check and use.
I agree with Pavel that the compiler optimization may drop the copying operation, and thus READ_ONCE() should be used here. I will submit a v2 patch soon.
Best wishes, Jia-Ju Bai
Hi Jia-Ju,
On Sun, Aug 30, 2020 at 03:33:11PM +0800, Jia-Ju Bai wrote:
On 2020/8/30 1:16, Laurent Pinchart wrote:
On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Yes, it seems quite dubious to me. If we *really* want to guard against rogue hardware here, the whole DMA buffer should be copied. I don't think it's worth it, a rogue PCI device can do much more harm.
From the original driver code, data[0] is considered to be bad and thus it should be checked, because the content of the DMA buffer may be problematic.
Based on this consideration, data[0] can be also modified to bypass the check, and thus its value should be copied to a local variable for the check and use.
What makes you think the hardware would do that ?
I agree with Pavel that the compiler optimization may drop the copying operation, and thus READ_ONCE() should be used here. I will submit a v2 patch soon.
On 2020/8/31 6:25, Laurent Pinchart wrote:
Hi Jia-Ju,
On Sun, Aug 30, 2020 at 03:33:11PM +0800, Jia-Ju Bai wrote:
On 2020/8/30 1:16, Laurent Pinchart wrote:
On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Yes, it seems quite dubious to me. If we *really* want to guard against rogue hardware here, the whole DMA buffer should be copied. I don't think it's worth it, a rogue PCI device can do much more harm.
From the original driver code, data[0] is considered to be bad and thus it should be checked, because the content of the DMA buffer may be problematic.
Based on this consideration, data[0] can be also modified to bypass the check, and thus its value should be copied to a local variable for the check and use.
What makes you think the hardware would do that ?
Several recent papers show that the bad values from malicious or problematic hardware can cause security problems: [NDSS'19] PeriScope: An Effective Probing and Fuzzing Framework for the Hardware-OS Boundary [NDSS'19] Thunderclap: Exploring Vulnerabilities in Operating System IOMMU Protection via DMA from Untrustworthy Peripherals [USENIX Security'20] USBFuzz: A Framework for Fuzzing USB Drivers by Device Emulation
In this case, the values from DMA can be bad, and the driver should carefully check these values to avoid security problems. IOMMU is an effective method to prevent the hardware from accessing arbitrary memory address via DMA, but it does not check whether the values from DMA are safe.
I find that some drivers (including the av7110 driver) check (or try to check) the values from DMA, and thus I think these drivers have considered such security problems. However, some of these checks are not rigorous, so that they can be bypassed in some cases. The problem that I reported is such an example.
Best wishes, Jia-Ju Bai
Hi Jia-Ju,
On Mon, Aug 31, 2020 at 09:45:14PM +0800, Jia-Ju Bai wrote:
On 2020/8/31 6:25, Laurent Pinchart wrote:
On Sun, Aug 30, 2020 at 03:33:11PM +0800, Jia-Ju Bai wrote:
On 2020/8/30 1:16, Laurent Pinchart wrote:
On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Yes, it seems quite dubious to me. If we *really* want to guard against rogue hardware here, the whole DMA buffer should be copied. I don't think it's worth it, a rogue PCI device can do much more harm.
From the original driver code, data[0] is considered to be bad and thus it should be checked, because the content of the DMA buffer may be problematic.
Based on this consideration, data[0] can be also modified to bypass the check, and thus its value should be copied to a local variable for the check and use.
What makes you think the hardware would do that ?
Several recent papers show that the bad values from malicious or problematic hardware can cause security problems: [NDSS'19] PeriScope: An Effective Probing and Fuzzing Framework for the Hardware-OS Boundary [NDSS'19] Thunderclap: Exploring Vulnerabilities in Operating System IOMMU Protection via DMA from Untrustworthy Peripherals [USENIX Security'20] USBFuzz: A Framework for Fuzzing USB Drivers by Device Emulation
In this case, the values from DMA can be bad, and the driver should carefully check these values to avoid security problems. IOMMU is an effective method to prevent the hardware from accessing arbitrary memory address via DMA, but it does not check whether the values from DMA are safe.
I find that some drivers (including the av7110 driver) check (or try to check) the values from DMA, and thus I think these drivers have considered such security problems. However, some of these checks are not rigorous, so that they can be bypassed in some cases. The problem that I reported is such an example.
The AV7110 is an old chip, I'm not even sure if it can be used with a modern system that supports IOMMUs for PCI devices. Without that, it's game over anyway. Before trying to address the issue of a malicious AV7110 playing with DMA and CPU races, I would ensure that it's worth it.
On 2020/8/29 20:10, Pavel Machek wrote:
Hi!
The value av7110->debi_virt is stored in DMA memory, and it is assigned to data, and thus data[0] can be modified at any time by malicious hardware. In this case, "if (data[0] < 2)" can be passed, but then data[0] can be changed into a large number, which may cause buffer overflow when the code "av7110->ci_slot[data[0]]" is used.
To fix this possible bug, data[0] is assigned to a local variable, which replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work around any such checks, but...
+++ b/drivers/media/pci/ttpci/av7110.c @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt;
u8 data_0 = data[0];
if ((data[0] < 2) && data[2] == 0xff) {
if (data_0 < 2 && data[2] == 0xff) { int flags = 0; if (data[5] > 0) flags |= CA_CI_MODULE_PRESENT; if (data[5] > 5) flags |= CA_CI_MODULE_READY;
av7110->ci_slot[data[0]].flags = flags;
av7110->ci_slot[data_0].flags = flags;This does not even do what it says. Compiler is still free to access data[0] multiple times. It needs READ_ONCE() to be effective.
Thanks for this advice, I will submit a v2 patch soon.
Best wishes, Jia-Ju Bai
From: Luis Chamberlain mcgrof@kernel.org
[ Upstream commit b431ef837e3374da0db8ff6683170359aaa0859c ]
We make an assumption that a debugfs directory exists, but since this can fail ensure it exists before allowing blktrace setup to complete. Otherwise we end up stuffing blktrace files on the debugfs root directory. In the worst case scenario this *in theory* can create an eventual panic *iff* in the future a similarly named file is created prior on the debugfs root directory. This theoretical crash can happen due to a recursive removal followed by a specific dentry removal.
This doesn't fix any known crash, however I have seen the files go into the main debugfs root directory in cases where the debugfs directory was not created due to other internal bugs with blktrace now fixed.
blktrace is also completely useless without this directory, so this ensures to userspace we only setup blktrace if the kernel can stuff files where they are supposed to go into.
debugfs directory creations typically aren't checked for, and we have maintainers doing sweep removals of these checks, but since we need this check to ensure proper userspace blktrace functionality we make sure to annotate the justification for the check.
Signed-off-by: Luis Chamberlain mcgrof@kernel.org Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Bart Van Assche bvanassche@acm.org Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Sasha Levin sashal@kernel.org --- kernel/trace/blktrace.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 7a4ca2deb39bc..1442f6152abc2 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -529,6 +529,18 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, if (!dir) goto err;
+ /* + * As blktrace relies on debugfs for its interface the debugfs directory + * is required, contrary to the usual mantra of not checking for debugfs + * files or directories. + */ + if (IS_ERR_OR_NULL(dir)) { + pr_warn("debugfs_dir not present for %s so skipping\n", + buts->name); + ret = -ENOENT; + goto err; + } + bt->dev = dev; atomic_set(&bt->dropped, 0); INIT_LIST_HEAD(&bt->running_list);
From: Bodo Stroesser bstroesser@ts.fujitsu.com
[ Upstream commit 5a0c256d96f020e4771f6fd5524b80f89a2d3132 ]
If tcmu_handle_completions() has to process a padding shorter than sizeof(struct tcmu_cmd_entry), the current call to tcmu_flush_dcache_range() with sizeof(struct tcmu_cmd_entry) as length param is wrong and causes crashes on e.g. ARM, because tcmu_flush_dcache_range() in this case calls flush_dcache_page(vmalloc_to_page(start)); with start being an invalid address above the end of the vmalloc'ed area.
The fix is to use the minimum of remaining ring space and sizeof(struct tcmu_cmd_entry) as the length param.
The patch was tested on kernel 4.19.118.
See https://bugzilla.kernel.org/show_bug.cgi?id=208045#c10
Link: https://lore.kernel.org/r/20200629093756.8947-1-bstroesser@ts.fujitsu.com Tested-by: JiangYu lnsyyj@hotmail.com Acked-by: Mike Christie michael.christie@oracle.com Signed-off-by: Bodo Stroesser bstroesser@ts.fujitsu.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/target/target_core_user.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 8da89925a874d..238dfe859432e 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -1231,7 +1231,14 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
struct tcmu_cmd_entry *entry = (void *) mb + CMDR_OFF + udev->cmdr_last_cleaned;
- tcmu_flush_dcache_range(entry, sizeof(*entry)); + /* + * Flush max. up to end of cmd ring since current entry might + * be a padding that is shorter than sizeof(*entry) + */ + size_t ring_left = head_to_end(udev->cmdr_last_cleaned, + udev->cmdr_size); + tcmu_flush_dcache_range(entry, ring_left < sizeof(*entry) ? + ring_left : sizeof(*entry));
if (tcmu_hdr_get_op(entry->hdr.len_op) == TCMU_OP_PAD) { UPDATE_HEAD(udev->cmdr_last_cleaned,
From: Robin Murphy robin.murphy@arm.com
[ Upstream commit d3e3d2be688b4b5864538de61e750721a311e4fc ]
Unlike the other instances which represent a complete loss of consistency within the rcache mechanism itself, or a fundamental and obvious misconfiguration by an IOMMU driver, the BUG_ON() in iova_magazine_free_pfns() can be provoked at more or less any time in a "spooky action-at-a-distance" manner by any old device driver passing nonsense to dma_unmap_*() which then propagates through to queue_iova().
Not only is this well outside the IOVA layer's control, it's also nowhere near fatal enough to justify panicking anyway - all that really achieves is to make debugging the offending driver more difficult. Let's simply WARN and otherwise ignore bogus PFNs.
Reported-by: Prakash Gupta guptap@codeaurora.org Signed-off-by: Robin Murphy robin.murphy@arm.com Reviewed-by: Prakash Gupta guptap@codeaurora.org Link: https://lore.kernel.org/r/acbd2d092b42738a03a21b417ce64e27f8c91c86.159110329... Signed-off-by: Joerg Roedel jroedel@suse.de Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/iommu/iova.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c index 34c058c24b9d2..ce5cd05253db9 100644 --- a/drivers/iommu/iova.c +++ b/drivers/iommu/iova.c @@ -814,7 +814,9 @@ iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad) for (i = 0 ; i < mag->size; ++i) { struct iova *iova = private_find_iova(iovad, mag->pfns[i]);
- BUG_ON(!iova); + if (WARN_ON(!iova)) + continue; + private_free_iova(iovad, iova); }
From: Qiushi Wu wu000273@umn.edu
[ Upstream commit 20eca0123a35305e38b344d571cf32768854168c ]
kobject_init_and_add() takes reference even when it fails. If this function returns an error, kobject_put() must be called to properly clean up the memory associated with the object.
Signed-off-by: Qiushi Wu wu000273@umn.edu Reviewed-by: Felix Kuehling Felix.Kuehling@amd.com Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c index 0805c423a5ce0..5cf499a07806a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c @@ -592,8 +592,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev,
ret = kobject_init_and_add(dev->kobj_node, &node_type, sys_props.kobj_nodes, "%d", id); - if (ret < 0) + if (ret < 0) { + kobject_put(dev->kobj_node); return ret; + }
dev->kobj_mem = kobject_create_and_add("mem_banks", dev->kobj_node); if (!dev->kobj_mem) @@ -640,8 +642,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev, return -ENOMEM; ret = kobject_init_and_add(mem->kobj, &mem_type, dev->kobj_mem, "%d", i); - if (ret < 0) + if (ret < 0) { + kobject_put(mem->kobj); return ret; + }
mem->attr.name = "properties"; mem->attr.mode = KFD_SYSFS_FILE_MODE; @@ -659,8 +663,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev, return -ENOMEM; ret = kobject_init_and_add(cache->kobj, &cache_type, dev->kobj_cache, "%d", i); - if (ret < 0) + if (ret < 0) { + kobject_put(cache->kobj); return ret; + }
cache->attr.name = "properties"; cache->attr.mode = KFD_SYSFS_FILE_MODE; @@ -678,8 +684,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev, return -ENOMEM; ret = kobject_init_and_add(iolink->kobj, &iolink_type, dev->kobj_iolink, "%d", i); - if (ret < 0) + if (ret < 0) { + kobject_put(iolink->kobj); return ret; + }
iolink->attr.name = "properties"; iolink->attr.mode = KFD_SYSFS_FILE_MODE; @@ -759,8 +767,10 @@ static int kfd_topology_update_sysfs(void) ret = kobject_init_and_add(sys_props.kobj_topology, &sysprops_type, &kfd_device->kobj, "topology"); - if (ret < 0) + if (ret < 0) { + kobject_put(sys_props.kobj_topology); return ret; + }
sys_props.kobj_nodes = kobject_create_and_add("nodes", sys_props.kobj_topology);
From: Aditya Pakki pakki001@umn.edu
[ Upstream commit 6f2e8acdb48ed166b65d47837c31b177460491ec ]
On calling pm_runtime_get_sync() the reference count of the device is incremented. In case of failure, decrement the reference count before returning the error.
Signed-off-by: Aditya Pakki pakki001@umn.edu Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/radeon/radeon_connectors.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index de656f5553839..b9927101e8450 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -882,8 +882,10 @@ radeon_lvds_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
if (encoder) { @@ -1028,8 +1030,10 @@ radeon_vga_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
encoder = radeon_best_single_encoder(connector); @@ -1166,8 +1170,10 @@ radeon_tv_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
encoder = radeon_best_single_encoder(connector); @@ -1250,8 +1256,10 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
if (radeon_connector->detected_hpd_without_ddc) { @@ -1665,8 +1673,10 @@ radeon_dp_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
if (!force && radeon_check_hpd_status_unchanged(connector)) {
From: Navid Emamdoost navid.emamdoost@gmail.com
[ Upstream commit 9ba8923cbbe11564dd1bf9f3602add9a9cfbb5c6 ]
in amdgpu_driver_open_kms the call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index bb41936df0d97..2beaaf4bee687 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -835,7 +835,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
r = pm_runtime_get_sync(dev->dev); if (r < 0) - return r; + goto pm_put;
fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL); if (unlikely(!fpriv)) { @@ -883,6 +883,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
out_suspend: pm_runtime_mark_last_busy(dev->dev); +pm_put: pm_runtime_put_autosuspend(dev->dev);
return r;
From: Navid Emamdoost navid.emamdoost@gmail.com
[ Upstream commit 5509ac65f2fe5aa3c0003237ec629ca55024307c ]
in amdgpu_drm_ioctl the call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 5e29f14f4b301..63b1e325b45c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1085,11 +1085,12 @@ long amdgpu_drm_ioctl(struct file *filp, dev = file_priv->minor->dev; ret = pm_runtime_get_sync(dev->dev); if (ret < 0) - return ret; + goto out;
ret = drm_ioctl(filp, cmd, arg);
pm_runtime_mark_last_busy(dev->dev); +out: pm_runtime_put_autosuspend(dev->dev); return ret; }
From: Navid Emamdoost navid.emamdoost@gmail.com
[ Upstream commit e008fa6fb41544b63973a529b704ef342f47cc65 ]
in amdgpu_display_crtc_set_config, the call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 686a26de50f91..049a1961c3fa5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -275,7 +275,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
ret = pm_runtime_get_sync(dev->dev); if (ret < 0) - return ret; + goto out;
ret = drm_crtc_helper_set_config(set, ctx);
@@ -290,7 +290,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set, take the current one */ if (active && !adev->have_disp_power_ref) { adev->have_disp_power_ref = true; - return ret; + goto out; } /* if we have no active crtcs, then drop the power ref we got before */ @@ -299,6 +299,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set, adev->have_disp_power_ref = false; }
+out: /* drop the power reference we got coming in here */ pm_runtime_put_autosuspend(dev->dev); return ret;
From: Navid Emamdoost navid.emamdoost@gmail.com
[ Upstream commit f79f94765f8c39db0b7dec1d335ab046aac03f20 ]
The call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index c770d73352a79..c15286858f0bf 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -718,8 +718,10 @@ amdgpu_connector_lvds_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
if (encoder) { @@ -856,8 +858,10 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
encoder = amdgpu_connector_best_single_encoder(connector); @@ -979,8 +983,10 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) { @@ -1329,8 +1335,10 @@ amdgpu_connector_dp_detect(struct drm_connector *connector, bool force)
if (!drm_kms_helper_is_poll_worker()) { r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) + if (r < 0) { + pm_runtime_put_autosuspend(connector->dev->dev); return connector_status_disconnected; + } }
if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
From: Dick Kennedy dick.kennedy@broadcom.com
[ Upstream commit 03dbfe0668e6692917ac278883e0586cd7f7d753 ]
When vports are deleted, it is observed that there is memory/kthread leakage as the vport isn't fully being released.
There is a shost reference taken in scsi_add_host_dma that is not released during scsi_remove_host. It was noticed that other drivers resolve this by doing a scsi_host_put after calling scsi_remove_host.
The vport_delete routine is taking two references one that corresponds to an access to the scsi_host in the vport_delete routine and another that is released after the adapter mailbox command completes that destroys the VPI that corresponds to the vport.
Remove one of the references taken such that the second reference that is put will complete the missing scsi_add_host_dma reference and the shost will be terminated.
Link: https://lore.kernel.org/r/20200630215001.70793-8-jsmart2021@gmail.com Signed-off-by: Dick Kennedy dick.kennedy@broadcom.com Signed-off-by: James Smart jsmart2021@gmail.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/scsi/lpfc/lpfc_vport.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_vport.c b/drivers/scsi/lpfc/lpfc_vport.c index 1ff0f7de91058..64545b300dfc7 100644 --- a/drivers/scsi/lpfc/lpfc_vport.c +++ b/drivers/scsi/lpfc/lpfc_vport.c @@ -653,27 +653,16 @@ lpfc_vport_delete(struct fc_vport *fc_vport) vport->port_state < LPFC_VPORT_READY) return -EAGAIN; } + /* - * This is a bit of a mess. We want to ensure the shost doesn't get - * torn down until we're done with the embedded lpfc_vport structure. - * - * Beyond holding a reference for this function, we also need a - * reference for outstanding I/O requests we schedule during delete - * processing. But once we scsi_remove_host() we can no longer obtain - * a reference through scsi_host_get(). - * - * So we take two references here. We release one reference at the - * bottom of the function -- after delinking the vport. And we - * release the other at the completion of the unreg_vpi that get's - * initiated after we've disposed of all other resources associated - * with the port. + * Take early refcount for outstanding I/O requests we schedule during + * delete processing for unreg_vpi. Always keep this before + * scsi_remove_host() as we can no longer obtain a reference through + * scsi_host_get() after scsi_host_remove as shost is set to SHOST_DEL. */ if (!scsi_host_get(shost)) return VPORT_INVAL; - if (!scsi_host_get(shost)) { - scsi_host_put(shost); - return VPORT_INVAL; - } + lpfc_free_sysfs_attr(vport);
lpfc_debugfs_terminate(vport); @@ -820,8 +809,9 @@ lpfc_vport_delete(struct fc_vport *fc_vport) if (!(vport->vpi_state & LPFC_VPI_REGISTERED) || lpfc_mbx_unreg_vpi(vport)) scsi_host_put(shost); - } else + } else { scsi_host_put(shost); + }
lpfc_free_vpi(phba, vport->vpi); vport->work_port_events = 0;
From: Dave Chinner dchinner@redhat.com
[ Upstream commit 96355d5a1f0ee6dcc182c37db4894ec0c29f1692 ]
In tracking down a problem in this patchset, I discovered we are reclaiming dirty stale inodes. This wasn't discovered until inodes were always attached to the cluster buffer and then the rcu callback that freed inodes was assert failing because the inode still had an active pointer to the cluster buffer after it had been reclaimed.
Debugging the issue indicated that this was a pre-existing issue resulting from the way the inodes are handled in xfs_inactive_ifree. When we free a cluster buffer from xfs_ifree_cluster, all the inodes in cache are marked XFS_ISTALE. Those that are clean have nothing else done to them and so eventually get cleaned up by background reclaim. i.e. it is assumed we'll never dirty/relog an inode marked XFS_ISTALE.
On journal commit dirty stale inodes as are handled by both buffer and inode log items to run though xfs_istale_done() and removed from the AIL (buffer log item commit) or the log item will simply unpin it because the buffer log item will clean it. What happens to any specific inode is entirely dependent on which log item wins the commit race, but the result is the same - stale inodes are clean, not attached to the cluster buffer, and not in the AIL. Hence inode reclaim can just free these inodes without further care.
However, if the stale inode is relogged, it gets dirtied again and relogged into the CIL. Most of the time this isn't an issue, because relogging simply changes the inode's location in the current checkpoint. Problems arise, however, when the CIL checkpoints between two transactions in the xfs_inactive_ifree() deferops processing. This results in the XFS_ISTALE inode being redirtied and inserted into the CIL without any of the other stale cluster buffer infrastructure being in place.
Hence on journal commit, it simply gets unpinned, so it remains dirty in memory. Everything in inode writeback avoids XFS_ISTALE inodes so it can't be written back, and it is not tracked in the AIL so there's not even a trigger to attempt to clean the inode. Hence the inode just sits dirty in memory until inode reclaim comes along, sees that it is XFS_ISTALE, and goes to reclaim it. This reclaiming of a dirty inode caused use after free, list corruptions and other nasty issues later in this patchset.
Hence this patch addresses a violation of the "never log XFS_ISTALE inodes" caused by the deferops processing rolling a transaction and relogging a stale inode in xfs_inactive_free. It also adds a bunch of asserts to catch this problem in debug kernels so that we don't reintroduce this problem in future.
Reproducer for this issue was generic/558 on a v4 filesystem.
Signed-off-by: Dave Chinner dchinner@redhat.com Reviewed-by: Brian Foster bfoster@redhat.com Reviewed-by: Darrick J. Wong darrick.wong@oracle.com Signed-off-by: Darrick J. Wong darrick.wong@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/xfs/xfs_icache.c | 3 ++- fs/xfs/xfs_inode.c | 25 ++++++++++++++++++++++--- fs/xfs/xfs_trans_inode.c | 2 ++ 3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index 901f27ac94abc..56e9043bddc71 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -1127,7 +1127,7 @@ xfs_reclaim_inode( goto out_ifunlock; xfs_iunpin_wait(ip); } - if (xfs_iflags_test(ip, XFS_ISTALE) || xfs_inode_clean(ip)) { + if (xfs_inode_clean(ip)) { xfs_ifunlock(ip); goto reclaim; } @@ -1214,6 +1214,7 @@ xfs_reclaim_inode( xfs_ilock(ip, XFS_ILOCK_EXCL); xfs_qm_dqdetach(ip); xfs_iunlock(ip, XFS_ILOCK_EXCL); + ASSERT(xfs_inode_clean(ip));
__xfs_inode_free(ip); return error; diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index f2d06e1e49066..cd81d6d9848d1 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1772,10 +1772,31 @@ xfs_inactive_ifree( return error; }
+ /* + * We do not hold the inode locked across the entire rolling transaction + * here. We only need to hold it for the first transaction that + * xfs_ifree() builds, which may mark the inode XFS_ISTALE if the + * underlying cluster buffer is freed. Relogging an XFS_ISTALE inode + * here breaks the relationship between cluster buffer invalidation and + * stale inode invalidation on cluster buffer item journal commit + * completion, and can result in leaving dirty stale inodes hanging + * around in memory. + * + * We have no need for serialising this inode operation against other + * operations - we freed the inode and hence reallocation is required + * and that will serialise on reallocating the space the deferops need + * to free. Hence we can unlock the inode on the first commit of + * the transaction rather than roll it right through the deferops. This + * avoids relogging the XFS_ISTALE inode. + * + * We check that xfs_ifree() hasn't grown an internal transaction roll + * by asserting that the inode is still locked when it returns. + */ xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, 0); + xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
error = xfs_ifree(tp, ip); + ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); if (error) { /* * If we fail to free the inode, shut down. The cancel @@ -1788,7 +1809,6 @@ xfs_inactive_ifree( xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR); } xfs_trans_cancel(tp); - xfs_iunlock(ip, XFS_ILOCK_EXCL); return error; }
@@ -1806,7 +1826,6 @@ xfs_inactive_ifree( xfs_notice(mp, "%s: xfs_trans_commit returned error %d", __func__, error);
- xfs_iunlock(ip, XFS_ILOCK_EXCL); return 0; }
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c index 542927321a61b..ae453dd236a69 100644 --- a/fs/xfs/xfs_trans_inode.c +++ b/fs/xfs/xfs_trans_inode.c @@ -39,6 +39,7 @@ xfs_trans_ijoin(
ASSERT(iip->ili_lock_flags == 0); iip->ili_lock_flags = lock_flags; + ASSERT(!xfs_iflags_test(ip, XFS_ISTALE));
/* * Get a log_item_desc to point at the new item. @@ -90,6 +91,7 @@ xfs_trans_log_inode(
ASSERT(ip->i_itemp != NULL); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); + ASSERT(!xfs_iflags_test(ip, XFS_ISTALE));
/* * Don't bother with i_lock for the I_DIRTY_TIME check here, as races
From: "Desnes A. Nunes do Rosario" desnesn@linux.ibm.com
[ Upstream commit 3337bf41e0dd70b4064cdf60acdfcdc2d050066c ]
An extra count on ebb_state.stats.pmc_count[PMC_INDEX(pmc)] is being per- formed when count_pmc() is used to reset PMCs on a few selftests. This extra pmc_count can occasionally invalidate results, such as the ones from cycles_test shown hereafter. The ebb_check_count() failed with an above the upper limit error due to the extra value on ebb_state.stats.pmc_count.
Furthermore, this extra count is also indicated by extra PMC1 trace_log on the output of the cycle test (as well as on pmc56_overflow_test):
========== ... [21]: counter = 8 [22]: register SPRN_MMCR0 = 0x0000000080000080 [23]: register SPRN_PMC1 = 0x0000000080000004 [24]: counter = 9 [25]: register SPRN_MMCR0 = 0x0000000080000080 [26]: register SPRN_PMC1 = 0x0000000080000004 [27]: counter = 10 [28]: register SPRN_MMCR0 = 0x0000000080000080 [29]: register SPRN_PMC1 = 0x0000000080000004
[30]: register SPRN_PMC1 = 0x000000004000051e
PMC1 count (0x280000546) above upper limit 0x2800003e8 (+0x15e) [FAIL] Test FAILED on line 52 failure: cycles ==========
Signed-off-by: Desnes A. Nunes do Rosario desnesn@linux.ibm.com Tested-by: Sachin Sant sachinp@linux.vnet.ibm.com Signed-off-by: Michael Ellerman mpe@ellerman.id.au Link: https://lore.kernel.org/r/20200626164737.21943-1-desnesn@linux.ibm.com Signed-off-by: Sasha Levin sashal@kernel.org --- .../selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c | 2 -- tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c | 2 -- .../selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c | 2 -- .../selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c | 2 -- tools/testing/selftests/powerpc/pmu/ebb/ebb.c | 2 -- .../selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c | 2 -- .../selftests/powerpc/pmu/ebb/lost_exception_test.c | 1 - .../testing/selftests/powerpc/pmu/ebb/multi_counter_test.c | 7 ------- .../selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c | 2 -- .../testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c | 2 -- .../selftests/powerpc/pmu/ebb/pmc56_overflow_test.c | 2 -- 11 files changed, 26 deletions(-)
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c b/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c index 94110b1dcd3d8..031baa43646fb 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c @@ -91,8 +91,6 @@ int back_to_back_ebbs(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
event_close(&event); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c index 7c57a8d79535d..361e0be9df9ae 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c @@ -42,8 +42,6 @@ int cycles(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
event_close(&event); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c index ecf5ee3283a3e..fe7d0dc2a1a26 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c @@ -99,8 +99,6 @@ int cycles_with_freeze(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
printf("EBBs while frozen %d\n", ebbs_while_frozen); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c index c0faba520b35c..b9b30f974b5ea 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c @@ -71,8 +71,6 @@ int cycles_with_mmcr2(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
event_close(&event); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c index 46681fec549b8..2694ae161a84a 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c @@ -396,8 +396,6 @@ int ebb_child(union pipe read_pipe, union pipe write_pipe) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
event_close(&event); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c b/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c index a991d2ea8d0a1..174e4f4dae6c0 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c @@ -38,8 +38,6 @@ static int victim_child(union pipe read_pipe, union pipe write_pipe) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
FAIL_IF(ebb_state.stats.ebb_count == 0); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c b/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c index 2ed7ad33f7a3b..dddb95938304e 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c @@ -75,7 +75,6 @@ static int test_body(void) ebb_freeze_pmcs(); ebb_global_disable();
- count_pmc(4, sample_period); mtspr(SPRN_PMC4, 0xdead);
dump_summary_ebb_state(); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c b/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c index 6ff8c8ff27d66..035c02273cd49 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c @@ -70,13 +70,6 @@ int multi_counter(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - count_pmc(2, sample_period); - count_pmc(3, sample_period); - count_pmc(4, sample_period); - count_pmc(5, sample_period); - count_pmc(6, sample_period); - dump_ebb_state();
for (i = 0; i < 6; i++) diff --git a/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c b/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c index 037cb6154f360..3e9d4ac965c85 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c @@ -61,8 +61,6 @@ static int cycles_child(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_summary_ebb_state();
event_close(&event); diff --git a/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c b/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c index c5fa64790c22e..d90891fe96a32 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c @@ -82,8 +82,6 @@ static int test_body(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(1, sample_period); - dump_ebb_state();
if (mmcr0_mismatch) diff --git a/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c b/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c index 30e1ac62e8cb4..8ca92b9ee5b01 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c +++ b/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c @@ -76,8 +76,6 @@ int pmc56_overflow(void) ebb_global_disable(); ebb_freeze_pmcs();
- count_pmc(2, sample_period); - dump_ebb_state();
printf("PMC5/6 overflow %d\n", pmc56_overflowed);
From: Chao Yu yuchao0@huawei.com
[ Upstream commit 9627a7b31f3c4ff8bc8f3be3683983ffe6eaebe6 ]
- don't panic kernel if f2fs_get_node_page() fails in f2fs_recover_inline_data() or f2fs_recover_inline_xattr(); - return error number of f2fs_truncate_blocks() to f2fs_recover_inline_data()'s caller;
Signed-off-by: Chao Yu yuchao0@huawei.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- fs/f2fs/f2fs.h | 4 ++-- fs/f2fs/inline.c | 19 ++++++++++++------- fs/f2fs/node.c | 6 ++++-- fs/f2fs/recovery.c | 10 ++++++++-- 4 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 6b5b685af5990..53ffa6fe207a3 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -2921,7 +2921,7 @@ bool f2fs_alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid); void f2fs_alloc_nid_done(struct f2fs_sb_info *sbi, nid_t nid); void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid); int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink); -void f2fs_recover_inline_xattr(struct inode *inode, struct page *page); +int f2fs_recover_inline_xattr(struct inode *inode, struct page *page); int f2fs_recover_xattr_data(struct inode *inode, struct page *page); int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page); int f2fs_restore_node_summary(struct f2fs_sb_info *sbi, @@ -3314,7 +3314,7 @@ int f2fs_read_inline_data(struct inode *inode, struct page *page); int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page); int f2fs_convert_inline_inode(struct inode *inode); int f2fs_write_inline_data(struct inode *inode, struct page *page); -bool f2fs_recover_inline_data(struct inode *inode, struct page *npage); +int f2fs_recover_inline_data(struct inode *inode, struct page *npage); struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir, struct fscrypt_name *fname, struct page **res_page); int f2fs_make_empty_inline_dir(struct inode *inode, struct inode *parent, diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index c1ba29d10789d..2fabeb0bb28fd 100644 --- a/fs/f2fs/inline.c +++ b/fs/f2fs/inline.c @@ -256,7 +256,7 @@ int f2fs_write_inline_data(struct inode *inode, struct page *page) return 0; }
-bool f2fs_recover_inline_data(struct inode *inode, struct page *npage) +int f2fs_recover_inline_data(struct inode *inode, struct page *npage) { struct f2fs_sb_info *sbi = F2FS_I_SB(inode); struct f2fs_inode *ri = NULL; @@ -278,7 +278,8 @@ bool f2fs_recover_inline_data(struct inode *inode, struct page *npage) ri && (ri->i_inline & F2FS_INLINE_DATA)) { process_inline: ipage = f2fs_get_node_page(sbi, inode->i_ino); - f2fs_bug_on(sbi, IS_ERR(ipage)); + if (IS_ERR(ipage)) + return PTR_ERR(ipage);
f2fs_wait_on_page_writeback(ipage, NODE, true);
@@ -291,21 +292,25 @@ bool f2fs_recover_inline_data(struct inode *inode, struct page *npage)
set_page_dirty(ipage); f2fs_put_page(ipage, 1); - return true; + return 1; }
if (f2fs_has_inline_data(inode)) { ipage = f2fs_get_node_page(sbi, inode->i_ino); - f2fs_bug_on(sbi, IS_ERR(ipage)); + if (IS_ERR(ipage)) + return PTR_ERR(ipage); f2fs_truncate_inline_inode(inode, ipage, 0); clear_inode_flag(inode, FI_INLINE_DATA); f2fs_put_page(ipage, 1); } else if (ri && (ri->i_inline & F2FS_INLINE_DATA)) { - if (f2fs_truncate_blocks(inode, 0, false)) - return false; + int ret; + + ret = f2fs_truncate_blocks(inode, 0, false); + if (ret) + return ret; goto process_inline; } - return false; + return 0; }
struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir, diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index f0714c1258c79..2ff02541c53d5 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -2451,7 +2451,7 @@ int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink) return nr - nr_shrink; }
-void f2fs_recover_inline_xattr(struct inode *inode, struct page *page) +int f2fs_recover_inline_xattr(struct inode *inode, struct page *page) { void *src_addr, *dst_addr; size_t inline_size; @@ -2459,7 +2459,8 @@ void f2fs_recover_inline_xattr(struct inode *inode, struct page *page) struct f2fs_inode *ri;
ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); - f2fs_bug_on(F2FS_I_SB(inode), IS_ERR(ipage)); + if (IS_ERR(ipage)) + return PTR_ERR(ipage);
ri = F2FS_INODE(page); if (ri->i_inline & F2FS_INLINE_XATTR) { @@ -2478,6 +2479,7 @@ void f2fs_recover_inline_xattr(struct inode *inode, struct page *page) update_inode: f2fs_update_inode(inode, ipage); f2fs_put_page(ipage, 1); + return 0; }
int f2fs_recover_xattr_data(struct inode *inode, struct page *page) diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c index 733f005b85d65..ad0486beee2c0 100644 --- a/fs/f2fs/recovery.c +++ b/fs/f2fs/recovery.c @@ -471,7 +471,9 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
/* step 1: recover xattr */ if (IS_INODE(page)) { - f2fs_recover_inline_xattr(inode, page); + err = f2fs_recover_inline_xattr(inode, page); + if (err) + goto out; } else if (f2fs_has_xattr_block(ofs_of_node(page))) { err = f2fs_recover_xattr_data(inode, page); if (!err) @@ -480,8 +482,12 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode, }
/* step 2: recover inline data */ - if (f2fs_recover_inline_data(inode, page)) + err = f2fs_recover_inline_data(inode, page); + if (err) { + if (err == 1) + err = 0; goto out; + }
/* step 3: recover data indices */ start = f2fs_start_bidx_of_node(ofs_of_node(page), inode);
From: Aditya Pakki pakki001@umn.edu
[ Upstream commit 78c2ce9bde70be5be7e3615a2ae7024ed8173087 ]
On calling pm_runtime_get_sync() the reference count of the device is incremented. In case of failure, decrement the reference count before returning the error.
Signed-off-by: Aditya Pakki pakki001@umn.edu Cc: kjlu@umn.edu Cc: wu000273@umn.edu Cc: Allison Randal allison@lohutok.net Cc: Thomas Gleixner tglx@linutronix.de Cc: Enrico Weigelt info@metux.net cc: "Andrew F. Davis" afd@ti.com Cc: Tomi Valkeinen tomi.valkeinen@ti.com Cc: Alexios Zavras alexios.zavras@intel.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: YueHaibing yuehaibing@huawei.com Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Link: https://patchwork.freedesktop.org/patch/msgid/20200614030528.128064-1-pakki0... Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 7 +++++-- drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 7 +++++-- drivers/video/fbdev/omap2/omapfb/dss/dss.c | 7 +++++-- drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 5 +++-- drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c | 5 +++-- drivers/video/fbdev/omap2/omapfb/dss/venc.c | 7 +++++-- 6 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c index a06d9c25765c5..0bd582e845f31 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c @@ -531,8 +531,11 @@ int dispc_runtime_get(void) DSSDBG("dispc_runtime_get\n");
r = pm_runtime_get_sync(&dispc.pdev->dev); - WARN_ON(r < 0); - return r < 0 ? r : 0; + if (WARN_ON(r < 0)) { + pm_runtime_put_sync(&dispc.pdev->dev); + return r; + } + return 0; } EXPORT_SYMBOL(dispc_runtime_get);
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c index 8e1d60d48dbb0..50792d31533bf 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c @@ -1148,8 +1148,11 @@ static int dsi_runtime_get(struct platform_device *dsidev) DSSDBG("dsi_runtime_get\n");
r = pm_runtime_get_sync(&dsi->pdev->dev); - WARN_ON(r < 0); - return r < 0 ? r : 0; + if (WARN_ON(r < 0)) { + pm_runtime_put_sync(&dsi->pdev->dev); + return r; + } + return 0; }
static void dsi_runtime_put(struct platform_device *dsidev) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c index b6c6c24979dd6..faebf9a773ba5 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c @@ -779,8 +779,11 @@ int dss_runtime_get(void) DSSDBG("dss_runtime_get\n");
r = pm_runtime_get_sync(&dss.pdev->dev); - WARN_ON(r < 0); - return r < 0 ? r : 0; + if (WARN_ON(r < 0)) { + pm_runtime_put_sync(&dss.pdev->dev); + return r; + } + return 0; }
void dss_runtime_put(void) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c index 28de56e21c74b..9fd9a02bb871d 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c @@ -50,9 +50,10 @@ static int hdmi_runtime_get(void) DSSDBG("hdmi_runtime_get\n");
r = pm_runtime_get_sync(&hdmi.pdev->dev); - WARN_ON(r < 0); - if (r < 0) + if (WARN_ON(r < 0)) { + pm_runtime_put_sync(&hdmi.pdev->dev); return r; + }
return 0; } diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c index 2e2fcc3d6d4f7..13f3a5ce55294 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c @@ -54,9 +54,10 @@ static int hdmi_runtime_get(void) DSSDBG("hdmi_runtime_get\n");
r = pm_runtime_get_sync(&hdmi.pdev->dev); - WARN_ON(r < 0); - if (r < 0) + if (WARN_ON(r < 0)) { + pm_runtime_put_sync(&hdmi.pdev->dev); return r; + }
return 0; } diff --git a/drivers/video/fbdev/omap2/omapfb/dss/venc.c b/drivers/video/fbdev/omap2/omapfb/dss/venc.c index 392464da12e41..96714b4596d2d 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/venc.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/venc.c @@ -402,8 +402,11 @@ static int venc_runtime_get(void) DSSDBG("venc_runtime_get\n");
r = pm_runtime_get_sync(&venc.pdev->dev); - WARN_ON(r < 0); - return r < 0 ? r : 0; + if (WARN_ON(r < 0)) { + pm_runtime_put_sync(&venc.pdev->dev); + return r; + } + return 0; }
static void venc_runtime_put(void)
From: Qiushi Wu wu000273@umn.edu
[ Upstream commit 8a94644b440eef5a7b9c104ac8aa7a7f413e35e5 ]
kobject_init_and_add() takes a reference even when it fails. If it returns an error, kobject_put() must be called to clean up the memory associated with the object.
When kobject_init_and_add() fails, call kobject_put() instead of kfree().
b8eb718348b8 ("net-sysfs: Fix reference count leak in rx|netdev_queue_add_kobject") fixed a similar problem.
Link: https://lore.kernel.org/r/20200528021322.1984-1-wu000273@umn.edu Signed-off-by: Qiushi Wu wu000273@umn.edu Signed-off-by: Bjorn Helgaas bhelgaas@google.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/pci/slot.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index a32897f83ee51..fb7478b6c4f9d 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -303,13 +303,16 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, slot_name = make_slot_name(name); if (!slot_name) { err = -ENOMEM; + kfree(slot); goto err; }
err = kobject_init_and_add(&slot->kobj, &pci_slot_ktype, NULL, "%s", slot_name); - if (err) + if (err) { + kobject_put(&slot->kobj); goto err; + }
INIT_LIST_HEAD(&slot->list); list_add(&slot->list, &parent->slots); @@ -328,7 +331,6 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, mutex_unlock(&pci_slot_mutex); return slot; err: - kfree(slot); slot = ERR_PTR(err); goto out; }
From: Yangbo Lu yangbo.lu@nxp.com
[ Upstream commit 5656bb3857c4904d1dec6e1b8f876c1c0337274e ]
The timer fixed interval period pulse generator register is used to generate periodic pulses. The down count register loads the value programmed in the fixed period interval (FIPER). At every tick of the timer accumulator overflow, the counter decrements by the value of TMR_CTRL[TCLK_PERIOD]. It generates a pulse when the down counter value reaches zero. It reloads the down counter in the cycle following a pulse.
To use the TMR_FIPER register to generate desired periodic pulses. The value should programmed is, desired_period - tclk_period
Current tmr-fiper2 value is to generate 100us periodic pulses. (But the value should have been 99995, not 99990. The tclk_period is 5.) This patch is to generate 1 second periodic pulses with value 999999995 programmed which is more desired by user.
Signed-off-by: Yangbo Lu yangbo.lu@nxp.com Acked-by: Richard Cochran richardcochran@gmail.com Signed-off-by: Shawn Guo shawnguo@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- arch/arm/boot/dts/ls1021a.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi index 074b4ec520c63..d18c043264440 100644 --- a/arch/arm/boot/dts/ls1021a.dtsi +++ b/arch/arm/boot/dts/ls1021a.dtsi @@ -609,7 +609,7 @@ ptp_clock@2d10e00 { fsl,tmr-prsc = <2>; fsl,tmr-add = <0xaaaaaaab>; fsl,tmr-fiper1 = <999999995>; - fsl,tmr-fiper2 = <99990>; + fsl,tmr-fiper2 = <999999995>; fsl,max-adj = <499999999>; };
From: Reto Schneider code@reto-schneider.ch
[ Upstream commit 03128643eb5453a798db5770952c73dc64fcaf00 ]
If usb_submit_urb fails the allocated urb should be unanchored and released.
Signed-off-by: Reto Schneider code@reto-schneider.ch Signed-off-by: Kalle Valo kvalo@codeaurora.org Link: https://lore.kernel.org/r/20200622132113.14508-3-code@reto-schneider.ch Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/net/wireless/realtek/rtlwifi/usb.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c index 1893640555c1e..3d6c0d8c71d7e 100644 --- a/drivers/net/wireless/realtek/rtlwifi/usb.c +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c @@ -739,8 +739,11 @@ static int _rtl_usb_receive(struct ieee80211_hw *hw)
usb_anchor_urb(urb, &rtlusb->rx_submitted); err = usb_submit_urb(urb, GFP_KERNEL); - if (err) + if (err) { + usb_unanchor_urb(urb); + usb_free_urb(urb); goto err_out; + } usb_free_urb(urb); } return 0;
From: Peng Fan fanpeng@loongson.cn
[ Upstream commit a859647b4e6bfeb192284d27d24b6a0c914cae1d ]
Close "fd" before the return of map_vdso() and close "out_file" in main().
Signed-off-by: Peng Fan fanpeng@loongson.cn Signed-off-by: Thomas Bogendoerfer tsbogend@alpha.franken.de Signed-off-by: Sasha Levin sashal@kernel.org --- arch/mips/vdso/genvdso.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/arch/mips/vdso/genvdso.c b/arch/mips/vdso/genvdso.c index 530a36f465ced..afcc86726448e 100644 --- a/arch/mips/vdso/genvdso.c +++ b/arch/mips/vdso/genvdso.c @@ -126,6 +126,7 @@ static void *map_vdso(const char *path, size_t *_size) if (fstat(fd, &stat) != 0) { fprintf(stderr, "%s: Failed to stat '%s': %s\n", program_name, path, strerror(errno)); + close(fd); return NULL; }
@@ -134,6 +135,7 @@ static void *map_vdso(const char *path, size_t *_size) if (addr == MAP_FAILED) { fprintf(stderr, "%s: Failed to map '%s': %s\n", program_name, path, strerror(errno)); + close(fd); return NULL; }
@@ -143,6 +145,7 @@ static void *map_vdso(const char *path, size_t *_size) if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG) != 0) { fprintf(stderr, "%s: '%s' is not an ELF file\n", program_name, path); + close(fd); return NULL; }
@@ -154,6 +157,7 @@ static void *map_vdso(const char *path, size_t *_size) default: fprintf(stderr, "%s: '%s' has invalid ELF class\n", program_name, path); + close(fd); return NULL; }
@@ -165,6 +169,7 @@ static void *map_vdso(const char *path, size_t *_size) default: fprintf(stderr, "%s: '%s' has invalid ELF data order\n", program_name, path); + close(fd); return NULL; }
@@ -172,15 +177,18 @@ static void *map_vdso(const char *path, size_t *_size) fprintf(stderr, "%s: '%s' has invalid ELF machine (expected EM_MIPS)\n", program_name, path); + close(fd); return NULL; } else if (swap_uint16(ehdr->e_type) != ET_DYN) { fprintf(stderr, "%s: '%s' has invalid ELF type (expected ET_DYN)\n", program_name, path); + close(fd); return NULL; }
*_size = stat.st_size; + close(fd); return addr; }
@@ -284,10 +292,12 @@ int main(int argc, char **argv) /* Calculate and write symbol offsets to <output file> */ if (!get_symbols(dbg_vdso_path, dbg_vdso)) { unlink(out_path); + fclose(out_file); return EXIT_FAILURE; }
fprintf(out_file, "};\n"); + fclose(out_file);
return EXIT_SUCCESS; }
From: Hans Verkuil hverkuil-cisco@xs4all.nl
[ Upstream commit 6c42227c3467549ddc65efe99c869021d2f4a570 ]
Fix this smatch warning:
drivers/media/cec/core/cec-api.c:156 cec_adap_g_log_addrs() warn: check that 'log_addrs' doesn't leak information (struct has a hole after 'features')
Signed-off-by: Hans Verkuil hverkuil-cisco@xs4all.nl Signed-off-by: Mauro Carvalho Chehab mchehab+huawei@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/media/cec/cec-api.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c index 4961573850d54..b2b3f779592fd 100644 --- a/drivers/media/cec/cec-api.c +++ b/drivers/media/cec/cec-api.c @@ -147,7 +147,13 @@ static long cec_adap_g_log_addrs(struct cec_adapter *adap, struct cec_log_addrs log_addrs;
mutex_lock(&adap->lock); - log_addrs = adap->log_addrs; + /* + * We use memcpy here instead of assignment since there is a + * hole at the end of struct cec_log_addrs that an assignment + * might ignore. So when we do copy_to_user() we could leak + * one byte of memory. + */ + memcpy(&log_addrs, &adap->log_addrs, sizeof(log_addrs)); if (!adap->is_configured) memset(log_addrs.log_addr, CEC_LOG_ADDR_INVALID, sizeof(log_addrs.log_addr));
From: Ikjoon Jang ikjn@chromium.org
[ Upstream commit 68f775ddd2a6f513e225f9a565b054ab48fef142 ]
Add HID_QUIRK_NOGET for Logitech GROUP device.
Logitech GROUP is a compound with camera and audio. When the HID interface in an audio device is requested to get specific report id, all following control transfers are stalled and never be restored back.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=203419 Signed-off-by: Ikjoon Jang ikjn@chromium.org Signed-off-by: Jiri Kosina jkosina@suse.cz Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-quirks.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 20530d8adfbb8..2c100b73d3fc1 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -756,6 +756,7 @@ #define USB_DEVICE_ID_LOGITECH_G27_WHEEL 0xc29b #define USB_DEVICE_ID_LOGITECH_WII_WHEEL 0xc29c #define USB_DEVICE_ID_LOGITECH_ELITE_KBD 0xc30a +#define USB_DEVICE_ID_LOGITECH_GROUP_AUDIO 0x0882 #define USB_DEVICE_ID_S510_RECEIVER 0xc50c #define USB_DEVICE_ID_S510_RECEIVER_2 0xc517 #define USB_DEVICE_ID_LOGITECH_CORDLESS_DESKTOP_LX500 0xc512 diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index bdde16395b2ce..62f87f8bd9720 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -179,6 +179,7 @@ static const struct hid_device_id hid_quirks[] = { { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD2, USB_DEVICE_ID_SMARTJOY_DUAL_PLUS), HID_QUIRK_NOGET | HID_QUIRK_MULTI_INPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP, USB_DEVICE_ID_QUAD_USB_JOYPAD), HID_QUIRK_NOGET | HID_QUIRK_MULTI_INPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_XIN_MO, USB_DEVICE_ID_XIN_MO_DUAL_ARCADE), HID_QUIRK_MULTI_INPUT }, + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_GROUP_AUDIO), HID_QUIRK_NOGET },
{ 0 } };
From: Li Guifu bluce.liguifu@huawei.com
[ Upstream commit 99c787cfd2bd04926f1f553b30bd7dcea2caaba1 ]
During umount, f2fs_put_super() unregisters procfs entries after f2fs_destroy_segment_manager(), it may cause use-after-free issue when umount races with procfs accessing, fix it by relocating f2fs_unregister_sysfs().
[Chao Yu: change commit title/message a bit]
Signed-off-by: Li Guifu bluce.liguifu@huawei.com Reviewed-by: Chao Yu yuchao0@huawei.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- fs/f2fs/super.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 9782250c98156..161ce0eb8891a 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1004,6 +1004,9 @@ static void f2fs_put_super(struct super_block *sb) int i; bool dropped;
+ /* unregister procfs/sysfs entries in advance to avoid race case */ + f2fs_unregister_sysfs(sbi); + f2fs_quota_off_umount(sb);
/* prevent remaining shrinker jobs */ @@ -1067,8 +1070,6 @@ static void f2fs_put_super(struct super_block *sb)
kfree(sbi->ckpt);
- f2fs_unregister_sysfs(sbi); - sb->s_fs_info = NULL; if (sbi->s_chksum_driver) crypto_free_shash(sbi->s_chksum_driver);
From: Aditya Pakki pakki001@umn.edu
[ Upstream commit bfad51c7633325b5d4b32444efe04329d53297b2 ]
nouveau_fbcon_open() calls calls pm_runtime_get_sync() that increments the reference count. In case of failure, decrement the ref count before returning the error.
Signed-off-by: Aditya Pakki pakki001@umn.edu Signed-off-by: Ben Skeggs bskeggs@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 406cb99af7f21..d4fe52ec4c966 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -189,8 +189,10 @@ nouveau_fbcon_open(struct fb_info *info, int user) struct nouveau_fbdev *fbcon = info->par; struct nouveau_drm *drm = nouveau_drm(fbcon->helper.dev); int ret = pm_runtime_get_sync(drm->dev->dev); - if (ret < 0 && ret != -EACCES) + if (ret < 0 && ret != -EACCES) { + pm_runtime_put(drm->dev->dev); return ret; + } return 0; }
From: Aditya Pakki pakki001@umn.edu
[ Upstream commit a2cdf39536b0d21fb06113f5e16692513d7bcb9c ]
nv50_disp_atomic_commit() calls calls pm_runtime_get_sync and in turn increments the reference count. In case of failure, decrement the ref count before returning the error.
Signed-off-by: Aditya Pakki pakki001@umn.edu Signed-off-by: Ben Skeggs bskeggs@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 10107e551fac3..e06ea8c8184cb 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1920,8 +1920,10 @@ nv50_disp_atomic_commit(struct drm_device *dev, int ret, i;
ret = pm_runtime_get_sync(dev->dev); - if (ret < 0 && ret != -EACCES) + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_autosuspend(dev->dev); return ret; + }
ret = drm_atomic_helper_setup_commit(state, nonblock); if (ret)
From: Aditya Pakki pakki001@umn.edu
[ Upstream commit 990a1162986e8eff7ca18cc5a0e03b4304392ae2 ]
nouveau_connector_detect() calls pm_runtime_get_sync and in turn increments the reference count. In case of failure, decrement the ref count before returning the error.
Signed-off-by: Aditya Pakki pakki001@umn.edu Signed-off-by: Ben Skeggs bskeggs@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/nouveau/nouveau_connector.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index fb0094fc55834..b71afde8f115a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -551,8 +551,10 @@ nouveau_connector_detect(struct drm_connector *connector, bool force) pm_runtime_get_noresume(dev->dev); } else { ret = pm_runtime_get_sync(dev->dev); - if (ret < 0 && ret != -EACCES) + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_autosuspend(dev->dev); return conn_status; + } }
nv_encoder = nouveau_connector_ddc_detect(connector);
From: Chris Wilson chris@chris-wilson.co.uk
[ Upstream commit a7ef9b28aa8d72a1656fa6f0a01bbd1493886317 ]
Though the number of lock-acquisitions is tracked as unsigned long, this is passed as the divisor to div_s64() which interprets it as a s32, giving nonsense values with more than 2 billion acquisitons. E.g.
acquisitions holdtime-min holdtime-max holdtime-total holdtime-avg ------------------------------------------------------------------------- 2350439395 0.07 353.38 649647067.36 0.-32
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Ingo Molnar mingo@kernel.org Cc: Peter Zijlstra peterz@infradead.org Link: https://lore.kernel.org/r/20200725185110.11588-1-chris@chris-wilson.co.uk Signed-off-by: Sasha Levin sashal@kernel.org --- kernel/locking/lockdep_proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c index 6fcc4650f0c48..53cc3bb7025a5 100644 --- a/kernel/locking/lockdep_proc.c +++ b/kernel/locking/lockdep_proc.c @@ -394,7 +394,7 @@ static void seq_lock_time(struct seq_file *m, struct lock_time *lt) seq_time(m, lt->min); seq_time(m, lt->max); seq_time(m, lt->total); - seq_time(m, lt->nr ? div_s64(lt->total, lt->nr) : 0); + seq_time(m, lt->nr ? div64_u64(lt->total, lt->nr) : 0); }
static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
From: Qu Wenruo wqu@suse.com
[ Upstream commit a7f8b1c2ac21bf081b41264c9cfd6260dffa6246 ]
The incoming qgroup reserved space timing will move the data reservation to ordered extent completely.
However in btrfs_punch_hole_lock_range() will call btrfs_invalidate_page(), which will clear QGROUP_RESERVED bit for the range.
In current stage it's OK, but if we're making ordered extents handle the reserved space, then btrfs_punch_hole_lock_range() can clear the QGROUP_RESERVED bit before we submit ordered extent, leading to qgroup reserved space leakage.
So here change the timing to make reserve data space after btrfs_punch_hole_lock_range(). The new timing is fine for either current code or the new code.
Reviewed-by: Josef Bacik josef@toxicpanda.com Signed-off-by: Qu Wenruo wqu@suse.com Reviewed-by: David Sterba dsterba@suse.com Signed-off-by: David Sterba dsterba@suse.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/btrfs/file.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index dc1841855a69a..646152f305843 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -3010,14 +3010,14 @@ static int btrfs_zero_range(struct inode *inode, if (ret < 0) goto out; space_reserved = true; - ret = btrfs_qgroup_reserve_data(inode, &data_reserved, - alloc_start, bytes_to_reserve); - if (ret) - goto out; ret = btrfs_punch_hole_lock_range(inode, lockstart, lockend, &cached_state); if (ret) goto out; + ret = btrfs_qgroup_reserve_data(inode, &data_reserved, + alloc_start, bytes_to_reserve); + if (ret) + goto out; ret = btrfs_prealloc_file_range(inode, mode, alloc_start, alloc_end - alloc_start, i_blocksize(inode),
Hi!
[ Upstream commit a7f8b1c2ac21bf081b41264c9cfd6260dffa6246 ]
The incoming qgroup reserved space timing will move the data reservation to ordered extent completely.
However in btrfs_punch_hole_lock_range() will call btrfs_invalidate_page(), which will clear QGROUP_RESERVED bit for the range.
In current stage it's OK, but if we're making ordered extents handle the reserved space, then btrfs_punch_hole_lock_range() can clear the QGROUP_RESERVED bit before we submit ordered extent, leading to qgroup reserved space leakage.
So here change the timing to make reserve data space after btrfs_punch_hole_lock_range(). The new timing is fine for either current code or the new code.
I'm not sure why this is queued for -stable. It is preparation for future work, and that work is not queued for -stable.
Best regards, Pavel
On Sat, Aug 29, 2020 at 02:11:23PM +0200, Pavel Machek wrote:
Hi!
[ Upstream commit a7f8b1c2ac21bf081b41264c9cfd6260dffa6246 ]
The incoming qgroup reserved space timing will move the data reservation to ordered extent completely.
However in btrfs_punch_hole_lock_range() will call btrfs_invalidate_page(), which will clear QGROUP_RESERVED bit for the range.
In current stage it's OK, but if we're making ordered extents handle the reserved space, then btrfs_punch_hole_lock_range() can clear the QGROUP_RESERVED bit before we submit ordered extent, leading to qgroup reserved space leakage.
So here change the timing to make reserve data space after btrfs_punch_hole_lock_range(). The new timing is fine for either current code or the new code.
I'm not sure why this is queued for -stable. It is preparation for future work, and that work is not queued for -stable.
So you understand why it was queued: it's preparation for a fix that is relevant to 4.19 but didn't apply cleanly.
I can look into what happened next week, or if you'd sent me a backport I'd be happy to take it.
From: Jing Xiangfeng jingxiangfeng@huawei.com
[ Upstream commit 68e12e5f61354eb42cfffbc20a693153fc39738e ]
If scsi_host_lookup() fails we will jump to put_host which may cause a panic. Jump to exit_set_fnode instead.
Link: https://lore.kernel.org/r/20200615081226.183068-1-jingxiangfeng@huawei.com Reviewed-by: Mike Christie michael.christie@oracle.com Signed-off-by: Jing Xiangfeng jingxiangfeng@huawei.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/scsi/scsi_transport_iscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 04d095488c764..6983473011980 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -3172,7 +3172,7 @@ static int iscsi_set_flashnode_param(struct iscsi_transport *transport, pr_err("%s could not find host no %u\n", __func__, ev->u.set_flashnode.host_no); err = -ENODEV; - goto put_host; + goto exit_set_fnode; }
idx = ev->u.set_flashnode.flashnode_idx;
From: Xiubo Li xiubli@redhat.com
[ Upstream commit fa9967734227b44acb1b6918033f9122dc7825b9 ]
Make sure the delayed work stopped before releasing the resources.
cancel_delayed_work_sync() will only guarantee that the work finishes executing if the work is already in the ->worklist. That means after the cancel_delayed_work_sync() returns, it will leave the work requeued if it was rearmed at the end. That can lead to a use after free once the work struct is freed.
Fix it by flushing the delayed work instead of trying to cancel it, and ensure that the work doesn't rearm if the mdsc is stopping.
URL: https://tracker.ceph.com/issues/46293 Signed-off-by: Xiubo Li xiubli@redhat.com Reviewed-by: Jeff Layton jlayton@kernel.org Signed-off-by: Ilya Dryomov idryomov@gmail.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/ceph/mds_client.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index a2e903203bf9f..ff2022b07d1de 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -3615,6 +3615,9 @@ static void delayed_work(struct work_struct *work) dout("mdsc delayed_work\n"); ceph_check_delayed_caps(mdsc);
+ if (mdsc->stopping) + return; + mutex_lock(&mdsc->mutex); renew_interval = mdsc->mdsmap->m_session_timeout >> 2; renew_caps = time_after_eq(jiffies, HZ*renew_interval + @@ -3949,7 +3952,16 @@ void ceph_mdsc_force_umount(struct ceph_mds_client *mdsc) static void ceph_mdsc_stop(struct ceph_mds_client *mdsc) { dout("stop\n"); - cancel_delayed_work_sync(&mdsc->delayed_work); /* cancel timer */ + /* + * Make sure the delayed work stopped before releasing + * the resources. + * + * Because the cancel_delayed_work_sync() will only + * guarantee that the work finishes executing. But the + * delayed work will re-arm itself again after that. + */ + flush_delayed_work(&mdsc->delayed_work); + if (mdsc->mdsmap) ceph_mdsmap_destroy(mdsc->mdsmap); kfree(mdsc->sessions);
From: Javed Hasan jhasan@marvell.com
[ Upstream commit e95b4789ff4380733006836d28e554dc296b2298 ]
In fcoe_sysfs_fcf_del(), we first deleted the fcf from the list and then freed it if ctlr_dev was not NULL. This was causing a memory leak.
Free the fcf even if ctlr_dev is NULL.
Link: https://lore.kernel.org/r/20200729081824.30996-3-jhasan@marvell.com Reviewed-by: Girish Basrur gbasrur@marvell.com Reviewed-by: Santosh Vernekar svernekar@marvell.com Reviewed-by: Saurav Kashyap skashyap@marvell.com Reviewed-by: Shyam Sundar ssundar@marvell.com Signed-off-by: Javed Hasan jhasan@marvell.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/scsi/fcoe/fcoe_ctlr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c index 24cbd0a2cc69f..658c0726581f9 100644 --- a/drivers/scsi/fcoe/fcoe_ctlr.c +++ b/drivers/scsi/fcoe/fcoe_ctlr.c @@ -267,9 +267,9 @@ static void fcoe_sysfs_fcf_del(struct fcoe_fcf *new) WARN_ON(!fcf_dev); new->fcf_dev = NULL; fcoe_fcf_device_delete(fcf_dev); - kfree(new); mutex_unlock(&cdev->lock); } + kfree(new); }
/**
From: Jason Baron jbaron@akamai.com
[ Upstream commit 709ed1bcef12398ac1a35c149f3e582db04456c2 ]
The Intel uncore driver may claim some of the pci ids from ie31200 which means that the ie31200 edac driver will not initialize them as part of pci_register_driver().
Let's add a fallback for this case to 'pci_get_device()' to get a reference on the device such that it can still be configured. This is similar in approach to other edac drivers.
Signed-off-by: Jason Baron jbaron@akamai.com Cc: Borislav Petkov bp@suse.de Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: linux-edac linux-edac@vger.kernel.org Signed-off-by: Tony Luck tony.luck@intel.com Link: https://lore.kernel.org/r/1594923911-10885-1-git-send-email-jbaron@akamai.co... Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/edac/ie31200_edac.c | 50 ++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c index aac9b9b360b80..9e4781a807cfa 100644 --- a/drivers/edac/ie31200_edac.c +++ b/drivers/edac/ie31200_edac.c @@ -147,6 +147,8 @@ (n << (28 + (2 * skl) - PAGE_SHIFT))
static int nr_channels; +static struct pci_dev *mci_pdev; +static int ie31200_registered = 1;
struct ie31200_priv { void __iomem *window; @@ -518,12 +520,16 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx) static int ie31200_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) { - edac_dbg(0, "MC:\n"); + int rc;
+ edac_dbg(0, "MC:\n"); if (pci_enable_device(pdev) < 0) return -EIO; + rc = ie31200_probe1(pdev, ent->driver_data); + if (rc == 0 && !mci_pdev) + mci_pdev = pci_dev_get(pdev);
- return ie31200_probe1(pdev, ent->driver_data); + return rc; }
static void ie31200_remove_one(struct pci_dev *pdev) @@ -532,6 +538,8 @@ static void ie31200_remove_one(struct pci_dev *pdev) struct ie31200_priv *priv;
edac_dbg(0, "\n"); + pci_dev_put(mci_pdev); + mci_pdev = NULL; mci = edac_mc_del_mc(&pdev->dev); if (!mci) return; @@ -583,17 +591,53 @@ static struct pci_driver ie31200_driver = {
static int __init ie31200_init(void) { + int pci_rc, i; + edac_dbg(3, "MC:\n"); /* Ensure that the OPSTATE is set correctly for POLL or NMI */ opstate_init();
- return pci_register_driver(&ie31200_driver); + pci_rc = pci_register_driver(&ie31200_driver); + if (pci_rc < 0) + goto fail0; + + if (!mci_pdev) { + ie31200_registered = 0; + for (i = 0; ie31200_pci_tbl[i].vendor != 0; i++) { + mci_pdev = pci_get_device(ie31200_pci_tbl[i].vendor, + ie31200_pci_tbl[i].device, + NULL); + if (mci_pdev) + break; + } + if (!mci_pdev) { + edac_dbg(0, "ie31200 pci_get_device fail\n"); + pci_rc = -ENODEV; + goto fail1; + } + pci_rc = ie31200_init_one(mci_pdev, &ie31200_pci_tbl[i]); + if (pci_rc < 0) { + edac_dbg(0, "ie31200 init fail\n"); + pci_rc = -ENODEV; + goto fail1; + } + } + return 0; + +fail1: + pci_unregister_driver(&ie31200_driver); +fail0: + pci_dev_put(mci_pdev); + + return pci_rc; }
static void __exit ie31200_exit(void) { edac_dbg(3, "MC:\n"); pci_unregister_driver(&ie31200_driver); + if (!ie31200_registered) + ie31200_remove_one(mci_pdev); }
module_init(ie31200_init);
linux-stable-mirror@lists.linaro.org
-
Jia-Ju Bai -
Laurent Pinchart -
Pavel Machek -
Sasha Levin -
Sean Young