Re: USB Attached SCSI breakage due no udev involvement
On Sun, May 10, 2020 at 09:55:57AM +0700, Dio Putra wrote: > Hi, it's first time for me to report user-space breakage in here, so > i'm begging your pardon. > > I want to report that Linux 5.4 breaking my USB mount workflow due > udevadm monitor report here (I'm using vanilla kernel 5.4.39 on > Slackware64 Current and vanilla kernel 4.4.221 on Slackware64 14.2): Sorry, but what actually changed that you can see in the logs? What functionality broke? What used to work that no longer does work? And 4.4.221 is quite different from 5.4, is that the jump that you are seeing breakage in, or is it in some smaller jump? thanks, greg k-h
[rcu:dev.2020.05.07a] BUILD SUCCESS 1af6526eb52eb1fb840f5941e1b8691d49e03674
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git dev.2020.05.07a branch HEAD: 1af6526eb52eb1fb840f5941e1b8691d49e03674 locking/osq_lock: Annotate a data race in osq_lock i386-tinyconfig vmlinux size: == TOTAL TEXT arch/x86/events/zhaoxin/built-in.* try_invoke_on_locked_down_task() == +1 0 353159365e72 rcu: Add KCSAN stubs 0 0 4f58820fd710 srcu: Add KCSAN stubs 0 0 2f0846956355 rcu: Mark rcu_state.ncpus to detect concurrent writes 0 0 314eeb43e5f2 rcu: Add *_ONCE() and data_race() to rcu_node ->exp_tasks pl 0 0 065a6db12a80 rcu: Add READ_ONCE and data_race() to rcu_node ->boost_tasks 0 0 b68c6146512d srcu: Add data_race() to ->srcu_lock_count and ->srcu_unlock 0 0 5822b8126ff0 rcu: Add WRITE_ONCE() to rcu_node ->boost_tasks 0 0 47fbb074536e rcu: Use data_race() for RCU CPU stall-warning prints 0 0 53965dbe5396 drm: Make drm_dp_mst_dsc_aux_for_port() safe for old compile 0 0 1fca4d12f463 rcu: Expedite first two FQS scans under callback-overload co 0 0 fcbcc0e70050 rcu: Fix the (t=0 jiffies) false positive 0 0 ddc465936643 Revert "rculist: Describe variadic macro argument in a Sphin 0 0 c28d5c09d09f rcu: Get rid of some doc warnings in update.c 0 0 62ae19511f1e rcu: Mark rcu_state.gp_seq to detect more concurrent writes 0 0 a66dbda7893f rcu: Replace assigned pointer ret value by corresponding boo 0 0 da44cd6c8e88 rcu: Replace 1 by true 0 0 29ffebc5fcc0 rcu: Convert ULONG_CMP_GE() to time_after() for jiffy compar 0 0 7b2413111a63 rcu: Convert rcu_initiate_boost() ULONG_CMP_GE() to time_aft 0 0 e2f3ccfa6200 rcu: Convert rcu_nohz_full_cpu() ULONG_CMP_LT() to time_befo +83 +84+136 +83 f736e0f1a55a Merge branches 'fixes.2020.04.27a', 'kfree_rcu.2020.04.27a', -224 -224 0 0 2d9d829af55c Merge branch 'kcsan-dev.2020.04.13c' into HEAD 0 0 0 0 99a5d03ba959 Merge branch 'lkmm-dev.2020.04.15b' into HEAD +38 +38 0 0 3123dcd3ef7f fork: Annotate a data race in vm_area_dup() 0 0 0 0 2ba74f25e0de x86/mm/pat: Mark an intentional data race 0 0 0 0 cd59625dedde rculist: Add ASSERT_EXCLUSIVE_ACCESS() to __list_splice_init 0 0 0 0 ca6e49239a17 locktorture: Use true and false to assign to bool variables 0 0 0 0 8c8786cd0247 srcu: Fix a typo in
Re: [PATCH net-next 4/4] net: phy: bcm54140: add cable diagnostics support
Hi Michael,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on next-20200508]
[cannot apply to net/master linus/master ipvs/master v5.7-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Michael-Walle/net-phy-broadcom-cable-tester-support/20200510-063955
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
2c674bec76d35b75c7c730f863424387c9e9633a
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
>> drivers/net/phy/bcm54140.c:834:13: error: 'PHY_POLL_CABLE_TEST' undeclared
>> here (not in a function)
.flags = PHY_POLL_CABLE_TEST,
^~~
>> drivers/net/phy/bcm54140.c:846:4: error: 'struct phy_driver' has no member
>> named 'cable_test_start'
.cable_test_start = bcm_phy_cable_test_start_rdb,
^~~~
>> drivers/net/phy/bcm54140.c:846:23: error: initialization from incompatible
>> pointer type [-Werror=incompatible-pointer-types]
.cable_test_start = bcm_phy_cable_test_start_rdb,
^~~~
drivers/net/phy/bcm54140.c:846:23: note: (near initialization for
'bcm54140_drivers[0].set_loopback')
>> drivers/net/phy/bcm54140.c:847:4: error: 'struct phy_driver' has no member
>> named 'cable_test_get_status'
.cable_test_get_status = bcm_phy_cable_test_get_status_rdb,
^
>> drivers/net/phy/bcm54140.c:847:28: warning: excess elements in struct
>> initializer
.cable_test_get_status = bcm_phy_cable_test_get_status_rdb,
^
drivers/net/phy/bcm54140.c:847:28: note: (near initialization for
'bcm54140_drivers[0]')
cc1: some warnings being treated as errors
vim +/PHY_POLL_CABLE_TEST +834 drivers/net/phy/bcm54140.c
828
829 static struct phy_driver bcm54140_drivers[] = {
830 {
831 .phy_id = PHY_ID_BCM54140,
832 .phy_id_mask= BCM54140_PHY_ID_MASK,
833 .name = "Broadcom BCM54140",
> 834 .flags = PHY_POLL_CABLE_TEST,
835 .features = PHY_GBIT_FEATURES,
836 .config_init= bcm54140_config_init,
837 .did_interrupt = bcm54140_did_interrupt,
838 .ack_interrupt = bcm54140_ack_intr,
839 .config_intr= bcm54140_config_intr,
840 .probe = bcm54140_probe,
841 .suspend= genphy_suspend,
842 .resume = genphy_resume,
843 .soft_reset = genphy_soft_reset,
844 .get_tunable= bcm54140_get_tunable,
845 .set_tunable= bcm54140_set_tunable,
> 846 .cable_test_start = bcm_phy_cable_test_start_rdb,
> 847 .cable_test_get_status =
> bcm_phy_cable_test_get_status_rdb,
848 },
849 };
850 module_phy_driver(bcm54140_drivers);
851
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths
> Is there a way to add a Fixes tag that would not invoke the -stable
> process? And was that what you had in mind?
Christophe Jaillet proposed to complete the exception handling also for this
function implementation.
I find that such a software correction is qualified for this tag.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n183
Corresponding consequences can vary then according to the change management
of involved developers.
> I think 'undo_probe1' is both descriptive and consistent with commit
> 10e3cc180e64 ("net/sonic: Fix a resource leak in an error handling path in
> 'jazz_sonic_probe()'").
I can agree to this view (in principle).
By the way:
The referenced commit contains the tag “Fixes”.
https://lore.kernel.org/patchwork/patch/1231354/
https://lore.kernel.org/lkml/20200427061803.53857-1-christophe.jail...@wanadoo.fr/
> Your suggestion, 'free_dma' is also good.
Thanks for your positive feedback.
> But coming up with good alternatives is easy.
But the change acceptance can occasionally become harder.
> If every good alternative would be considered there would be no obvious way
> to get a patch merged.
I imagine that some alternatives can result in preferable solutions, can't they?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n460
Regards,
Markus
[PATCH] powerpc/kvm/book3s64/vio: fix some RCU-list locks
It is unsafe to traverse kvm->arch.spapr_tce_tables and
stt->iommu_tables without the RCU read lock held. Also, add
cond_resched_rcu() in places with the RCU read lock held that could take
a while to finish.
arch/powerpc/kvm/book3s_64_vio.c:76 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
no locks held by qemu-kvm/4265.
stack backtrace:
CPU: 96 PID: 4265 Comm: qemu-kvm Not tainted 5.7.0-rc4-next-20200508+ #2
Call Trace:
[c000201a8690f720] [c0715948] dump_stack+0xfc/0x174 (unreliable)
[c000201a8690f770] [c01d9470] lockdep_rcu_suspicious+0x140/0x164
[c000201a8690f7f0] [c00810b9fb48]
kvm_spapr_tce_release_iommu_group+0x1f0/0x220 [kvm]
[c000201a8690f870] [c00810b8462c]
kvm_spapr_tce_release_vfio_group+0x54/0xb0 [kvm]
[c000201a8690f8a0] [c00810b84710] kvm_vfio_destroy+0x88/0x140 [kvm]
[c000201a8690f8f0] [c00810b7d488] kvm_put_kvm+0x370/0x600 [kvm]
[c000201a8690f990] [c00810b7e3c0] kvm_vm_release+0x38/0x60 [kvm]
[c000201a8690f9c0] [c05223f4] __fput+0x124/0x330
[c000201a8690fa20] [c0151cd8] task_work_run+0xb8/0x130
[c000201a8690fa70] [c01197e8] do_exit+0x4e8/0xfa0
[c000201a8690fb70] [c011a374] do_group_exit+0x64/0xd0
[c000201a8690fbb0] [c0132c90] get_signal+0x1f0/0x1200
[c000201a8690fcc0] [c0020690] do_notify_resume+0x130/0x3c0
[c000201a8690fda0] [c0038d64] syscall_exit_prepare+0x1a4/0x280
[c000201a8690fe20] [c000c8f8] system_call_common+0xf8/0x278
arch/powerpc/kvm/book3s_64_vio.c:368 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4264:
#0: c000201ae2d000d8 (>mutex){+.+.}-{3:3}, at:
kvm_vcpu_ioctl+0xdc/0x950 [kvm]
#1: c000200c9ed0c468 (>srcu){}-{0:0}, at:
kvmppc_h_put_tce+0x88/0x340 [kvm]
arch/powerpc/kvm/book3s_64_vio.c:108 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by qemu-kvm/4257:
#0: c000200b1b363a40 (>lock){+.+.}-{3:3}, at:
kvm_vfio_set_attr+0x598/0x6c0 [kvm]
arch/powerpc/kvm/book3s_64_vio.c:146 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by qemu-kvm/4257:
#0: c000200b1b363a40 (>lock){+.+.}-{3:3}, at:
kvm_vfio_set_attr+0x598/0x6c0 [kvm]
Signed-off-by: Qian Cai
---
arch/powerpc/kvm/book3s_64_vio.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 50555ad1db93..4f5016bab723 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -73,6 +73,7 @@ extern void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
struct kvmppc_spapr_tce_iommu_table *stit, *tmp;
struct iommu_table_group *table_group = NULL;
+ rcu_read_lock();
list_for_each_entry_rcu(stt, >arch.spapr_tce_tables, list) {
table_group = iommu_group_get_iommudata(grp);
@@ -87,7 +88,9 @@ extern void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
kref_put(>kref, kvm_spapr_tce_liobn_put);
}
}
+ cond_resched_rcu();
}
+ rcu_read_unlock();
}
extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
@@ -105,12 +108,14 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm
*kvm, int tablefd,
if (!f.file)
return -EBADF;
+ rcu_read_lock();
list_for_each_entry_rcu(stt, >arch.spapr_tce_tables, list) {
if (stt == f.file->private_data) {
found = true;
break;
}
}
+ rcu_read_unlock();
fdput(f);
@@ -143,6 +148,7 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm
*kvm, int tablefd,
if (!tbl)
return -EINVAL;
+ rcu_read_lock();
list_for_each_entry_rcu(stit, >iommu_tables, next) {
if (tbl != stit->tbl)
continue;
@@ -150,14 +156,17 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm
*kvm, int tablefd,
if (!kref_get_unless_zero(>kref)) {
/* stit is being destroyed */
iommu_tce_table_put(tbl);
+ rcu_read_unlock();
return -ENOTTY;
}
/*
* The table is already known to this KVM, we just increased
* its KVM reference counter and can return.
*/
+ rcu_read_unlock();
return 0;
}
+ rcu_read_unlock();
stit = kzalloc(sizeof(*stit),
Re: [PATCH v3] kernel: add panic_on_taint
On 05/09/20 at 09:10pm, Randy Dunlap wrote: > On 5/9/20 7:59 PM, Baoquan He wrote: > > Read admin-guide/tainted-kernels.rst, but still do not get what 'G' means. > > I interpret 'G' as GPL (strictly it means that no proprietary module has > been loaded). But I don't see why TAINT_PROPRIETARY_MODULE is the only > taint flag that has a non-blank c_false character. It could just be blank > also AFAICT. Then the 'G' would not be there to confuse us. :) Yeah, seems c_false character is not so necessary. If no 'P' set, then it means no proprietary modules loaded naturally. We may need clean up the c_false in struct taint_flag, since c_true is enough to indicate what want to check.
[PATCH] powerpc/mm/book3s64/iommu: fix some RCU-list locks
It is safe to traverse mm->context.iommu_group_mem_list with either
mem_list_mutex or the RCU read lock held. Silence a few RCU-list false
positive warnings and fix a few missing RCU read locks.
arch/powerpc/mm/book3s64/iommu_api.c:330 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4305:
#0: c00bc3fe4d68 (>lock){+.+.}-{3:3}, at:
tce_iommu_ioctl.part.9+0xc7c/0x1870 [vfio_iommu_spapr_tce]
#1: c1501910 (mem_list_mutex){+.+.}-{3:3}, at: mm_iommu_get+0x50/0x190
arch/powerpc/mm/book3s64/iommu_api.c:132 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4305:
#0: c00bc3fe4d68 (>lock){+.+.}-{3:3}, at:
tce_iommu_ioctl.part.9+0xc7c/0x1870 [vfio_iommu_spapr_tce]
#1: c1501910 (mem_list_mutex){+.+.}-{3:3}, at:
mm_iommu_do_alloc+0x120/0x5f0
arch/powerpc/mm/book3s64/iommu_api.c:292 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4312:
#0: c00ecafe23c8 (>mutex){+.+.}-{3:3}, at:
kvm_vcpu_ioctl+0xdc/0x950 [kvm]
#1: c00045e6c468 (>srcu){}-{0:0}, at:
kvmppc_h_put_tce+0x88/0x340 [kvm]
arch/powerpc/mm/book3s64/iommu_api.c:424 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4312:
#0: c00ecafe23c8 (>mutex){+.+.}-{3:3}, at:
kvm_vcpu_ioctl+0xdc/0x950 [kvm]
#1: c00045e6c468 (>srcu){}-{0:0}, at:
kvmppc_h_put_tce+0x88/0x340 [kvm]
Signed-off-by: Qian Cai
---
arch/powerpc/mm/book3s64/iommu_api.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/iommu_api.c
b/arch/powerpc/mm/book3s64/iommu_api.c
index fa05bbd1f682..bf0108b6f445 100644
--- a/arch/powerpc/mm/book3s64/iommu_api.c
+++ b/arch/powerpc/mm/book3s64/iommu_api.c
@@ -129,7 +129,8 @@ static long mm_iommu_do_alloc(struct mm_struct *mm,
unsigned long ua,
mutex_lock(_list_mutex);
- list_for_each_entry_rcu(mem2, >context.iommu_group_mem_list, next) {
+ list_for_each_entry_rcu(mem2, >context.iommu_group_mem_list, next,
+ lockdep_is_held(_list_mutex)) {
/* Overlap? */
if ((mem2->ua < (ua + (entries << PAGE_SHIFT))) &&
(ua < (mem2->ua +
@@ -289,6 +290,7 @@ struct mm_iommu_table_group_mem_t *mm_iommu_lookup(struct
mm_struct *mm,
{
struct mm_iommu_table_group_mem_t *mem, *ret = NULL;
+ rcu_read_lock();
list_for_each_entry_rcu(mem, >context.iommu_group_mem_list, next) {
if ((mem->ua <= ua) &&
(ua + size <= mem->ua +
@@ -297,6 +299,7 @@ struct mm_iommu_table_group_mem_t *mm_iommu_lookup(struct
mm_struct *mm,
break;
}
}
+ rcu_read_unlock();
return ret;
}
@@ -327,7 +330,8 @@ struct mm_iommu_table_group_mem_t *mm_iommu_get(struct
mm_struct *mm,
mutex_lock(_list_mutex);
- list_for_each_entry_rcu(mem, >context.iommu_group_mem_list, next) {
+ list_for_each_entry_rcu(mem, >context.iommu_group_mem_list, next,
+ lockdep_is_held(_list_mutex)) {
if ((mem->ua == ua) && (mem->entries == entries)) {
ret = mem;
++mem->used;
@@ -421,6 +425,7 @@ bool mm_iommu_is_devmem(struct mm_struct *mm, unsigned long
hpa,
struct mm_iommu_table_group_mem_t *mem;
unsigned long end;
+ rcu_read_lock();
list_for_each_entry_rcu(mem, >context.iommu_group_mem_list, next) {
if (mem->dev_hpa == MM_IOMMU_TABLE_INVALID_HPA)
continue;
@@ -437,6 +442,7 @@ bool mm_iommu_is_devmem(struct mm_struct *mm, unsigned long
hpa,
return true;
}
}
+ rcu_read_unlock();
return false;
}
--
2.21.0 (Apple Git-122.2)
[PATCH v1] dt-bindings: serial: qca,ar9330-uart: Convert to json-schema
Convert the Qualcomm Atheros AR9330 High-Speed UART
Device Tree binding documentation to json-schema.
Signed-off-by: Oleksij Rempel
---
.../bindings/serial/qca,ar9330-uart.txt | 31
.../bindings/serial/qca,ar9330-uart.yaml | 50 +++
2 files changed, 50 insertions(+), 31 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
create mode 100644
Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
deleted file mode 100644
index 7d65126bd1d77..0
--- a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-* Qualcomm Atheros AR9330 High-Speed UART
-
-Required properties:
-
-- compatible: Must be "qca,ar9330-uart"
-
-- reg: Specifies the physical base address of the controller and
- the length of the memory mapped region.
-
-- interrupts: Specifies the interrupt source of the parent interrupt
- controller. The format of the interrupt specifier depends on the
- parent interrupt controller.
-
-Additional requirements:
-
- Each UART port must have an alias correctly numbered in "aliases"
- node.
-
-Example:
-
- aliases {
- serial0 =
- };
-
- uart0: uart@1802 {
- compatible = "qca,ar9330-uart";
- reg = <0x1802 0x14>;
-
- interrupt-parent = <>;
- interrupts = <3>;
- };
diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
new file mode 100644
index 0..a344369285b6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/qca,ar9330-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Atheros AR9330 High-Speed UART
+
+maintainers:
+ - Oleksij Rempel
+
+allOf:
+ - $ref: /schemas/serial.yaml#
+
+properties:
+ compatible:
+const: qca,ar9330-uart
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+const: uart
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+serial@1802 {
+ compatible = "qca,ar9330-uart";
+ reg = <0x1802 0x14>;
+ clocks = <>;
+ clock-names = "uart";
+ interrupt-parent = <>;
+ interrupts = <3>;
+};
+...
--
2.26.2
Re: [PATCH v9 8/8] x86/split_lock: Enable split lock detection initialization when running as an guest on KVM
On Fri, May 8, 2020 at 8:04 PM Xiaoyao Li wrote:
>
> When running as guest, enumerating feature split lock detection through
> CPU model is not easy since CPU model is configurable by host VMM.
>
> If running upon KVM, it can be enumerated through
> KVM_FEATURE_SPLIT_LOCK_DETECT,
This needs crystal clear documentation. What, exactly, is the host
telling the guest if it sets this flag?
> and if KVM_HINTS_SLD_FATAL is set, it
> needs to be set to sld_fatal mode.
This needs much better docs. Do you mean:
"If KVM_HINTS_SLD_FATAL is set, then the guest will get #AC if it does
a split-lock regardless of what is written to MSR_TEST_CTRL?"
>
> Signed-off-by: Xiaoyao Li
> ---
> arch/x86/include/asm/cpu.h | 2 ++
> arch/x86/kernel/cpu/intel.c | 12 ++--
> arch/x86/kernel/kvm.c | 3 +++
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index a57f00f1d5b5..5d5b488b4b45 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -42,12 +42,14 @@ unsigned int x86_model(unsigned int sig);
> unsigned int x86_stepping(unsigned int sig);
> #ifdef CONFIG_CPU_SUP_INTEL
> extern void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c);
> +extern void __init split_lock_setup(bool fatal);
> extern void switch_to_sld(unsigned long tifn);
> extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
> extern bool handle_guest_split_lock(unsigned long ip);
> extern bool split_lock_virt_switch(bool on);
> #else
> static inline void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) {}
> +static inline void __init split_lock_setup(bool fatal) {}
> static inline void switch_to_sld(unsigned long tifn) {}
> static inline bool handle_user_split_lock(struct pt_regs *regs, long
> error_code)
> {
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 1e2a74e8c592..02e24134b9b5 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -996,12 +996,18 @@ static bool split_lock_verify_msr(bool on)
> return ctrl == tmp;
> }
>
> -static void __init split_lock_setup(void)
> +void __init split_lock_setup(bool fatal)
> {
> enum split_lock_detect_state state = sld_warn;
> char arg[20];
> int i, ret;
>
> + if (fatal) {
> + state = sld_fatal;
> + pr_info("forced on, sending SIGBUS on user-space
> split_locks\n");
> + goto set_cap;
> + }
> +
> if (!split_lock_verify_msr(false)) {
> pr_info("MSR access failed: Disabled\n");
> return;
> @@ -1037,6 +1043,7 @@ static void __init split_lock_setup(void)
> return;
> }
>
> +set_cap:
> setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT);
> if (state == sld_fatal)
> setup_force_cpu_cap(X86_FEATURE_SLD_FATAL);
> @@ -1161,6 +1168,7 @@ void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
> const struct x86_cpu_id *m;
> u64 ia32_core_caps;
>
> + /* Note, paravirt support can enable SLD, e.g., see kvm_guest_init().
> */
> if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
> return;
>
> @@ -1182,5 +1190,5 @@ void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
> return;
> }
>
> - split_lock_setup();
> + split_lock_setup(false);
> }
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 6efe0410fb72..489ea89e2e8e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -670,6 +670,9 @@ static void __init kvm_guest_init(void)
> * overcommitted.
> */
> hardlockup_detector_disable();
> +
> + if (kvm_para_has_feature(KVM_FEATURE_SPLIT_LOCK_DETECT))
> + split_lock_setup(kvm_para_has_hint(KVM_HINTS_SLD_FATAL));
> }
>
> static noinline uint32_t __kvm_cpuid_base(void)
> --
> 2.18.2
>
[PATCH] powerpc/powernv/pci: fix a RCU-list lock
It is unsafe to traverse tbl->it_group_list without the RCU read lock.
WARNING: suspicious RCU usage
5.7.0-rc4-next-20200508 #1 Not tainted
-
arch/powerpc/platforms/powernv/pci-ioda-tce.c:355 RCU-list traversed in
non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
3 locks held by qemu-kvm/4305:
#0: c00bc3fe6988 (>group_lock){}-{3:3}, at:
vfio_fops_unl_ioctl+0x108/0x410 [vfio]
#1: c0080fcc7400 (_drivers_lock){+.+.}-{3:3}, at:
vfio_fops_unl_ioctl+0x148/0x410 [vfio]
#2: c00bc3fe4d68 (>lock){+.+.}-{3:3}, at:
tce_iommu_attach_group+0x3c/0x4f0 [vfio_iommu_spapr_tce]
stack backtrace:
CPU: 4 PID: 4305 Comm: qemu-kvm Not tainted 5.7.0-rc4-next-20200508 #1
Call Trace:
[c010f29afa60] [c07154c8] dump_stack+0xfc/0x174 (unreliable)
[c010f29afab0] [c01d8ff0] lockdep_rcu_suspicious+0x140/0x164
[c010f29afb30] [c00dae2c]
pnv_pci_unlink_table_and_group+0x11c/0x200
[c010f29afb70] [c00d4a34] pnv_pci_ioda2_unset_window+0xc4/0x190
[c010f29afbf0] [c00d4b4c] pnv_ioda2_take_ownership+0x4c/0xd0
[c010f29afc30] [c0080fd60ee0] tce_iommu_attach_group+0x2c8/0x4f0
[vfio_iommu_spapr_tce]
[c010f29afcd0] [c0080fcc11a0] vfio_fops_unl_ioctl+0x238/0x410 [vfio]
[c010f29afd50] [c05430a8] ksys_ioctl+0xd8/0x130
[c010f29afda0] [c0543128] sys_ioctl+0x28/0x40
[c010f29afdc0] [c0038af4] system_call_exception+0x114/0x1e0
[c010f29afe20] [c000c8f0] system_call_common+0xf0/0x278
Signed-off-by: Qian Cai
---
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index 5dc6847d5f4c..6be9cf292b4e 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -352,6 +352,8 @@ void pnv_pci_unlink_table_and_group(struct iommu_table *tbl,
/* Remove link to a group from table's list of attached groups */
found = false;
+
+ rcu_read_lock();
list_for_each_entry_rcu(tgl, >it_group_list, next) {
if (tgl->table_group == table_group) {
list_del_rcu(>next);
@@ -360,6 +362,8 @@ void pnv_pci_unlink_table_and_group(struct iommu_table *tbl,
break;
}
}
+ rcu_read_unlock();
+
if (WARN_ON(!found))
return;
--
2.21.0 (Apple Git-122.2)
Re: [PATCH v9 3/8] x86/split_lock: Introduce flag X86_FEATURE_SLD_FATAL and drop sld_state
On Fri, May 8, 2020 at 8:03 PM Xiaoyao Li wrote: > > Introduce a synthetic feature flag X86_FEATURE_SLD_FATAL, which means > kernel is in sld_fatal mode if set. > > Now sld_state is not needed any more that the state of SLD can be > inferred from X86_FEATURE_SPLIT_LOCK_DETECT and X86_FEATURE_SLD_FATAL. Is it too much to ask for Intel to actually allocate and define a CPUID bit that means "this CPU *always* sends #AC on a split lock"? This would be a pure documentation change, but it would make this architectural rather than something that each hypervisor needs to hack up. Meanwhile, I don't see why adding a cpufeature flag is worthwhile to avoid a less bizarre global variable. There's no performance issue here, and the old code looked a lot more comprehensible than the new code.
Re: [RFC PATCH 0/7] mm: Get rid of vmalloc_sync_(un)mappings()
On Sat, May 9, 2020 at 2:57 PM Joerg Roedel wrote: > > Hi Andy, > > On Sat, May 09, 2020 at 12:05:29PM -0700, Andy Lutomirski wrote: > > So, unless I'm missing something here, there is an absolute maximum of > > 512 top-level entries that ever need to be synchronized. > > And here is where your assumption is wrong. On 32-bit PAE systems it is > not the top-level entries that need to be synchronized for vmalloc, but > the second-level entries. And dependent on the kernel configuration, > there are (in total, not only vmalloc) 1536, 1024, or 512 of these > second-level entries. How much of them are actually used for vmalloc > depends on the size of the system RAM (but is at least 64), because > the vmalloc area begins after the kernel direct-mapping (with an 8MB > unmapped hole). I spent some time looking at the code, and I'm guessing you're talking about the 3-level !SHARED_KERNEL_PMD case. I can't quite figure out what's going on. Can you explain what is actually going on that causes different mms/pgds to have top-level entries in the kernel range that point to different tables? Because I'm not seeing why this makes any sense. > > > Now, there's an additional complication. On x86_64, we have a rule: > > those entries that need to be synced start out null and may, during > > the lifetime of the system, change *once*. They are never unmapped or > > modified after being allocated. This means that those entries can > > only ever point to a page *table* and not to a ginormous page. So, > > even if the hardware were to support ginormous pages (which, IIRC, it > > doesn't), we would be limited to merely immense and not ginormous > > pages in the vmalloc range. On x86_32, I don't think we have this > > rule right now. And this means that it's possible for one of these > > pages to be unmapped or modified. > > The reason for x86-32 being different is that the address space is > orders of magnitude smaller than on x86-64. We just have 4 top-level > entries with PAE paging and can't afford to partition kernel-adress > space on that level like we do on x86-64. That is the reason the address > space is partitioned on the second (PMD) level, which is also the reason > vmalloc synchronization needs to happen on that level. And because > that's not enough yet, its also the page-table level to map huge-pages. Why does it need to be partitioned at all? The only thing that comes to mind is that the LDT range is per-mm. So I can imagine that the PAE case with a 3G user / 1G kernel split has to have the vmalloc range and the LDT range in the same top-level entry. Yuck. > > > So my suggestion is that just apply the x86_64 rule to x86_32 as well. > > The practical effect will be that 2-level-paging systems will not be > > able to use huge pages in the vmalloc range, since the rule will be > > that the vmalloc-relevant entries in the top-level table must point to > > page *tables* instead of huge pages. > > I could very well live with prohibiting huge-page ioremap mappings for > x86-32. But as I wrote before, this doesn't solve the problems I am > trying to address with this patch-set, or would only address them if > significant amount of total system memory is used. > > The pre-allocation solution would work for x86-64, it would only need > 256kb of preallocated memory for the vmalloc range to never synchronize > or fault again. I have thought about that and did the math before > writing this patch-set, but doing the math for 32 bit drove me away from > it for reasons written above. > If it's *just* the LDT that's a problem, we could plausibly shrink the user address range a little bit and put the LDT in the user portion. I suppose this could end up creating its own set of problems involving tracking which code owns which page tables. > And since a lot of the vmalloc_sync_(un)mappings problems I debugged > were actually related to 32-bit, I want a solution that works for 32 and > 64-bit x86 (at least until support for x86-32 is removed). And I think > this patch-set provides a solution that works well for both. I'm not fundamentally objecting to your patch set, but I do want to understand what's going on that needs this stuff. > > > Joerg
Re: [patch V4 part 5 02/31] x86/entry: Provide helpers for execute on irqstack
On Tue, May 5, 2020 at 10:19 PM Thomas Gleixner wrote: > > Device interrupt handlers and system vector handlers are executed on the > interrupt stack. The stack switch happens in the low level assembly entry > code. This conflicts with the efforts to consolidate the exit code in C to > ensure correctness vs. RCU and tracing. > > As there is no way to move #DB away from IST due to the MOV SS issue, the > requirements vs. #DB and NMI for switching to the interrupt stack do not > exist anymore. The only requirement is that interrupts are disabled. Hi, tglx and Andy Lutomirski, Is there any information about "no way to move #DB away from IST due to the MOV SS issue"? IST-based #DB results to ist_shift(for nested #DB) and debug_idt(for #NMI vs. #DB) which are somewhat ugly. If IST-less #DB should work, debug stack should be switched in software manner like interrupt stack. There was a "POP/MOV SS" CVE/issue about #BP which lead to moving #BP to IST-less by d8ba61ba58c8 (x86/entry/64: Don't use IST entry for #BP stack) #DB #BP are considered as #NMI due to their super-interrupt ability. But the kernel has much more control over #DB and #BP which can be disabled by putting the code snip into non-instrument sections like __entry noinstr etc. Is it possible to implement IST-less #DB? Thanks, Lai
Re: [PATCH 3/5] exec: Remove recursion from search_binary_handler
On 2020/05/10 4:41, Eric W. Biederman wrote: > --- a/fs/binfmt_misc.c > +++ b/fs/binfmt_misc.c > @@ -234,10 +234,7 @@ static int load_misc_binary(struct linux_binprm *bprm) > if (retval < 0) > goto error; > > - retval = search_binary_handler(bprm); > - if (retval < 0) > - goto error; > - > + retval = 1; /* Search for the interpreter */ > ret: > dput(fmt->dentry); > return retval; Wouldn't this change cause if (fd_binary > 0) ksys_close(fd_binary); bprm->interp_flags = 0; bprm->interp_data = 0; not to be called when "Search for the interpreter" failed?
Re: [PATCH v2 2/2] serial: lantiq: Make driver modular
Hi Rahul, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on tty/tty-testing] [also build test WARNING on next-20200508] [cannot apply to usb/usb-testing linus/master linux/master v5.7-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Rahul-Tanwar/serial-lantiq-Make-UART-s-use-as-console-selectable/20200509-174007 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: h8300-allyesconfig (attached as .config) compiler: h8300-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=h8300 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot All warnings (new ones prefixed by >>): >> WARNING: modpost: vmlinux.o(.data+0x172c14): Section mismatch in reference >> from the variable lqasc_driver to the variable .init.text:lqasc_probe The variable lqasc_driver references the variable __init lqasc_probe If the reference is valid then annotate the variable with or __refdata (see linux/init.h) or name the variable: --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v3] kernel: add panic_on_taint
On 5/9/20 7:59 PM, Baoquan He wrote: > Read admin-guide/tainted-kernels.rst, but still do not get what 'G' means. I interpret 'G' as GPL (strictly it means that no proprietary module has been loaded). But I don't see why TAINT_PROPRIETARY_MODULE is the only taint flag that has a non-blank c_false character. It could just be blank also AFAICT. Then the 'G' would not be there to confuse us. :) -- ~Randy
Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels
Sam Ravnborg 于2020年5月10日周日 上午4:06写道：
>
> Hi Dillon.
>
> On Fri, May 08, 2020 at 06:13:43PM +0800, dillon min wrote:
> > Hi Sam,
> >
> > Thanks for your comments, i will rework this panel driver after l3gd20
> > patch submission.
> >
> > Sam Ravnborg 于2020年5月8日周五 下午5:02写道：
> >
> > > Hi Dillon.
> > >
> > > Patch submissions starts to look fine.
> > >
> > > On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.min...@gmail.com wrote:
> > > > From: dillon min
> > > >
> > > > This is a driver for 320x240 TFT panels, accepting a rgb input
> > > > streams that get adapted and scaled to the panel.
> > > This driver is, I suppose, prepared to be a driver for ILI9341 based
> > > panles, and as such not for a fixed resolution.
> > > I expect (hope) we in the future will see more panels added.
> > >
> > > As i checked ili9341 datasheets, this panel just support 240x320
> > resolution only. if i'm wrong , please correct me. thanks
> > https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf
> >
> > This panel can support 9 different kinds of interface , "3/4-line Serial
> > Interface" have been supported by tiny/ili9341.c. i was verified it
> > but the performance on stm32f4 is very low.
> It had somehow escaped my mind we already have a driver for ili9341
> based panels.
> And we do not want onyl one driver for this IC.
> So please extent the current driver to match your panel as well as
> the original panel.
> And then also extend the current binding for ili9341, which
> you may have to convert to DT Schema in the process.
>
> This is more work now but when it is done we end up with a much better
> solution than if we introduce an additional driver for ili9341.
>
> Sorry for not catching this in the inital review.
>
> Sam
>
>
Hi Sam,
Yes, it's should be more reasonable to support different interface of
a panel in one driver.
i will add mipi dpi & dbi support to this panel in one driver just
like panel-simple.c(dpi & dsi)
one question, what to do next:
1 should i just add code to tiny/ili9341.c to support mipi dpi.
which means remove panel/panel-ilitek-ili9341.c
2 merge tiny/ili9341.c mipi dbi support to panel/panel-ilitek-ili9341.c ,
which means remove tiny/ili9341.c after this work done.
to extend dts binding for different interface. Ie.
original:
164 static const struct of_device_id ili9341_of_match[] = {
165 { .compatible = "adafruit,yx240qv29" },
166 { }
167 };
168 MODULE_DEVICE_TABLE(of, ili9341_of_match);
169
170 static const struct spi_device_id ili9341_id[] = {
171 { "yx240qv29", 0 },
172 { }
173 };
174 MODULE_DEVICE_TABLE(spi, ili9341_id);
i want to extend of_device_id and spi_device_id for driver to identify
dpi or dbi in probe.
"adafruit,yx240qv29" is for users who use original tiny drm driver
mipi dbi compatible
for panel's parallel mcu and parallel rgb interface.
static const struct of_device_id ili9341_of_match[] = {
{ .compatible = "adafruit,yx240qv29" },
{ .compatible = "ili9341,parallel-mcu" },
{ .compatible = "ili9341,parallel-rgb" },
{ }
};
static const struct spi_device_id ili9341_id[] = {
{ "yx240qv29", 0 },
{ "parallel-mcu", 0 },
{ "parallel-rgb", 0 },
{ }
};
static int ili9341_probe(struct spi_device *spi)
{
...
const struct spi_device_id *id = spi_get_device_id(spi);
if (!strcmp(id->name, "yx240qv29")) {
mipi dbi register
} else if (!strcmp(id->name, "parallel-rgb") ||
!strcmp(id->name, "parallel-mcu") {
mipi dpi register
} else {
failed handle
}
}
if this is a good idea to extend dts binding ?
thanks,
best regards.
Dillon
> >
> > currently, i just have stm32f429-disco in hands, with 18-bit parallel rgb
> > bus connected on this board. reference to panel-ilitek-ili9322 and
> > panel-ilitek-ili9881 driver, i have some plan to rewrite this driver.
> >
> > 1 add your below comments in.
> > 2 use dc-gpio, reset-gpio, rgb-interface-bits from dts to config panel
> > interface.
> > 3 drop regmap, porting drm_mipi_dbi's mipi_dbi_command to config panel
> > paramter. like tiny/ili9341.c
> > 4 support rgb-16, rgb-18 interface.
> > 5 use optional regulator or power gpio to control panel's power, as panel
> > power is always on for my board, so i can't test this part. could i add the
> > code which can't be tested?
> > 6 support rotation in panel config (currently , i rotate the screen by
> > kernel cmdline paramter)
> >
> > if you have any other common panel configuration should be add , please
> > inform me.
> >
> > thanks.
> >
> > >
> > > Some things to fix, see comments in the follwoing.
> > >
> > > Sam
> > >
> > > >
> > > > Signed-off-by: dillon min
> > > > ---
> > > > drivers/gpu/drm/panel/Kconfig| 8 +
> > > > drivers/gpu/drm/panel/Makefile | 1 +
> > > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561
> > > +++
>
Re: [PATCH v2] netprio_cgroup: Fix unlimited memory leak of v2 cgroups
On Fri, 8 May 2020 22:58:29 -0700 Jakub Kicinski wrote:
> On Sat, 9 May 2020 11:32:10 +0800 Zefan Li wrote:
> > If systemd is configured to use hybrid mode which enables the use of
> > both cgroup v1 and v2, systemd will create new cgroup on both the default
> > root (v2) and netprio_cgroup hierarchy (v1) for a new session and attach
> > task to the two cgroups. If the task does some network thing then the v2
> > cgroup can never be freed after the session exited.
> >
> > One of our machines ran into OOM due to this memory leak.
> >
> > In the scenario described above when sk_alloc() is called cgroup_sk_alloc()
> > thought it's in v2 mode, so it stores the cgroup pointer in sk->sk_cgrp_data
> > and increments the cgroup refcnt, but then sock_update_netprioidx() thought
> > it's in v1 mode, so it stores netprioidx value in sk->sk_cgrp_data, so the
> > cgroup refcnt will never be freed.
> >
> > Currently we do the mode switch when someone writes to the ifpriomap cgroup
> > control file. The easiest fix is to also do the switch when a task is
> > attached
> > to a new cgroup.
> >
> > Fixes: bd1060a1d671("sock, cgroup: add sock->sk_cgroup")
>
> ^ space missing here
>
> > Reported-by: Yang Yingliang
> > Tested-by: Yang Yingliang
> > Signed-off-by: Zefan Li
Fixed up the commit message and applied, thank you.
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Sat, 9 May 2020 23:43:08 + Saeed Mahameed wrote: > > Saeed, I'm expecting you to take this and the mlx4 patch via your > > trees. > > Yes for the mlx5 patch, but usually Dave takes mlx4 patches directly. Ack, it said IB on it, but looks like the patch can as well be applied to net-next, so I took it.
Re: [PATCH] IB/mlx4: Replace zero-length array with flexible-array
On Thu, 7 May 2020 13:59:21 -0500 Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> ...
Applied, thank you!
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Thu, 2020-05-07 at 13:59 -0500, Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array
> member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure,
> which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof
> operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible
> array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be
> hiding
hmmm, we actually have some tooling that rely on this to identify such
0 length fields .. since the structs in this file are usually auto-
generated from the hw sepcs .. now i see that these tools are broken in
our CI with this patch applied.
I guess we will need to fix them, and fix our code auto-generation
tools.
overall i am ok with this patch. I will apply it to mlx5-next.
and submit it upstream soom.
> some bugs. So, this work (flexible-array member conversions) will
> also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
> ---
> include/linux/mlx5/driver.h |2 -
> include/linux/mlx5/mlx5_ifc.h | 66 +
> -
> include/linux/mlx5/qp.h |2 -
> 3 files changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/mlx5/driver.h
> b/include/linux/mlx5/driver.h
> index 6f8f79ef829b..1a4ba36275de 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -200,7 +200,7 @@ struct mlx5_rsc_debug {
> void *object;
> enum dbg_rsc_type type;
> struct dentry *root;
> - struct mlx5_field_desc fields[0];
> + struct mlx5_field_desc fields[];
> };
>
> enum mlx5_dev_event {
> diff --git a/include/linux/mlx5/mlx5_ifc.h
> b/include/linux/mlx5/mlx5_ifc.h
> index 69b27c7dfc3e..c55686ff6504 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -1677,7 +1677,7 @@ struct mlx5_ifc_wq_bits {
>
> u8 reserved_at_140[0x4c0];
>
> - struct mlx5_ifc_cmd_pas_bits pas[0];
> + struct mlx5_ifc_cmd_pas_bits pas[];
> };
>
> struct mlx5_ifc_rq_num_bits {
> @@ -1895,7 +1895,7 @@ struct mlx5_ifc_resource_dump_menu_segment_bits
> {
> u8 reserved_at_20[0x10];
> u8 num_of_records[0x10];
>
> - struct mlx5_ifc_resource_dump_menu_record_bits record[0];
> + struct mlx5_ifc_resource_dump_menu_record_bits record[];
> };
>
> struct mlx5_ifc_resource_dump_resource_segment_bits {
> @@ -1907,7 +1907,7 @@ struct
> mlx5_ifc_resource_dump_resource_segment_bits {
>
> u8 index2[0x20];
>
> - u8 payload[0][0x20];
> + u8 payload[][0x20];
> };
>
> struct mlx5_ifc_resource_dump_terminate_segment_bits {
> @@ -2984,7 +2984,7 @@ struct mlx5_ifc_flow_context_bits {
>
> u8 reserved_at_1200[0x600];
>
> - union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits
> destination[0];
> + union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits
> destination[];
> };
>
> enum {
> @@ -3276,7 +3276,7 @@ struct mlx5_ifc_rqtc_bits {
>
> u8 reserved_at_e0[0x6a0];
>
> - struct mlx5_ifc_rq_num_bits rq_num[0];
> + struct mlx5_ifc_rq_num_bits rq_num[];
> };
>
> enum {
> @@ -3388,7 +3388,7 @@ struct mlx5_ifc_nic_vport_context_bits {
>
> u8 reserved_at_7e0[0x20];
>
> - u8 current_uc_mac_address[0][0x40];
> + u8 current_uc_mac_address[][0x40];
> };
>
> enum {
> @@ -4310,7 +4310,7 @@ struct mlx5_ifc_query_xrc_srq_out_bits {
>
> u8 reserved_at_280[0x600];
>
> - u8 pas[0][0x40];
> + u8 pas[][0x40];
> };
>
> struct mlx5_ifc_query_xrc_srq_in_bits {
> @@ -4588,7 +4588,7 @@ struct mlx5_ifc_query_srq_out_bits {
>
> u8 reserved_at_280[0x600];
>
> - u8 pas[0][0x40];
> + u8 pas[][0x40];
> };
>
> struct
[PATCH] ALSA: hda/realtek: Add quirk for Samsung Notebook
Some models of the Samsung Notebook 9 have very quiet and distorted
headphone output. This quirk changes the VREF value of the ALC298
codec NID 0x1a from default HIZ to new 100.
Signed-off-by: Mike Pozulp
---
sound/pci/hda/patch_realtek.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 63e1a56f705b..9b14ed5c75d7 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5923,6 +5923,7 @@ enum {
ALC294_FIXUP_ASUS_DUAL_SPK,
ALC285_FIXUP_THINKPAD_HEADSET_JACK,
ALC294_FIXUP_ASUS_HPE,
+ ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET,
};
static const struct hda_fixup alc269_fixups[] = {
@@ -7061,6 +7062,13 @@ static const struct hda_fixup alc269_fixups[] = {
.chained = true,
.chain_id = ALC294_FIXUP_ASUS_HEADSET_MIC
},
+ [ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET] = {
+ .type = HDA_FIXUP_VERBS,
+ .v.verbs = (const struct hda_verb[]) {
+ { 0x1a, AC_VERB_SET_PIN_WIDGET_CONTROL, 0xc5 },
+ { }
+ },
+ },
};
static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -7336,6 +7344,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x1b7d, 0xa831, "Ordissimo EVE2 ",
ALC269VB_FIXUP_ORDISSIMO_EVE2), /* Also known as Malata PC-B1303 */
SND_PCI_QUIRK(0x1d72, 0x1901, "RedmiBook 14",
ALC256_FIXUP_ASUS_HEADSET_MIC),
SND_PCI_QUIRK(0x10ec, 0x118c, "Medion EE4254 MD62100",
ALC256_FIXUP_MEDION_HEADSET_NO_PRESENCE),
+ SND_PCI_QUIRK(0x144d, 0xc169, "Samsung Notebook 9 Pen
(NP930SBE-K01US)", ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET),
+ SND_PCI_QUIRK(0x144d, 0xc176, "Samsung Notebook 9 Pro
(NP930MBE-K04US)", ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET),
#if 0
/* Below is a quirk table taken from the old code.
--
2.24.1
Re: [PATCHES] uaccess simple access_ok() removals
On Sat, May 09, 2020 at 05:34:58PM -0700, Linus Torvalds wrote: > On Sat, May 9, 2020 at 4:41 PM Al Viro wrote: > > > > Individual patches in followups; if nobody screams - into #for-next > > it goes... > > Looks fine to me, although I only read your commit logs, I didn't > verify that what you stated was actually true (ie the whole "only used > for xyz" parts). > > But I'll take your word for it. Particularly the double-underscore > versions are getting rare (and presumably some of the other branches > you have make it rarer still). I have - FWIW, right now I'm going through the patch series for netdev; once I'm done with turning commit messages into something printable (and finish rereading the patches themselves), there it goes, hopefully tonight. Then a bunch of small branches + repost of csum one + some of the weird shit (comedi compat ioctls - the largest pile of __get_user() and __put_user() outside of arch/* is festering there). Then regset stuff + (probably) resurrection of i915 stuff. I've got a bunch of stuff around execve(), but that'll have to wait until I get through the recently posted execve-related patchsets... Then there's e.g. mempolicy compat stuff, etc., but I'd rather wait for several days before posting that, reviewers' bandwidth being finite...
Re: [PATCH net-next 3/4] net: phy: broadcom: add cable test support
Hi Michael, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] [also build test ERROR on next-20200508] [cannot apply to net/master linus/master ipvs/master v5.7-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Michael-Walle/net-phy-broadcom-cable-tester-support/20200510-063955 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 2c674bec76d35b75c7c730f863424387c9e9633a config: i386-allyesconfig (attached as .config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot All errors (new ones prefixed by >>): drivers/net/phy/bcm-phy-lib.c: In function 'bcm_phy_cable_test_report_trans': >> drivers/net/phy/bcm-phy-lib.c:639:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_OK' undeclared (first use in this function); >> did you mean 'ETHTOOL_A_COALESCE_PKT_RATE_LOW'? return ETHTOOL_A_CABLE_RESULT_CODE_OK; ^~ ETHTOOL_A_COALESCE_PKT_RATE_LOW drivers/net/phy/bcm-phy-lib.c:639:10: note: each undeclared identifier is reported only once for each function it appears in >> drivers/net/phy/bcm-phy-lib.c:641:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_OPEN' undeclared (first use in this function); >> did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_OK'? return ETHTOOL_A_CABLE_RESULT_CODE_OPEN; ^~~~ ETHTOOL_A_CABLE_RESULT_CODE_OK >> drivers/net/phy/bcm-phy-lib.c:643:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT' undeclared (first use in this >> function); did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_OPEN'? return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT; ^~ ETHTOOL_A_CABLE_RESULT_CODE_OPEN >> drivers/net/phy/bcm-phy-lib.c:645:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT' undeclared (first use in this >> function); did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT'? return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT; ^~~ ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT >> drivers/net/phy/bcm-phy-lib.c:649:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC' undeclared (first use in this >> function); did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_OPEN'? return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC; ^~ ETHTOOL_A_CABLE_RESULT_CODE_OPEN drivers/net/phy/bcm-phy-lib.c: In function 'bcm_phy_report_length': >> drivers/net/phy/bcm-phy-lib.c:681:2: error: implicit declaration of function >> 'ethnl_cable_test_fault_length' [-Werror=implicit-function-declaration] ethnl_cable_test_fault_length(phydev, pair, val); ^ drivers/net/phy/bcm-phy-lib.c: In function '_bcm_phy_cable_test_get_status': >> drivers/net/phy/bcm-phy-lib.c:719:2: error: implicit declaration of function >> 'ethnl_cable_test_result'; did you mean 'bcm_phy_cable_test_start'? >> [-Werror=implicit-function-declaration] ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A, ^~~ bcm_phy_cable_test_start >> drivers/net/phy/bcm-phy-lib.c:719:34: error: 'ETHTOOL_A_CABLE_PAIR_A' >> undeclared (first use in this function); did you mean 'ETHTOOL_A_PAUSE_MAX'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A, ^~ ETHTOOL_A_PAUSE_MAX >> drivers/net/phy/bcm-phy-lib.c:721:34: error: 'ETHTOOL_A_CABLE_PAIR_B' >> undeclared (first use in this function); did you mean >> 'ETHTOOL_A_CABLE_PAIR_A'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_B, ^~ ETHTOOL_A_CABLE_PAIR_A >> drivers/net/phy/bcm-phy-lib.c:723:34: error: 'ETHTOOL_A_CABLE_PAIR_C' >> undeclared (first use in this function); did you mean >> 'ETHTOOL_A_CABLE_PAIR_B'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_C, ^~ ETHTOOL_A_CABLE_PAIR_B >> drivers/net/phy/bcm-phy-lib.c:725:34: error: 'ETHTOOL_A_CABLE_PAIR_D' >> undeclared (first use in this function); did you mean >> 'ETHTOOL_A_CABLE_PAIR_C'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_D, ^~ ETHTOOL_A_CABLE_PAIR_C cc1: some warnings being treated as errors vim +639
Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
On Sat, May 09, 2020 at 05:57:56PM -0700, Linus Torvalds wrote: > On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa > wrote: > > > > I think that this access_ok() check helps reducing partial writes (either > > "whole amount was processed" or "not processed at all" unless -ENOMEM). > > No it doesn't. > > "access_ok()" only checks the range being a valid user address range. > > It doesn't actually help at all if the worry is "what if we take a > page fault in the middle". Because it simply doesn't check those > kinds of things. > > Now, if somebody passes actual invalid ranges (ie kernel addresses or > other crazy stuff), they only have themselves to blame. The invalid > range will be noticed when actually doing the user copy, and then > you'll get EFAULT there. But there's no point in trying to figure that > out early - it's only adding overhead, and it doesn't help any normal > case. It might be a good idea to add Documentation/what-access_ok-does_not ;-/ In addition to what you've mentioned, * access_ok() does not fault anything in; never had. * access_ok() does not verify that memory is readable/writable/there at all; never had, except for genuine 80386 and (maybe) some of the shittier 486 clones. * access_ok() does not protect you from the length being insanely large; even on i386 it can pass with length being a bit under 3Gb. If you count upon it to prevent kmalloc() complaints about insanely large allocation (yes, I've seen that excuse used), you are wrong. * on a bunch of architectures access_ok() never rejects anything, and no, that's _not_ MMU-less ones. sparc64, for example. Or s390, or parisc, etc.
Re: [PATCH v3] kernel: add panic_on_taint
On 05/09/20 at 09:57am, Rafael Aquini wrote: > Analogously to the introduction of panic_on_warn, this patch > introduces a kernel option named panic_on_taint in order to > provide a simple and generic way to stop execution and catch > a coredump when the kernel gets tainted by any given taint flag. > > This is useful for debugging sessions as it avoids rebuilding > the kernel to explicitly add calls to panic() or BUG() into > code sites that introduce the taint flags of interest. > Another, perhaps less frequent, use for this option would be > as a mean for assuring a security policy (in paranoid mode) > case where no single taint is allowed for the running system. > > Suggested-by: Qian Cai > Signed-off-by: Rafael Aquini > --- > Changelog: > * v2: get rid of unnecessary/misguided compiler hints (Luis) > * v2: enhance documentation text for the new kernel parameter (Randy) > * v3: drop sysctl interface, keep it only as a kernel parameter (Luis) > > Documentation/admin-guide/kdump/kdump.rst | 10 + > .../admin-guide/kernel-parameters.txt | 15 +++ > include/linux/kernel.h| 2 + > kernel/panic.c| 40 +++ > kernel/sysctl.c | 9 - > 5 files changed, 75 insertions(+), 1 deletion(-) > > diff --git a/Documentation/admin-guide/kdump/kdump.rst > b/Documentation/admin-guide/kdump/kdump.rst > index ac7e131d2935..de3cf6d377cc 100644 > --- a/Documentation/admin-guide/kdump/kdump.rst > +++ b/Documentation/admin-guide/kdump/kdump.rst > @@ -521,6 +521,16 @@ will cause a kdump to occur at the panic() call. In > cases where a user wants > to specify this during runtime, /proc/sys/kernel/panic_on_warn can be set to > 1 > to achieve the same behaviour. > > +Trigger Kdump on add_taint() > + > + > +The kernel parameter, panic_on_taint, calls panic() from within add_taint(), > +whenever the value set in this bitmask matches with the bit flag being set > +by add_taint(). This will cause a kdump to occur at the panic() call. > +In cases where a user wants to specify this during runtime, > +/proc/sys/kernel/panic_on_taint can be set to a respective bitmask value > +to achieve the same behaviour. > + > Contact > === > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > index 7bc83f3d9bdf..4a69fe49a70d 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3404,6 +3404,21 @@ > panic_on_warn panic() instead of WARN(). Useful to cause kdump > on a WARN(). > > + panic_on_taint= [KNL] conditionally panic() in add_taint() > + Format: Changed it as 'Format: ' to be consistent with the existing other options? > + Specifies, as a string, the TAINT flag set that will > + compose a bitmask for calling panic() when the kernel > + gets tainted. > + See Documentation/admin-guide/tainted-kernels.rst for > + details on the taint flags that users can pick to > + compose the bitmask to assign to panic_on_taint. > + When the string is prefixed with a '-' the bitmask > + set in panic_on_taint will be mutually exclusive > + with the sysctl knob kernel.tainted, and any attempt > + to write to that sysctl will fail with -EINVAL for > + any taint value that masks with the flags set for > + this option. > + > crash_kexec_post_notifiers > Run kdump after running panic-notifiers and dumping > kmsg. This only for the users who doubt kdump always > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 9b7a8d74a9d6..66bc102cb59a 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -528,6 +528,8 @@ extern int panic_on_oops; > extern int panic_on_unrecovered_nmi; > extern int panic_on_io_nmi; > extern int panic_on_warn; > +extern unsigned long panic_on_taint; > +extern bool panic_on_taint_exclusive; > extern int sysctl_panic_on_rcu_stall; > extern int sysctl_panic_on_stackoverflow; > > diff --git a/kernel/panic.c b/kernel/panic.c > index b69ee9e76cb2..65c62f8a1de8 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -44,6 +45,8 @@ static int pause_on_oops_flag; > static DEFINE_SPINLOCK(pause_on_oops_lock); > bool crash_kexec_post_notifiers; > int panic_on_warn __read_mostly; > +unsigned long panic_on_taint; > +bool panic_on_taint_exclusive = false; > > int panic_timeout = CONFIG_PANIC_TIMEOUT; >
USB Attached SCSI breakage due no udev involvement
Hi, it's first time for me to report user-space breakage in here, so
i'm begging your pardon.
I want to report that Linux 5.4 breaking my USB mount workflow due
udevadm monitor report here (I'm using vanilla kernel 5.4.39 on
Slackware64 Current and vanilla kernel 4.4.221 on Slackware64 14.2):
$ udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent
KERNEL[4929.925420] add /devices/pci:00/:00:14.0/usb4/4-2 (usb)
KERNEL[4929.928109] add /devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0 (usb)
KERNEL[4929.929256] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5 (scsi)
KERNEL[4929.929316] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/scsi_host/host5
(scsi_host)
KERNEL[4929.929374] bind /devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0 (usb)
KERNEL[4929.929421] bind /devices/pci:00/:00:14.0/usb4/4-2 (usb)
KERNEL[4931.221449] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0
(scsi)
KERNEL[4931.221554] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0
(scsi)
KERNEL[4931.221600] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/scsi_device/5:0:0:0
(scsi_device)
KERNEL[4931.223340] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/bsg/5:0:0:0
(bsg)
KERNEL[4931.223422] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/scsi_disk/5:0:0:0
(scsi_disk)
KERNEL[4931.234657] add /devices/virtual/bdi/8:32 (bdi)
KERNEL[4931.239494] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/block/sdc
(block)
KERNEL[4931.239876] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/block/sdc/sdc1
(block)
KERNEL[4931.239942] add
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/block/sdc/sdc2
(block)
KERNEL[4931.243863] bind
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0
(scsi)
KERNEL[4945.416243] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/bsg/5:0:0:0
(bsg)
KERNEL[4945.416590] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/scsi_device/5:0:0:0
(scsi_device)
KERNEL[4945.416971] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/scsi_disk/5:0:0:0
(scsi_disk)
KERNEL[4945.419960] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/block/sdc/sdc2
(block)
KERNEL[4945.420133] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/block/sdc/sdc1
(block)
KERNEL[4945.420493] remove /devices/virtual/bdi/8:32 (bdi)
KERNEL[4945.420730] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0/block/sdc
(block)
KERNEL[4945.424904] unbind
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0
(scsi)
KERNEL[4945.424955] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0/5:0:0:0
(scsi)
KERNEL[4945.428890] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/target5:0:0
(scsi)
KERNEL[4945.428929] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5/scsi_host/host5
(scsi_host)
KERNEL[4945.428944] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0/host5 (scsi)
KERNEL[4945.437943] unbind
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0 (usb)
KERNEL[4945.437997] remove
/devices/pci:00/:00:14.0/usb4/4-2/4-2:1.0 (usb)
KERNEL[4945.438372] unbind /devices/pci:00/:00:14.0/usb4/4-2 (usb)
KERNEL[4945.438414] remove /devices/pci:00/:00:14.0/usb4/4-2 (usb)
Where's udev involvement? In Linux 4.4.221 (Slackware64 14.2), I got
udev involvement in udevadm monitor. When digging to use strace,
here’s comparasion of linux-4.4.221 and linux-5.4.39:
[Slackware64-14.2 linux-kernel-4.4.221]$ sudo strace -p 2102
strace: Process 2102 attached
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name(12)={sa_family=AF_NETLINK, pid=0,
groups=0001}, msg_iov(1)=[{"add@/devices/pci:00/:00:"...,
8192}], msg_controllen=32, [{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, {pid=0, uid=0, gid=0}}], msg_flags=0}, 0) =
250
write(1, "KERNEL[159.846137] add /dev"..., 76) = 76
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name(12)={sa_family=AF_NETLINK, pid=0,
groups=0001}, msg_iov(1)=[{"add@/devices/pci:00/:00:"...,
8192}], msg_controllen=32, [{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, {pid=0, uid=0, gid=0}}], msg_flags=0}, 0) =
280
write(1, "KERNEL[159.847267] add /dev"..., 84) = 84
epoll_wait(3, [{EPOLLIN, {u32=4, u64=4}}], 4, -1) = 1
recvmsg(4, {msg_name(12)={sa_family=AF_NETLINK, pid=598,
groups=0002},
msg_iov(1)=[{"libudev\0\376\355\312\376(\0\0\0(\0\0\0\23\2\0\0\5w\305\345'\370\365\f"...,
8192}],
[PATCH] clk / soc: mediatek: fix ptr_ret.cocci warnings
From: kbuild test robot
drivers/soc/mediatek/mtk-mmsys.c:28:1-3: WARNING: PTR_ERR_OR_ZERO can be used
Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
Generated by: scripts/coccinelle/api/ptr_ret.cocci
Fixes: 13032709e232 ("clk / soc: mediatek: Move mt8173 MMSYS to platform
driver")
CC: Matthias Brugger
Signed-off-by: kbuild test robot
---
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master
head: 30e2206e11ce27ae910cc0dab21472429e400a87
commit: 13032709e2328553970f0002df5edce6aac69425 [1266/7905] clk / soc:
mediatek: Move mt8173 MMSYS to platform driver
mtk-mmsys.c |5 +
1 file changed, 1 insertion(+), 4 deletions(-)
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -25,10 +25,7 @@ static int mtk_mmsys_probe(struct platfo
clks = platform_device_register_data(>dev, data->clk_driver,
PLATFORM_DEVID_AUTO, NULL, 0);
- if (IS_ERR(clks))
- return PTR_ERR(clks);
-
- return 0;
+ return PTR_ERR_OR_ZERO(clks);
}
static const struct of_device_id of_match_mtk_mmsys[] = {
Re: [patch V4 part 5 13/31] x86/irq: Convey vector as argument and not in ptregs
On Tue, May 5, 2020 at 10:23 PM Thomas Gleixner wrote: > +/* > + * ASM code to emit the common vector entry stubs where each stub is > + * packed into 8 bytes. > + * > + * Note, that the 'pushq imm8' is emitted via '.byte 0x6a, vector' because > + * GCC treats the local vector variable as unsigned int and would expand > + * all vectors above 0x7F to a 5 byte push. The original code did an > + * adjustment of the vector number to be in the signed byte range to avoid > + * this. While clever it's mindboggling counterintuitive and requires the > + * odd conversion back to a real vector number in the C entry points. Using > + * .byte achieves the same thing and the only fixup needed in the C entry > + * point is to mask off the bits above bit 7 because the push is sign > + * extending. > + */ > + .align 8 > +SYM_CODE_START(irq_entries_start) > +vector=FIRST_EXTERNAL_VECTOR > +.rept (FIRST_SYSTEM_VECTOR - FIRST_EXTERNAL_VECTOR) > + UNWIND_HINT_IRET_REGS > + .byte 0x6a, vector > + jmp common_interrupt > + .align 8 > +vector=vector+1 > +.endr > +SYM_CODE_END(irq_entries_start) Hello, tglx Using ".byte 0x6a, vector" is somewhat ugly. I hope it should be " pushq $(s8_to_s64(vector))", which can also help to reduce bunches of comments about ".byte 0x6a, vector". However, I don't know how to implement s8_to_s64() here. But at least the following code works (generates the same two-byte machine code as ".byte 0x6a, vector" does): .if vector < 128 pushq $(vector) .else pushq $(0xff00+vector) .endif Thanks, Lai
Re: [PATCH v5 1/9] w1_therm: adding code comments and code reordering
Hi--
On 5/9/20 4:57 PM, Akira Shimahara wrote:
> Adding code comments to split code in dedicated parts. After the global
> declarations (defines, macros and function declarations), code is organized
> as follow :
> - Device and family dependent structures and functions
> - Interfaces functions
> - Helpers functions
> - Hardware functions
> - Sysfs interface functions
>
> Signed-off-by: Akira Shimahara
> ---
> Main motivation on the first patch of this serie is to clean up the code,
> document it and reorder it to prepare the next patches, which are clearer
> after this.
>
> One main point is to keep all device/family dependent code gather at the
> beginning to ease adding new devices if required.
>
> Changes in v5:
> - All patch serie in one .c file
> - Correcting some comments
> - adding include
>
> drivers/w1/slaves/w1_therm.c | 403 ---
> 1 file changed, 237 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 18f08d7..f7147b2 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -41,55 +41,99 @@
> static int w1_strong_pullup = 1;
> module_param_named(strong_pullup, w1_strong_pullup, int, 0);
>
> +/*---Macros-*/
> +
> +/* return the address of the refcnt in the family data */
> +#define THERM_REFCNT(family_data) \
> + (&((struct w1_therm_family_data *)family_data)->refcnt)
> +
> +/*--Structs-*/
> +
> +/**
> + * struct w1_therm_family_converter
> + * \brief Used to bind standard function call
> + * to device specific function
> + * it could be routed to NULL if device don't support feature
> + */
Hi,
All of the struct and function documentation comments in all patches in
this patch series should be using kernel-doc notation instead of the above
(whatever it is; I don't know what it is).
> +struct w1_therm_family_converter {
> + u8 broken;
> + u16 reserved;
> + struct w1_family*f;
> + int (*convert)(u8 rom[9]);
> + int (*precision)(struct device *device, int val);
> + int (*eeprom)(struct device *device);
> +};
> +
thanks.
--
~Randy
Re: [PATCH 01/15] taint: add module firmware crash taint support
On 5/9/20 9:46 AM, Luis Chamberlain wrote: > On Sat, May 09, 2020 at 11:18:29AM -0400, Rafael Aquini wrote: >> We are still missing the documentation bits for this >> new flag, though. > > Ah yeah sorry about that. > >> How about having a blurb similar to: >> >> diff --git a/Documentation/admin-guide/tainted-kernels.rst >> b/Documentation/admin-guide/tainted-kernels.rst >> index 71e9184a9079..5c6a9e2478b0 100644 >> --- a/Documentation/admin-guide/tainted-kernels.rst >> +++ b/Documentation/admin-guide/tainted-kernels.rst >> @@ -100,6 +100,7 @@ Bit Log Number Reason that got the kernel tainted >> 15 _/K 32768 kernel has been live patched >> 16 _/X 65536 auxiliary taint, defined for and used by distros >> 17 _/T 131072 kernel was built with the struct randomization plugin >> + 18 _/Q 262144 driver firmware crash annotation >> === === == >> >> Note: The character ``_`` is representing a blank in this table to make >> reading >> @@ -162,3 +163,7 @@ More detailed explanation for tainting >> produce extremely unusual kernel structure layouts (even performance >> pathological ones), which is important to know when debugging. Set at >> build time. >> + >> + 18) ``Q`` Device drivers might annotate the kernel with this taint, in >> cases >> + their firmware might have crashed leaving the driver in a crippled and >> + potentially useless state. > > Sure, I'll modify it a bit to add the use case to help with support > issues, ie, to help rule out firmware issues. Please also update tools/debugging/kernel-chktaint. > I'm starting to think that to make this even more usesul later we may > want to add a uevent to add_taint() so that userspace can decide to look > into this, ignore it, or report something to the user, say on their > desktop. thanks. -- ~Randy
Re: [PATCH 00/15] net: taint when the device driver firmware crashes
On 5/9/20 6:58 PM, Andrew Lunn wrote: On Sat, May 09, 2020 at 06:01:51PM -0700, Shannon Nelson wrote: On 5/8/20 9:35 PM, Luis Chamberlain wrote: Device driver firmware can crash, and sometimes, this can leave your system in a state which makes the device or subsystem completely useless. Detecting this by inspecting /proc/sys/kernel/tainted instead of scraping some magical words from the kernel log, which is driver specific, is much easier. So instead this series provides a helper which lets drivers annotate this and shows how to use this on networking drivers. If the driver is able to detect that the device firmware has come back alive, through user intervention or whatever, should there be a way to "untaint" the kernel? Or would you expect it to remain tainted? Hi Shannon In general, you don't want to be able to untained. Say a non-GPL licenced module is loaded, which taints the kernel. It might then try to untaint the kernel to hide its. Yeah, obviously we don't want this to be abuseable. I was just wondering about reversing this particular status if the broken device could get itself fixed. As for firmware, how much damage can the firmware do as it crashed? If it is a DMA master, it could of splattered stuff through memory. Restarting the firmware is not going to reverse the damage it has done. True, and tho' the driver might get the thing restarted, it wouldn't necessarily know what kind of damage had ensued. Carry on, sln
Re: [PATCH] gcc-plugins: remove always false $(if ...) in Makefile
On Sun, May 10, 2020 at 11:00:44AM +0900, Masahiro Yamada wrote:
> This is the remnant of commit c17d6179ad5a ("gcc-plugins: remove unused
> GCC_PLUGIN_SUBDIR").
>
> $(if $(findstring /,$(p)),...) is always false because none of plugins
> contains '/' in the file name.
>
> Clean up the code.
>
> Signed-off-by: Masahiro Yamada
Reviewed-by: Kees Cook
--
Kees Cook
[PATCH] gcc-plugins: remove always false $(if ...) in Makefile
This is the remnant of commit c17d6179ad5a ("gcc-plugins: remove unused
GCC_PLUGIN_SUBDIR").
$(if $(findstring /,$(p)),...) is always false because none of plugins
contains '/' in the file name.
Clean up the code.
Signed-off-by: Masahiro Yamada
---
scripts/gcc-plugins/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile
index 80f354289eeb..4014ba7e2fbd 100644
--- a/scripts/gcc-plugins/Makefile
+++ b/scripts/gcc-plugins/Makefile
@@ -14,7 +14,7 @@ $(objtree)/$(obj)/randomize_layout_seed.h: FORCE
$(call if_changed,create_randomize_layout_seed)
targets = randomize_layout_seed.h randomize_layout_hash.h
-hostcxxlibs-y := $(foreach p,$(GCC_PLUGIN),$(if $(findstring /,$(p)),,$(p)))
+hostcxxlibs-y := $(GCC_PLUGIN)
always-y := $(hostcxxlibs-y)
$(foreach p,$(hostcxxlibs-y:%.so=%),$(eval $(p)-objs := $(p).o))
--
2.25.1
Re: [PATCH 00/15] net: taint when the device driver firmware crashes
On Sat, May 09, 2020 at 06:01:51PM -0700, Shannon Nelson wrote: > On 5/8/20 9:35 PM, Luis Chamberlain wrote: > > Device driver firmware can crash, and sometimes, this can leave your > > system in a state which makes the device or subsystem completely > > useless. Detecting this by inspecting /proc/sys/kernel/tainted instead > > of scraping some magical words from the kernel log, which is driver > > specific, is much easier. So instead this series provides a helper which > > lets drivers annotate this and shows how to use this on networking > > drivers. > > > If the driver is able to detect that the device firmware has come back > alive, through user intervention or whatever, should there be a way to > "untaint" the kernel? Or would you expect it to remain tainted? Hi Shannon In general, you don't want to be able to untained. Say a non-GPL licenced module is loaded, which taints the kernel. It might then try to untaint the kernel to hide its. As for firmware, how much damage can the firmware do as it crashed? If it is a DMA master, it could of splattered stuff through memory. Restarting the firmware is not going to reverse the damage it has done. Andrew
[PATCH v6 4/6] arch/x86/kvm: Refactor L1D flushing
Move out the initialization function to l1d_flush_init_once()
so that it can be reused for subsequent patches. The side-effect
of this patch is that the memory allocated for l1d flush pages
is no longer freed up and the memory allocated once is shared
amongst callers.
l1d_flush_sw/hw() are now abstracted under arch_l1d_flush().
vmx_l1d_flush_mutex however continues to exist as it also used
from other code paths.
Suggested-by: Thomas Gleixner
Signed-off-by: Balbir Singh
---
arch/x86/include/asm/cacheflush.h | 12 +++---
arch/x86/kernel/l1d_flush.c | 64 +++
arch/x86/kvm/vmx/vmx.c| 20 ++
3 files changed, 57 insertions(+), 39 deletions(-)
diff --git a/arch/x86/include/asm/cacheflush.h
b/arch/x86/include/asm/cacheflush.h
index 21cc3b28fa63..851d8f1ab827 100644
--- a/arch/x86/include/asm/cacheflush.h
+++ b/arch/x86/include/asm/cacheflush.h
@@ -7,11 +7,13 @@
#include
#define L1D_CACHE_ORDER 4
+
+enum l1d_flush_options {
+ L1D_FLUSH_POPULATE_TLB = 0x1,
+};
+
void clflush_cache_range(void *addr, unsigned int size);
-void l1d_flush_populate_tlb(void *l1d_flush_pages);
-void *l1d_flush_alloc_pages(void);
-void l1d_flush_cleanup_pages(void *l1d_flush_pages);
-void l1d_flush_sw(void *l1d_flush_pages);
-int l1d_flush_hw(void);
+int l1d_flush_init_once(void);
+void arch_l1d_flush(enum l1d_flush_options options);
#endif /* _ASM_X86_CACHEFLUSH_H */
diff --git a/arch/x86/kernel/l1d_flush.c b/arch/x86/kernel/l1d_flush.c
index 5871794f890d..ad66e5fe1565 100644
--- a/arch/x86/kernel/l1d_flush.c
+++ b/arch/x86/kernel/l1d_flush.c
@@ -1,10 +1,10 @@
#include
#include
-void *l1d_flush_alloc_pages(void)
+static void *l1d_flush_alloc_pages(void)
{
struct page *page;
- void *l1d_flush_pages = NULL;
+ void *flush_pages = NULL;
int i;
/*
@@ -14,7 +14,7 @@ void *l1d_flush_alloc_pages(void)
page = alloc_pages(GFP_KERNEL, L1D_CACHE_ORDER);
if (!page)
return NULL;
- l1d_flush_pages = page_address(page);
+ flush_pages = page_address(page);
/*
* Initialize each page with a different pattern in
@@ -22,25 +22,19 @@ void *l1d_flush_alloc_pages(void)
* virtualization case.
*/
for (i = 0; i < 1u << L1D_CACHE_ORDER; ++i) {
- memset(l1d_flush_pages + i * PAGE_SIZE, i + 1,
+ memset(flush_pages + i * PAGE_SIZE, i + 1,
PAGE_SIZE);
}
- return l1d_flush_pages;
+ return flush_pages;
}
-EXPORT_SYMBOL_GPL(l1d_flush_alloc_pages);
-void l1d_flush_cleanup_pages(void *l1d_flush_pages)
-{
- free_pages((unsigned long)l1d_flush_pages, L1D_CACHE_ORDER);
-}
-EXPORT_SYMBOL_GPL(l1d_flush_cleanup_pages);
/*
* Not all users of l1d flush would want to populate the TLB first
* split out the function so that callers can optionally flush the L1D
* cache via sw without prefetching the TLB.
*/
-void l1d_flush_populate_tlb(void *l1d_flush_pages)
+static void l1d_flush_populate_tlb(void *l1d_flush_pages)
{
int size = PAGE_SIZE << L1D_CACHE_ORDER;
@@ -58,9 +52,8 @@ void l1d_flush_populate_tlb(void *l1d_flush_pages)
[size] "r" (size)
: "eax", "ebx", "ecx", "edx");
}
-EXPORT_SYMBOL_GPL(l1d_flush_populate_tlb);
-int l1d_flush_hw(void)
+static int l1d_flush_hw(void)
{
if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
@@ -68,9 +61,8 @@ int l1d_flush_hw(void)
}
return -ENOTSUPP;
}
-EXPORT_SYMBOL_GPL(l1d_flush_hw);
-void l1d_flush_sw(void *l1d_flush_pages)
+static void l1d_flush_sw(void *l1d_flush_pages)
{
int size = PAGE_SIZE << L1D_CACHE_ORDER;
@@ -87,4 +79,42 @@ void l1d_flush_sw(void *l1d_flush_pages)
[size] "r" (size)
: "eax", "ecx");
}
-EXPORT_SYMBOL_GPL(l1d_flush_sw);
+
+static void *l1d_flush_pages;
+static DEFINE_MUTEX(l1d_flush_mutex);
+
+/*
+ * Initialize and setup L1D flush once, each caller will reuse the
+ * l1d_flush_pages for flushing, no per CPU allocations or NUMA aware
+ * allocations at the moment.
+ */
+int l1d_flush_init_once(void)
+{
+ int ret = 0;
+
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ return -ENOTSUPP;
+
+ if (static_cpu_has(X86_FEATURE_FLUSH_L1D) || l1d_flush_pages)
+ return ret;
+
+ mutex_lock(_flush_mutex);
+ if (!l1d_flush_pages)
+ l1d_flush_pages = l1d_flush_alloc_pages();
+ ret = l1d_flush_pages ? 0 : -ENOMEM;
+ mutex_unlock(_flush_mutex);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(l1d_flush_init_once);
+
+void arch_l1d_flush(enum l1d_flush_options options)
+{
+ if (!l1d_flush_hw())
+ return;
+
+ if (options & L1D_FLUSH_POPULATE_TLB)
+ l1d_flush_populate_tlb(l1d_flush_pages);
+
+ l1d_flush_sw(l1d_flush_pages);
+}
[PATCH v6 2/6] arch/x86/kvm: Refactor tlbflush and l1d flush
Refactor the existing assembly bits into smaller helper functions
and also abstract L1D_FLUSH into a helper function. Use these
functions in kvm for L1D flushing.
Reviewed-by: Kees Cook
Signed-off-by: Balbir Singh
---
arch/x86/include/asm/cacheflush.h | 3 ++
arch/x86/kernel/l1d_flush.c | 54 +++
arch/x86/kvm/vmx/vmx.c| 29 ++---
3 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/arch/x86/include/asm/cacheflush.h
b/arch/x86/include/asm/cacheflush.h
index bac56fcd9790..21cc3b28fa63 100644
--- a/arch/x86/include/asm/cacheflush.h
+++ b/arch/x86/include/asm/cacheflush.h
@@ -8,7 +8,10 @@
#define L1D_CACHE_ORDER 4
void clflush_cache_range(void *addr, unsigned int size);
+void l1d_flush_populate_tlb(void *l1d_flush_pages);
void *l1d_flush_alloc_pages(void);
void l1d_flush_cleanup_pages(void *l1d_flush_pages);
+void l1d_flush_sw(void *l1d_flush_pages);
+int l1d_flush_hw(void);
#endif /* _ASM_X86_CACHEFLUSH_H */
diff --git a/arch/x86/kernel/l1d_flush.c b/arch/x86/kernel/l1d_flush.c
index d605878c8f28..5871794f890d 100644
--- a/arch/x86/kernel/l1d_flush.c
+++ b/arch/x86/kernel/l1d_flush.c
@@ -34,3 +34,57 @@ void l1d_flush_cleanup_pages(void *l1d_flush_pages)
free_pages((unsigned long)l1d_flush_pages, L1D_CACHE_ORDER);
}
EXPORT_SYMBOL_GPL(l1d_flush_cleanup_pages);
+
+/*
+ * Not all users of l1d flush would want to populate the TLB first
+ * split out the function so that callers can optionally flush the L1D
+ * cache via sw without prefetching the TLB.
+ */
+void l1d_flush_populate_tlb(void *l1d_flush_pages)
+{
+ int size = PAGE_SIZE << L1D_CACHE_ORDER;
+
+ asm volatile(
+ /* First ensure the pages are in the TLB */
+ "xorl %%eax, %%eax\n"
+ ".Lpopulate_tlb:\n\t"
+ "movzbl (%[flush_pages], %%" _ASM_AX "), %%ecx\n\t"
+ "addl $4096, %%eax\n\t"
+ "cmpl %%eax, %[size]\n\t"
+ "jne.Lpopulate_tlb\n\t"
+ "xorl %%eax, %%eax\n\t"
+ "cpuid\n\t"
+ :: [flush_pages] "r" (l1d_flush_pages),
+ [size] "r" (size)
+ : "eax", "ebx", "ecx", "edx");
+}
+EXPORT_SYMBOL_GPL(l1d_flush_populate_tlb);
+
+int l1d_flush_hw(void)
+{
+ if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
+ wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
+ return 0;
+ }
+ return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(l1d_flush_hw);
+
+void l1d_flush_sw(void *l1d_flush_pages)
+{
+ int size = PAGE_SIZE << L1D_CACHE_ORDER;
+
+ asm volatile(
+ /* Fill the cache */
+ "xorl %%eax, %%eax\n"
+ ".Lfill_cache:\n"
+ "movzbl (%[flush_pages], %%" _ASM_AX "), %%ecx\n\t"
+ "addl $64, %%eax\n\t"
+ "cmpl %%eax, %[size]\n\t"
+ "jne.Lfill_cache\n\t"
+ "lfence\n"
+ :: [flush_pages] "r" (l1d_flush_pages),
+ [size] "r" (size)
+ : "eax", "ecx");
+}
+EXPORT_SYMBOL_GPL(l1d_flush_sw);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f35654db904a..4f95927aad4c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6031,8 +6031,6 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu,
*/
static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
{
- int size = PAGE_SIZE << L1D_CACHE_ORDER;
-
/*
* This code is only executed when the the flush mode is 'cond' or
* 'always'
@@ -6061,32 +6059,11 @@ static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
vcpu->stat.l1d_flush++;
- if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
- wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
+ if (!l1d_flush_hw())
return;
- }
- asm volatile(
- /* First ensure the pages are in the TLB */
- "xorl %%eax, %%eax\n"
- ".Lpopulate_tlb:\n\t"
- "movzbl (%[flush_pages], %%" _ASM_AX "), %%ecx\n\t"
- "addl $4096, %%eax\n\t"
- "cmpl %%eax, %[size]\n\t"
- "jne.Lpopulate_tlb\n\t"
- "xorl %%eax, %%eax\n\t"
- "cpuid\n\t"
- /* Now fill the cache */
- "xorl %%eax, %%eax\n"
- ".Lfill_cache:\n"
- "movzbl (%[flush_pages], %%" _ASM_AX "), %%ecx\n\t"
- "addl $64, %%eax\n\t"
- "cmpl %%eax, %[size]\n\t"
- "jne.Lfill_cache\n\t"
- "lfence\n"
- :: [flush_pages] "r" (vmx_l1d_flush_pages),
- [size] "r" (size)
- : "eax", "ebx", "ecx", "edx");
+ l1d_flush_populate_tlb(vmx_l1d_flush_pages);
+ l1d_flush_sw(vmx_l1d_flush_pages);
}
static void
[PATCH v6 6/6] Documentation: Add L1D flushing Documentation
Add documentation of l1d flushing, explain the need for the feature and how it can be used. Signed-off-by: Balbir Singh Reviewed-by: Kees Cook --- Documentation/admin-guide/hw-vuln/index.rst | 1 + .../admin-guide/hw-vuln/l1d_flush.rst | 40 +++ 2 files changed, 41 insertions(+) create mode 100644 Documentation/admin-guide/hw-vuln/l1d_flush.rst diff --git a/Documentation/admin-guide/hw-vuln/index.rst b/Documentation/admin-guide/hw-vuln/index.rst index 0795e3c2643f..35633b299d45 100644 --- a/Documentation/admin-guide/hw-vuln/index.rst +++ b/Documentation/admin-guide/hw-vuln/index.rst @@ -14,3 +14,4 @@ are configurable at compile, boot or run time. mds tsx_async_abort multihit.rst + l1d_flush diff --git a/Documentation/admin-guide/hw-vuln/l1d_flush.rst b/Documentation/admin-guide/hw-vuln/l1d_flush.rst new file mode 100644 index ..7d515b8c29f1 --- /dev/null +++ b/Documentation/admin-guide/hw-vuln/l1d_flush.rst @@ -0,0 +1,40 @@ +L1D Flushing for the paranoid += + +With an increasing number of vulnerabilities being reported around data +leaks from L1D, a new user space mechanism to flush the L1D cache on +context switch is added to the kernel. This should help address +CVE-2020-0550 and for paranoid applications, keep them safe from any +yet to be discovered vulnerabilities, related to leaks from the L1D +cache. + +Tasks can opt in to this mechanism by using a prctl (implemented only +for x86 at the moment). + +Related CVES + +At the present moment, the following CVEs can be addressed by this +mechanism + += == +CVE-2020-0550 Improper Data Forwarding OS related aspects += == + +Usage Guidelines + +Applications can call ``prctl(2)`` with one of these two arguments + +1. PR_SET_L1D_FLUSH - flush the L1D cache on context switch (out) +2. PR_GET_L1D_FLUSH - get the current state of the L1D cache flush, returns 1 + if set and 0 if not set. + +**NOTE**: The feature is disabled by default, applications to need to specifically +opt into the feature to enable it. + +Mitigation +-- +When PR_SET_L1D_FLUSH is enabled for a task, on switching tasks (when +the address space changes), a flush of the L1D cache is performed for +the task when it leaves the CPU. If the underlying CPU supports L1D +flushing in hardware, the hardware mechanism is used, otherwise a software +fallback, similar to the mechanism used by L1TF is used. -- 2.17.1
[PATCH v6 1/6] arch/x86/kvm: Refactor l1d flush lifecycle management
Split out the allocation and free routines to be used in a follow
up set of patches (to reuse for L1D flushing).
Signed-off-by: Balbir Singh
Reviewed-by: Kees Cook
---
arch/x86/include/asm/cacheflush.h | 3 +++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/l1d_flush.c | 36 +++
arch/x86/kvm/vmx/vmx.c| 25 +++--
4 files changed, 43 insertions(+), 22 deletions(-)
create mode 100644 arch/x86/kernel/l1d_flush.c
diff --git a/arch/x86/include/asm/cacheflush.h
b/arch/x86/include/asm/cacheflush.h
index 63feaf2a5f93..bac56fcd9790 100644
--- a/arch/x86/include/asm/cacheflush.h
+++ b/arch/x86/include/asm/cacheflush.h
@@ -6,6 +6,9 @@
#include
#include
+#define L1D_CACHE_ORDER 4
void clflush_cache_range(void *addr, unsigned int size);
+void *l1d_flush_alloc_pages(void);
+void l1d_flush_cleanup_pages(void *l1d_flush_pages);
#endif /* _ASM_X86_CACHEFLUSH_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e77261db2391..c17c1e3c1a0b 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -157,3 +157,4 @@ ifeq ($(CONFIG_X86_64),y)
endif
obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o
+obj-y += l1d_flush.o
diff --git a/arch/x86/kernel/l1d_flush.c b/arch/x86/kernel/l1d_flush.c
new file mode 100644
index ..d605878c8f28
--- /dev/null
+++ b/arch/x86/kernel/l1d_flush.c
@@ -0,0 +1,36 @@
+#include
+#include
+
+void *l1d_flush_alloc_pages(void)
+{
+ struct page *page;
+ void *l1d_flush_pages = NULL;
+ int i;
+
+ /*
+* This allocation for l1d_flush_pages is not tied to a VM/task's
+* lifetime and so should not be charged to a memcg.
+*/
+ page = alloc_pages(GFP_KERNEL, L1D_CACHE_ORDER);
+ if (!page)
+ return NULL;
+ l1d_flush_pages = page_address(page);
+
+ /*
+* Initialize each page with a different pattern in
+* order to protect against KSM in the nested
+* virtualization case.
+*/
+ for (i = 0; i < 1u << L1D_CACHE_ORDER; ++i) {
+ memset(l1d_flush_pages + i * PAGE_SIZE, i + 1,
+ PAGE_SIZE);
+ }
+ return l1d_flush_pages;
+}
+EXPORT_SYMBOL_GPL(l1d_flush_alloc_pages);
+
+void l1d_flush_cleanup_pages(void *l1d_flush_pages)
+{
+ free_pages((unsigned long)l1d_flush_pages, L1D_CACHE_ORDER);
+}
+EXPORT_SYMBOL_GPL(l1d_flush_cleanup_pages);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 00d31a5e0089..f35654db904a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -203,14 +203,10 @@ static const struct {
[VMENTER_L1D_FLUSH_NOT_REQUIRED] = {"not required", false},
};
-#define L1D_CACHE_ORDER 4
static void *vmx_l1d_flush_pages;
static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf)
{
- struct page *page;
- unsigned int i;
-
if (!boot_cpu_has_bug(X86_BUG_L1TF)) {
l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED;
return 0;
@@ -253,24 +249,9 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state
l1tf)
if (l1tf != VMENTER_L1D_FLUSH_NEVER && !vmx_l1d_flush_pages &&
!boot_cpu_has(X86_FEATURE_FLUSH_L1D)) {
- /*
-* This allocation for vmx_l1d_flush_pages is not tied to a VM
-* lifetime and so should not be charged to a memcg.
-*/
- page = alloc_pages(GFP_KERNEL, L1D_CACHE_ORDER);
- if (!page)
+ vmx_l1d_flush_pages = l1d_flush_alloc_pages();
+ if (!vmx_l1d_flush_pages)
return -ENOMEM;
- vmx_l1d_flush_pages = page_address(page);
-
- /*
-* Initialize each page with a different pattern in
-* order to protect against KSM in the nested
-* virtualization case.
-*/
- for (i = 0; i < 1u << L1D_CACHE_ORDER; ++i) {
- memset(vmx_l1d_flush_pages + i * PAGE_SIZE, i + 1,
- PAGE_SIZE);
- }
}
l1tf_vmx_mitigation = l1tf;
@@ -8099,7 +8080,7 @@ static struct kvm_x86_init_ops vmx_init_ops __initdata = {
static void vmx_cleanup_l1d_flush(void)
{
if (vmx_l1d_flush_pages) {
- free_pages((unsigned long)vmx_l1d_flush_pages, L1D_CACHE_ORDER);
+ l1d_flush_cleanup_pages(vmx_l1d_flush_pages);
vmx_l1d_flush_pages = NULL;
}
/* Restore state so sysfs ignores VMX */
--
2.17.1
[PATCH v6 0/6] Optionally flush L1D on context switch
Provide a mechanism to flush the L1D cache on context switch. The goal is to allow tasks that are paranoid due to the recent snoop assisted data sampling vulnerabilites, to flush their L1D on being switched out. This protects their data from being snooped or leaked via side channels after the task has context switched out. Changelog v6: - Fix the complaint about variable shadowing (Reported-by: kbuild test robot ) Changelog v5: - Based on Tom's recommendation, restrict the patches to Intel CPUs only (thomas.lenda...@amd.com) - Update reviewed-by tags based on v4. Changelog v4: - Refactor the L1D flushing code even further, pages are now allocated once and never freed. Simplify the exported functions. - Change the name prefixs to be more consistent (l1d_flush_*) - Refactoring of the code done in the spirit of the comments, prctl still requires arch bits for get/set L1D flush and ofcourse in the arch switch_mm bits flushing the L1D cache. Changelog v3: - Refactor the return value of what flush_l1d_cache_hw() returns - Refactor the code, so that the generic setup bits come first (patch 3 from previous posting is now patches 3 and 4) - Move from arch_prctl() to the prctl() interface as recommend in the reviews. Changelog v2: - Fix a miss of mutex_unlock (caught by Borislav Petkov ) - Add documentation about the changes (Josh Poimboeuf ) Changelog: - Refactor the code and reuse cond_ibpb() - code bits provided by tglx - Merge mm state tracking for ibpb and l1d flush - Rename TIF_L1D_FLUSH to TIF_SPEC_FLUSH_L1D Changelog RFC: - Reuse existing code for allocation and flush - Simplify the goto logic in the actual l1d_flush function - Optimize the code path with jump labels/static functions The previous version of these patches are posted at: https://lore.kernel.org/lkml/20200504041343.9651-1-sbl...@amazon.com/ Balbir Singh (6): arch/x86/kvm: Refactor l1d flush lifecycle management arch/x86/kvm: Refactor tlbflush and l1d flush arch/x86/mm: Refactor cond_ibpb() to support other use cases arch/x86/kvm: Refactor L1D flushing Optionally flush L1D on context switch Documentation: Add L1D flushing Documentation Documentation/admin-guide/hw-vuln/index.rst | 1 + .../admin-guide/hw-vuln/l1d_flush.rst | 40 ++ arch/x86/include/asm/cacheflush.h | 8 ++ arch/x86/include/asm/thread_info.h| 7 +- arch/x86/include/asm/tlbflush.h | 2 +- arch/x86/kernel/Makefile | 1 + arch/x86/kernel/l1d_flush.c | 120 ++ arch/x86/kvm/vmx/vmx.c| 62 + arch/x86/mm/tlb.c | 83 +--- include/uapi/linux/prctl.h| 4 + kernel/sys.c | 20 +++ 11 files changed, 266 insertions(+), 82 deletions(-) create mode 100644 Documentation/admin-guide/hw-vuln/l1d_flush.rst create mode 100644 arch/x86/kernel/l1d_flush.c -- 2.17.1
[PATCH v6 3/6] arch/x86/mm: Refactor cond_ibpb() to support other use cases
cond_ibpb() has the necessary bits required to track the
previous mm in switch_mm_irqs_off(). This can be reused for
other use cases like L1D flushing (on context switch out).
Suggested-by: Thomas Gleixner
Signed-off-by: Balbir Singh
---
arch/x86/include/asm/tlbflush.h | 2 +-
arch/x86/mm/tlb.c | 43 +
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 8c87a2e0b660..a927d40664df 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -83,7 +83,7 @@ struct tlb_state {
/* Last user mm for optimizing IBPB */
union {
struct mm_struct*last_user_mm;
- unsigned long last_user_mm_ibpb;
+ unsigned long last_user_mm_spec;
};
u16 loaded_mm_asid;
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index cf81902e6992..10056b8d8f01 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -43,10 +43,11 @@
*/
/*
- * Use bit 0 to mangle the TIF_SPEC_IB state into the mm pointer which is
- * stored in cpu_tlb_state.last_user_mm_ibpb.
+ * Bits to mangle the TIF_SPEC_IB state into the mm pointer which is
+ * stored in cpu_tlb_state.last_user_mm_spec.
*/
#define LAST_USER_MM_IBPB 0x1UL
+#define LAST_USER_MM_SPEC_MASK (LAST_USER_MM_IBPB)
/*
* The x86 feature is called PCID (Process Context IDentifier). It is similar
@@ -345,19 +346,24 @@ static void sync_current_stack_to_mm(struct mm_struct *mm)
}
}
-static inline unsigned long mm_mangle_tif_spec_ib(struct task_struct *next)
+static inline unsigned long mm_mangle_tif_spec_bits(struct task_struct *next)
{
unsigned long next_tif = task_thread_info(next)->flags;
- unsigned long ibpb = (next_tif >> TIF_SPEC_IB) & LAST_USER_MM_IBPB;
+ unsigned long spec_bits = (next_tif >> TIF_SPEC_IB) &
LAST_USER_MM_SPEC_MASK;
- return (unsigned long)next->mm | ibpb;
+ return (unsigned long)next->mm | spec_bits;
}
-static void cond_ibpb(struct task_struct *next)
+static void cond_mitigation(struct task_struct *next)
{
+ unsigned long prev_mm, next_mm;
+
if (!next || !next->mm)
return;
+ next_mm = mm_mangle_tif_spec_bits(next);
+ prev_mm = this_cpu_read(cpu_tlbstate.last_user_mm_spec);
+
/*
* Both, the conditional and the always IBPB mode use the mm
* pointer to avoid the IBPB when switching between tasks of the
@@ -368,8 +374,6 @@ static void cond_ibpb(struct task_struct *next)
* exposed data is not really interesting.
*/
if (static_branch_likely(_mm_cond_ibpb)) {
- unsigned long prev_mm, next_mm;
-
/*
* This is a bit more complex than the always mode because
* it has to handle two cases:
@@ -399,20 +403,14 @@ static void cond_ibpb(struct task_struct *next)
* Optimize this with reasonably small overhead for the
* above cases. Mangle the TIF_SPEC_IB bit into the mm
* pointer of the incoming task which is stored in
-* cpu_tlbstate.last_user_mm_ibpb for comparison.
-*/
- next_mm = mm_mangle_tif_spec_ib(next);
- prev_mm = this_cpu_read(cpu_tlbstate.last_user_mm_ibpb);
-
- /*
+* cpu_tlbstate.last_user_mm_spec for comparison.
+*
* Issue IBPB only if the mm's are different and one or
* both have the IBPB bit set.
*/
if (next_mm != prev_mm &&
(next_mm | prev_mm) & LAST_USER_MM_IBPB)
indirect_branch_prediction_barrier();
-
- this_cpu_write(cpu_tlbstate.last_user_mm_ibpb, next_mm);
}
if (static_branch_unlikely(_mm_always_ibpb)) {
@@ -421,11 +419,12 @@ static void cond_ibpb(struct task_struct *next)
* different context than the user space task which ran
* last on this CPU.
*/
- if (this_cpu_read(cpu_tlbstate.last_user_mm) != next->mm) {
+ if ((prev_mm & ~LAST_USER_MM_SPEC_MASK) !=
+ (unsigned long)next->mm)
indirect_branch_prediction_barrier();
- this_cpu_write(cpu_tlbstate.last_user_mm, next->mm);
- }
}
+
+ this_cpu_write(cpu_tlbstate.last_user_mm_spec, next_mm);
}
#ifdef CONFIG_PERF_EVENTS
@@ -550,8 +549,10 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct
mm_struct *next,
* Avoid user/user BTB poisoning by flushing the branch
* predictor when switching between processes. This stops
* one process from doing Spectre-v2 attacks on another.
+* The hook can
[PATCH v6 5/6] Optionally flush L1D on context switch
Implement a mechanism to selectively flush the L1D cache. The goal is to
allow tasks that are paranoid due to the recent snoop assisted data sampling
vulnerabilites, to flush their L1D on being switched out. This protects
their data from being snooped or leaked via side channels after the task
has context switched out.
There are two scenarios we might want to protect against, a task leaving
the CPU with data still in L1D (which is the main concern of this patch),
the second scenario is a malicious task coming in (not so well trusted)
for which we want to clean up the cache before it starts. Only the case
for the former is addressed.
A new thread_info flag TIF_SPEC_FLUSH_L1D is added to track tasks which
opt-into L1D flushing. cpu_tlbstate.last_user_mm_spec is used to convert
the TIF flags into mm state (per cpu via last_user_mm_spec) in
cond_mitigation(), which then used to do decide when to call flush_l1d().
Add prctl()'s to opt-in to the L1D cache on context switch out, the
existing mechanisms of tracking prev_mm via cpu_tlbstate is
reused to track state of the tasks and to flush the L1D cache.
The prctl interface is generic and can be ported over to other
architectures.
Suggested-by: Thomas Gleixner
Signed-off-by: Balbir Singh
Reviewed-by: Kees Cook
---
arch/x86/include/asm/thread_info.h | 7 -
arch/x86/mm/tlb.c | 44 --
include/uapi/linux/prctl.h | 4 +++
kernel/sys.c | 20 ++
4 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/thread_info.h
b/arch/x86/include/asm/thread_info.h
index 8de8ceccb8bc..67de693d9ba1 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -84,7 +84,7 @@ struct thread_info {
#define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */
#define TIF_SECCOMP8 /* secure computing */
#define TIF_SPEC_IB9 /* Indirect branch speculation
mitigation */
-#define TIF_SPEC_FORCE_UPDATE 10 /* Force speculation MSR update in
context switch */
+#define TIF_SPEC_FLUSH_L1D 10 /* Flush L1D on mm switches (processes)
*/
#define TIF_USER_RETURN_NOTIFY 11 /* notify kernel of userspace return */
#define TIF_UPROBE 12 /* breakpointed or singlestepping */
#define TIF_PATCH_PENDING 13 /* pending live patching update */
@@ -96,6 +96,7 @@ struct thread_info {
#define TIF_MEMDIE 20 /* is terminating due to OOM killer */
#define TIF_POLLING_NRFLAG 21 /* idle is polling for TIF_NEED_RESCHED
*/
#define TIF_IO_BITMAP 22 /* uses I/O bitmap */
+#define TIF_SPEC_FORCE_UPDATE 23 /* Force speculation MSR update in
context switch */
#define TIF_FORCED_TF 24 /* true if TF in eflags artificially */
#define TIF_BLOCKSTEP 25 /* set when we want DEBUGCTLMSR_BTF */
#define TIF_LAZY_MMU_UPDATES 27 /* task is updating the mmu lazily */
@@ -132,6 +133,7 @@ struct thread_info {
#define _TIF_ADDR32(1 << TIF_ADDR32)
#define _TIF_X32 (1 << TIF_X32)
#define _TIF_FSCHECK (1 << TIF_FSCHECK)
+#define _TIF_SPEC_FLUSH_L1D(1 << TIF_SPEC_FLUSH_L1D)
/* Work to do before invoking the actual syscall. */
#define _TIF_WORK_SYSCALL_ENTRY\
@@ -235,6 +237,9 @@ static inline int arch_within_stack_frames(const void *
const stack,
current_thread_info()->status & TS_COMPAT)
#endif
+extern int arch_prctl_l1d_flush_set(struct task_struct *tsk, unsigned long
enable);
+extern int arch_prctl_l1d_flush_get(struct task_struct *tsk);
+
extern void arch_task_cache_init(void);
extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct
*src);
extern void arch_release_task_struct(struct task_struct *tsk);
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 10056b8d8f01..7ea9bc9e089f 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -43,11 +44,12 @@
*/
/*
- * Bits to mangle the TIF_SPEC_IB state into the mm pointer which is
+ * Bits to mangle the TIF_SPEC_* state into the mm pointer which is
* stored in cpu_tlb_state.last_user_mm_spec.
*/
#define LAST_USER_MM_IBPB 0x1UL
-#define LAST_USER_MM_SPEC_MASK (LAST_USER_MM_IBPB)
+#define LAST_USER_MM_L1D_FLUSH 0x2UL
+#define LAST_USER_MM_SPEC_MASK (LAST_USER_MM_IBPB | LAST_USER_MM_L1D_FLUSH)
/*
* The x86 feature is called PCID (Process Context IDentifier). It is similar
@@ -308,6 +310,35 @@ void leave_mm(int cpu)
}
EXPORT_SYMBOL_GPL(leave_mm);
+static int enable_l1d_flush_for_task(struct task_struct *tsk)
+{
+ int ret = l1d_flush_init_once();
+
+ if (ret < 0)
+ return ret;
+
+ set_ti_thread_flag(>thread_info, TIF_SPEC_FLUSH_L1D);
+ return ret;
+}
+
+static int disable_l1d_flush_for_task(struct
Re: [PATCH v11 00/18] Enable FSGSBASE instructions
On 5/9/20 10:36 AM, Sasha Levin wrote: > Changes from v10: > > - Rewrite the commit message for patch #1. > - Document communication/acks from userspace projects that are >potentially affected by this. I'm glad someone's pushing this forward. But, I'm also very curious how you came to be submitting this series. Is this a team effort between you, the And[iy]s and Chang? Or, were you just trying to help out? I was hoping to see some acknowledgement of this situation in the cover letter but didn't see anything.
Re: [PATCH] tty: hvc: Fix data abort due to race in hvc_open
On 2020-05-06 02:48, Greg KH wrote:
On Mon, Apr 27, 2020 at 08:26:01PM -0700, Raghavendra Rao Ananta wrote:
Potentially, hvc_open() can be called in parallel when two tasks calls
open() on /dev/hvcX. In such a scenario, if the
hp->ops->notifier_add()
callback in the function fails, where it sets the tty->driver_data to
NULL, the parallel hvc_open() can see this NULL and cause a memory
abort.
Hence, serialize hvc_open and check if tty->private_data is NULL
before
proceeding ahead.
The issue can be easily reproduced by launching two tasks
simultaneously
that does nothing but open() and close() on /dev/hvcX.
For example:
$ ./simple_open_close /dev/hvc0 & ./simple_open_close /dev/hvc0 &
Signed-off-by: Raghavendra Rao Ananta
---
drivers/tty/hvc/hvc_console.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c
b/drivers/tty/hvc/hvc_console.c
index 436cc51c92c3..ebe26fe5ac09 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -75,6 +75,8 @@ static LIST_HEAD(hvc_structs);
*/
static DEFINE_MUTEX(hvc_structs_mutex);
+/* Mutex to serialize hvc_open */
+static DEFINE_MUTEX(hvc_open_mutex);
/*
* This value is used to assign a tty->index value to a hvc_struct
based
* upon order of exposure via hvc_probe(), when we can not match it
to
@@ -346,16 +348,24 @@ static int hvc_install(struct tty_driver
*driver, struct tty_struct *tty)
*/
static int hvc_open(struct tty_struct *tty, struct file * filp)
{
- struct hvc_struct *hp = tty->driver_data;
+ struct hvc_struct *hp;
unsigned long flags;
int rc = 0;
+ mutex_lock(_open_mutex);
+
+ hp = tty->driver_data;
+ if (!hp) {
+ rc = -EIO;
+ goto out;
+ }
+
spin_lock_irqsave(>port.lock, flags);
/* Check and then increment for fast path open. */
if (hp->port.count++ > 0) {
spin_unlock_irqrestore(>port.lock, flags);
hvc_kick();
- return 0;
+ goto out;
} /* else count == 0 */
spin_unlock_irqrestore(>port.lock, flags);
Wait, why isn't this driver just calling tty_port_open() instead of
trying to open-code all of this?
Keeping a single mutext for open will not protect it from close, it
will
just slow things down a bit. There should already be a tty lock held
by
the tty core for open() to keep it from racing things, right?
The tty lock should have been held, but not likely across ->install()
and
->open() callbacks, thus resulting in a race between hvc_install() and
hvc_open(),
where hvc_install() sets a data and the hvc_open() clears it. hvc_open()
doesn't
check if the data was set to NULL and proceeds.
Try just removing all of this logic and replacing it with a call to
tty_port_open() and see if that fixes this issue.
As "proof" of this, I don't see other serial drivers needing a single
mutex for their open calls, do you?
thanks,
greg k-h
Thank you.
Raghavendra
Re: [PATCH] drm/mediatek: eliminate the magic number in array size
Hi, Bernard:
Bernard Zhao 於 2020年5月6日 週三 下午8:43寫道：
>
> Eiminate the magic number in array size, there macro defines in
> hdmi.h.
Applied to mediatek-drm-next [1], thanks.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next
Regards,
Chun-Kuang
>
> Signed-off-by: Bernard Zhao
> ---
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index ff43a3d80410..4c962c7f06e5 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -982,7 +982,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi
> *hdmi,
> struct drm_display_mode *mode)
> {
> struct hdmi_avi_infoframe frame;
> - u8 buffer[17];
> + u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
> ssize_t err;
>
> err = drm_hdmi_avi_infoframe_from_display_mode(,
> @@ -1008,7 +1008,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi
> *hdmi,
> const char *product)
> {
> struct hdmi_spd_infoframe frame;
> - u8 buffer[29];
> + u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_SPD_INFOFRAME_SIZE];
> ssize_t err;
>
> err = hdmi_spd_infoframe_init(, vendor, product);
> @@ -1031,7 +1031,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi
> *hdmi,
> static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
> {
> struct hdmi_audio_infoframe frame;
> - u8 buffer[14];
> + u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
> ssize_t err;
>
> err = hdmi_audio_infoframe_init();
> --
> 2.26.2
>
Re: [PATCH v2] drm/mediatek: cleanup coding style in mediatek a bit
Hi, Bernard: Bernard Zhao 於 2020年5月6日 週三 下午8:34寫道： > > This code change is to make code bit more readable. > Applied to mediatek-drm-next [1], thanks. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next Regards, Chun-Kuang > Signed-off-by: Bernard Zhao > --- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c > b/drivers/gpu/drm/mediatek/mtk_hdmi.c > index ff43a3d80410..43e9876fd50c 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c > @@ -311,14 +311,10 @@ static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi > *hdmi, u8 *buffer, > u8 checksum; > int ctrl_frame_en = 0; > > - frame_type = *buffer; > - buffer += 1; > - frame_ver = *buffer; > - buffer += 1; > - frame_len = *buffer; > - buffer += 1; > - checksum = *buffer; > - buffer += 1; > + frame_type = *buffer++; > + frame_ver = *buffer++; > + frame_len = *buffer++; > + checksum = *buffer++; > frame_data = buffer; > > dev_dbg(hdmi->dev, > -- > 2.26.2 >
Re: [PATCH 0/3] Convert mtk-dpi to drm_bridge API
Hi, Enric: Enric Balletbo i Serra 於 2020年5月4日 週一 下午10:14寫道： > > The mtk-dpi driver still uses the drm_encoder API which is now somehow > deprecated. We started to move all the Mediatek drivers to the drm_bridge API, > like we did for the mtk-dsi driver [1], this is another small step to be able > to > fully convert the DRM Mediatek drivers to the drm_bridge API. A dummy > drm_encoder is maintained in the mtk-dpi driver but the end goal is move all > the > dummy drm_encoder (mtk-dsi, mtk-dpi, etc) to the main mtk_drm_drv driver. For this series, applied to mediatek-drm-next [1], thanks. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next Regards, Chun-Kuang. > > Best regards, > Enric > > [1] https://lore.kernel.org/patchwork/project/lkml/list/?series=441559 > > Enric Balletbo i Serra (3): > drm/mediatek: mtk_dpi: Rename bridge to next_bridge > drm/mediatek: mtk_dpi: Convert to bridge driver > drm/mediatek: mtk_dpi: Use simple encoder > > drivers/gpu/drm/mediatek/mtk_dpi.c | 84 ++ > 1 file changed, 39 insertions(+), 45 deletions(-) > > -- > 2.26.2 >
Re: [PATCH] parisc: suppress error messages for 'make clean'
Hi Helge, On Sun, May 10, 2020 at 2:39 AM Helge Deller wrote: > > * Masahiro Yamada : > > Hi Helge, > > > > On Sat, May 9, 2020 at 6:46 AM Helge Deller wrote: > > > > > > * Masahiro Yamada : > > > > On Sat, Apr 25, 2020 at 2:47 PM Masahiro Yamada > > > > wrote: > > > > > > > > > > 'make ARCH=parisc clean' emits a tons of error messages as follows: > > > > > > > > > > $ make ARCH=parisc clean > > > > > gcc: error: unrecognized command line option '-mno-space-regs' > > > > > gcc: error: unrecognized command line option > > > > > '-mfast-indirect-calls'; did you mean '-mforce-indirect-call'? > > > > > gcc: error: unrecognized command line option '-mdisable-fpregs' > > > > > gcc: error: missing argument to '-Wframe-larger-than=' > > > > > gcc: error: unrecognized command line option '-mno-space-regs' > > > > > gcc: error: unrecognized command line option > > > > > '-mfast-indirect-calls'; did you mean '-mforce-indirect-call'? > > > > > gcc: error: unrecognized command line option '-mdisable-fpregs' > > > > > gcc: error: missing argument to '-Wframe-larger-than=' > > > > > ... > > > > > > > > > > You can supporess them except '-Wframe-larger-than' by setting correct > > > > > CROSS_COMPILE=, but we should not require any compiler for cleaning. > > > > > > > > > > This $(shell ...) is evaluated so many times because LIBGCC is > > > > > exported. > > > > > Use the ':=' operator to evaluate it just once, and sink the stderr. > > > > > > > > > > > > > Applied to linux-kbuild. > > > > > > That patch breaks then building the boot loader/compressor: > > > ... > > > hppa-linux-gnu-ld-X -e startup --as-needed -T > > > arch/parisc/boot/compressed/vmlinux.lds > > > arch/parisc/boot/compressed/head.o arch/parisc/boot/compressed/real2.o > > > arch/parisc/boot/compressed/firmware.o arch/parisc/boot/compressed/misc.o > > > arch/parisc/boot/compressed/piggy.o -o arch/parisc/boot/compressed/vmlinux > > > hppa-linux-gnu-ld: arch/parisc/boot/compressed/misc.o: in function > > > `dec_vli': > > > (.text+0x104): undefined reference to `__ashldi3' > > > hppa-linux-gnu-ld: arch/parisc/boot/compressed/misc.o: in function > > > `lzma_len': > > > (.text+0x2b0): undefined reference to `$$mulI' > > > hppa-linux-gnu-ld: (.text+0x344): undefined reference to `$$mulI' > > > hppa-linux-gnu-ld: (.text+0x3f8): undefined reference to `$$mulI' > > > > > > > > > The patch below works, but I wonder if it's possible to avoid > > > to examine LIBGCC twice? > > > > > > Helge > > > > > > Sorry for the breakage. > > > > How about moving LIBGCC below ? > > Good idea. > The patch below does work for me. > We do not need $KBUILD_CFLAGS to get the libgcc.a filename, I not not sure about this change. Generally speaking, the path to libgcc.a is affected by compiler flags, especially, bit size flags, endian flags, etc. For example, my distro gcc is biarch for i386/x86_64. $ gcc -print-libgcc-file-name /usr/lib/gcc/x86_64-linux-gnu/9/libgcc.a $ gcc -m64 -print-libgcc-file-name /usr/lib/gcc/x86_64-linux-gnu/9/libgcc.a $ gcc -m32 -print-libgcc-file-name /usr/lib/gcc/x86_64-linux-gnu/9/32/libgcc.a One real example in Linux is arch/arc/Makefile. ARC supports both big endian and little endian. If you drop cflags-y from line 59 of arch/arc/Makefile, vmlinux fails to link due to endian mismatch. I am not familiar with parisc. Also, as it turned out, this change is not so trivial. I think the best approach is to leave this up to you since you are the parisc maintainer. I will drop this patch from my kbuild tree, then you will apply what you think is best to your tree. What do you think? > so we do not need to pipe the output to /dev/null either. > Can you try if that works, and if yes, can you apply it? > > Helge > > > diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile > index 628cd8bb7ad8..fadbbd010337 100644 > --- a/arch/parisc/Makefile > +++ b/arch/parisc/Makefile > @@ -21,8 +21,6 @@ KBUILD_IMAGE := vmlinuz > > NM = sh $(srctree)/arch/parisc/nm > CHECKFLAGS += -D__hppa__=1 > -LIBGCC = $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) > -export LIBGCC > > ifdef CONFIG_64BIT > UTS_MACHINE:= parisc64 > @@ -110,6 +108,8 @@ cflags-$(CONFIG_PA8X00) += -march=2.0 > -mschedule=8000 > head-y := arch/parisc/kernel/head.o > > KBUILD_CFLAGS += $(cflags-y) > +LIBGCC := $(shell $(CC) -print-libgcc-file-name) > +export LIBGCC > > kernel-y := mm/ kernel/ math-emu/ > -- Best Regards Masahiro Yamada
Re: [RFC PATCH 0/7] mm: Get rid of vmalloc_sync_(un)mappings()
On Sat, May 09, 2020 at 11:25:16AM +0200, Peter Zijlstra wrote: > On Fri, May 08, 2020 at 11:34:07PM +0200, Joerg Roedel wrote: > > On Fri, May 08, 2020 at 09:20:00PM +0200, Peter Zijlstra wrote: > > > The only concern I have is the pgd_lock lock hold times. > > > > > > By not doing on-demand faults anymore, and consistently calling > > > sync_global_*(), we iterate that pgd_list thing much more often than > > > before if I'm not mistaken. > > > > Should not be a problem, from what I have seen this function is not > > called often on x86-64. The vmalloc area needs to be synchronized at > > the top-level there, which is by now P4D (with 4-level paging). And the > > vmalloc area takes 64 entries, when all of them are populated the > > function will not be called again. > > Right; it's just that the moment you do trigger it, it'll iterate that > pgd_list and that is potentially huge. Then again, that's not a new > problem. > > I suppose we can deal with it if/when it becomes an actual problem. > > I had a quick look and I think it might be possible to make it an RCU > managed list. We'd have to remove the pgd_list entry in > put_task_struct_rcu_user(). Then we can play games in sync_global_*() > and use RCU iteration. New tasks (which can be missed in the RCU > iteration) will already have a full clone of the PGD anyway. One of the things on my long-term todo list is to replace mm_struct.mmlist with an XArray containing all mm_structs. Then we can use one mark to indicate maybe-swapped and another mark to indicate ... whatever it is pgd_list indicates. Iterating an XArray (whether the entire thing or with marks) is RCU-safe and faster than iterating a linked list, so this should solve the problem?
Re: [PATCH v4 4/5] blktrace: break out of blktrace setup on concurrent calls
On 2020-05-08 20:10, Luis Chamberlain wrote:
> @@ -493,6 +496,12 @@ static int do_blk_trace_setup(struct request_queue *q,
> char *name, dev_t dev,
>*/
> strreplace(buts->name, '/', '_');
>
> + if (q->blk_trace) {
> + pr_warn("Concurrent blktraces are not allowed on %s\n",
> + buts->name);
> + return -EBUSY;
> + }
> +
> bt = kzalloc(sizeof(*bt), GFP_KERNEL);
> if (!bt)
> return -ENOMEM;
Is this really sufficient? Shouldn't concurrent do_blk_trace_setup()
calls that refer to the same request queue be serialized to really
prevent that debugfs attribute creation fails?
How about using the block device name instead of the partition name in
the error message since the concurrency context is the block device and
not the partition?
Thanks,
Bart.
Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
On 2020/05/10 9:57, Linus Torvalds wrote: > On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa > wrote: >> >> I think that this access_ok() check helps reducing partial writes (either >> "whole amount was processed" or "not processed at all" unless -ENOMEM). > > No it doesn't. > > "access_ok()" only checks the range being a valid user address range. > I see. Thank you. Acked-by: Tetsuo Handa
[PATCH v5] streamline_config.pl: add LMC_KEEP to preserve some kconfigs
Sometimes it is useful to preserve batches of configs when making
localmodconfig. For example, I usually don't want any usb and fs
modules to be disabled. Now we can do it by:
$ make LMC_KEEP="drivers/usb:fs" localmodconfig
Signed-off-by: Changbin Du
---
v4: fix typo.
v3: rename LOCALMODCONFIG_PRESERVE to shorter LMC_KEEP.
v2: fix typo in documentation. (Randy Dunlap)
---
Documentation/admin-guide/README.rst | 8 +++-
scripts/kconfig/Makefile | 1 +
scripts/kconfig/streamline_config.pl | 21 +
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/README.rst
b/Documentation/admin-guide/README.rst
index cc6151fc0845..407aa206bb70 100644
--- a/Documentation/admin-guide/README.rst
+++ b/Documentation/admin-guide/README.rst
@@ -209,10 +209,16 @@ Configuring the kernel
store the lsmod of that machine into a file
and pass it in as a LSMOD parameter.
+ Also, you can preserve modules in certain folders
+ or kconfig files by specifying their paths in
+ parameter LMC_KEEP.
+
target$ lsmod > /tmp/mylsmod
target$ scp /tmp/mylsmod host:/tmp
- host$ make LSMOD=/tmp/mylsmod localmodconfig
+ host$ make LSMOD=/tmp/mylsmod \
+ LMC_KEEP="drivers/usb:drivers/gpu:fs" \
+ localmodconfig
The above also works when cross compiling.
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index c9d0a4a8efb3..e0abbf5805f5 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -123,6 +123,7 @@ help:
@echo ' gconfig - Update current config utilising a GTK+
based front-end'
@echo ' oldconfig - Update current config utilising a provided
.config as base'
@echo ' localmodconfig - Update current config disabling modules not
loaded'
+ @echo 'except those preserved by LMC_KEEP
environment variable'
@echo ' localyesconfig - Update current config converting local mods
to core'
@echo ' defconfig - New config with default from ARCH supplied
defconfig'
@echo ' savedefconfig - Save current config as ./defconfig (minimal
config)'
diff --git a/scripts/kconfig/streamline_config.pl
b/scripts/kconfig/streamline_config.pl
index e2f8504f5a2d..19857d18d814 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -143,6 +143,7 @@ my %depends;
my %selects;
my %prompts;
my %objects;
+my %config2kfile;
my $var;
my $iflevel = 0;
my @ifdeps;
@@ -201,6 +202,7 @@ sub read_kconfig {
if (/^\s*(menu)?config\s+(\S+)\s*$/) {
$state = "NEW";
$config = $2;
+ $config2kfile{"CONFIG_$config"} = $kconfig;
# Add depends for 'if' nesting
for (my $i = 0; $i < $iflevel; $i++) {
@@ -591,6 +593,20 @@ while ($repeat) {
}
my %setconfigs;
+my @preserved_kconfigs = split(/:/,$ENV{LMC_KEEP});
+
+sub in_preserved_kconfigs {
+my $kconfig = $config2kfile{$_[0]};
+if (!defined($kconfig)) {
+return 0;
+}
+foreach my $excl (@preserved_kconfigs) {
+if($kconfig =~ /^$excl/) {
+return 1;
+}
+}
+return 0;
+}
# Finally, read the .config file and turn off any module enabled that
# we could not find a reason to keep enabled.
@@ -644,6 +660,11 @@ foreach my $line (@config_file) {
}
if (/^(CONFIG.*)=(m|y)/) {
+if (in_preserved_kconfigs($1)) {
+dprint "Preserve config $1";
+print;
+next;
+}
if (defined($configs{$1})) {
if ($localyesconfig) {
$setconfigs{$1} = 'y';
--
2.25.1
Re: [PATCH] bpfilter: check if $(CC) can static link in Kconfig
On Sat, May 9, 2020 at 12:40 AM Masahiro Yamada wrote: > > On Fedora, linking static libraries requires the glibc-static RPM > package, which is not part of the glibc-devel package. > > CONFIG_CC_CAN_LINK does not check the capability of static linking, > so you can enable CONFIG_BPFILTER_UMH, then fail to build. > > HOSTLD net/bpfilter/bpfilter_umh > /usr/bin/ld: cannot find -lc > collect2: error: ld returned 1 exit status > > Add CONFIG_CC_CAN_LINK_STATIC, and make CONFIG_BPFILTER_UMH depend > on it. > > Reported-by: Valdis Kletnieks > Signed-off-by: Masahiro Yamada Thanks! Acked-by: Alexei Starovoitov
Re: [PATCH 00/15] net: taint when the device driver firmware crashes
On 5/8/20 9:35 PM, Luis Chamberlain wrote: Device driver firmware can crash, and sometimes, this can leave your system in a state which makes the device or subsystem completely useless. Detecting this by inspecting /proc/sys/kernel/tainted instead of scraping some magical words from the kernel log, which is driver specific, is much easier. So instead this series provides a helper which lets drivers annotate this and shows how to use this on networking drivers. If the driver is able to detect that the device firmware has come back alive, through user intervention or whatever, should there be a way to "untaint" the kernel? Or would you expect it to remain tainted? sln
Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa wrote: > > I think that this access_ok() check helps reducing partial writes (either > "whole amount was processed" or "not processed at all" unless -ENOMEM). No it doesn't. "access_ok()" only checks the range being a valid user address range. It doesn't actually help at all if the worry is "what if we take a page fault in the middle". Because it simply doesn't check those kinds of things. Now, if somebody passes actual invalid ranges (ie kernel addresses or other crazy stuff), they only have themselves to blame. The invalid range will be noticed when actually doing the user copy, and then you'll get EFAULT there. But there's no point in trying to figure that out early - it's only adding overhead, and it doesn't help any normal case. Linus
Re: [PATCH v4 3/5] blktrace: fix debugfs use after free
On 2020-05-08 20:10, Luis Chamberlain wrote:
> Screenshots of what the debugfs for block looks like after running
> blktrace on a system with sg0 which has a raid controllerand then sg1
> as the media changer:
>
> # ls -l /sys/kernel/debug/block
> total 0
> drwxr-xr-x 3 root root 0 May 9 02:31 bsg
> drwxr-xr-x 19 root root 0 May 9 02:31 nvme0n1
> drwxr-xr-x 19 root root 0 May 9 02:31 nvme1n1
> lrwxrwxrwx 1 root root 0 May 9 02:31 nvme1n1p1 -> nvme1n1
> lrwxrwxrwx 1 root root 0 May 9 02:31 nvme1n1p2 -> nvme1n1
> lrwxrwxrwx 1 root root 0 May 9 02:31 nvme1n1p3 -> nvme1n1
> lrwxrwxrwx 1 root root 0 May 9 02:31 nvme1n1p5 -> nvme1n1
> lrwxrwxrwx 1 root root 0 May 9 02:31 nvme1n1p6 -> nvme1n1
> drwxr-xr-x 2 root root 0 May 9 02:33 sch0
> lrwxrwxrwx 1 root root 0 May 9 02:33 sg0 -> bsg/2:0:0:0
> lrwxrwxrwx 1 root root 0 May 9 02:33 sg1 -> sch0
> drwxr-xr-x 5 root root 0 May 9 02:31 vda
> lrwxrwxrwx 1 root root 0 May 9 02:31 vda1 -> vda
So this patch creates one soft link per partition at partition creation
time instead of letting the blktrace code create one directory per
partition when tracing starts? Does this break running blktrace
simultaneously for a partition and for the entire block device?
> +static struct dentry *queue_debugfs_symlink_type(struct request_queue *q,
> + const char *src,
> + const char *dst,
> + enum blk_debugfs_dir_type type)
> +{
> + struct dentry *dentry = ERR_PTR(-EINVAL);
> + char *dir_dst;
> +
> + dir_dst = kzalloc(PATH_MAX, GFP_KERNEL);
> + if (!dir_dst)
> + return dentry;
> +
> + switch (type) {
> + case BLK_DBG_DIR_BASE:
> + if (dst)
> + snprintf(dir_dst, PATH_MAX, "%s", dst);
> + else if (!IS_ERR_OR_NULL(q->debugfs_dir))
> + snprintf(dir_dst, PATH_MAX, "%s",
> + q->debugfs_dir->d_name.name);
> + else
> + goto out;
> + break;
> + case BLK_DBG_DIR_BSG:
> + if (dst)
> + snprintf(dir_dst, PATH_MAX, "bsg/%s", dst);
> + else
> + goto out;
> + break;
> + }
> +
> + /*
> + * The base block debugfs directory is always used for the symlinks,
> + * their target is what changes.
> + */
> + dentry = debugfs_create_symlink(src, blk_debugfs_root, dir_dst);
> +out:
> + kfree(dir_dst);
> +
> + return dentry;
> +}
Please use kasprintf() instead of k?alloc() followed by snprintf().
Thanks,
Bart.
Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
Hello, Al. I think that this access_ok() check helps reducing partial writes (either "whole amount was processed" or "not processed at all" unless -ENOMEM). Do you think that such attempt is pointless? Then, please go ahead... On 2020/05/10 8:45, Al Viro wrote: > From: Al Viro > > address is passed only to get_user() > > Signed-off-by: Al Viro > --- > security/tomoyo/common.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c > index 1b467381986f..f93f8acd05f7 100644 > --- a/security/tomoyo/common.c > +++ b/security/tomoyo/common.c > @@ -2662,8 +2662,6 @@ ssize_t tomoyo_write_control(struct tomoyo_io_buffer > *head, > > if (!head->write) > return -EINVAL; > - if (!access_ok(buffer, buffer_len)) > - return -EFAULT; > if (mutex_lock_interruptible(>io_sem)) > return -EINTR; > head->read_user_buf_avail = 0; >
Re: [PATCH v11 12/18] x86/fsgsbase/64: move save_fsgs to header file
Hi Sasha,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/x86/asm]
[also build test ERROR on tip/auto-latest linus/master tip/x86/core v5.7-rc4
next-20200508]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Sasha-Levin/Enable-FSGSBASE-instructions/20200510-032805
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
2ce0d7f9766f0e49bb54f149c77bae89464932fb
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
In file included from arch/x86/include/uapi/asm/ptrace.h:6:0,
from arch/x86/include/asm/ptrace.h:7,
from arch/x86/include/asm/math_emu.h:5,
from arch/x86/include/asm/processor.h:13,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from arch/x86/kernel/process.c:6:
>> arch/x86/include/uapi/asm/ptrace-abi.h:16:12: error: expected identifier
>> before numeric constant
#define FS 9
^
>> arch/x86/kernel/process.h:42:2: note: in expansion of macro 'FS'
FS,
^~
In file included from arch/x86/kernel/process.c:46:0:
arch/x86/kernel/process.h: In function 'save_base_legacy':
>> arch/x86/kernel/process.h:85:18: error: 'struct thread_struct' has no member
>> named 'fsbase'
prev_p->thread.fsbase = 0;
^
>> arch/x86/kernel/process.h:87:18: error: 'struct thread_struct' has no member
>> named 'gsbase'
prev_p->thread.gsbase = 0;
^
In file included from arch/x86/include/asm/ptrace.h:5:0,
from arch/x86/include/asm/math_emu.h:5,
from arch/x86/include/asm/processor.h:13,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from arch/x86/kernel/process.c:6:
arch/x86/kernel/process.h: In function 'save_fsgs':
>> arch/x86/kernel/process.h:93:30: error: 'struct thread_struct' has no member
>> named 'fsindex'
savesegment(fs, task->thread.fsindex);
^
arch/x86/include/asm/segment.h:368:32: note: in definition of macro
'savesegment'
asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
^
>> arch/x86/kernel/process.h:94:30: error: 'struct thread_struct' has no member
>> named 'gsindex'
savesegment(gs, task->thread.gsindex);
^
arch/x86/include/asm/segment.h:368:32: note: in definition of macro
'savesegment'
asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
^
In file included from arch/x86/kernel/process.c:46:0:
arch/x86/kernel/process.h:101:15: error: 'struct thread_struct' has no
member named 'fsbase'
task->thread.fsbase = rdfsbase();
^
>> arch/x86/kernel/process.h:101:25: error: implicit declaration of function
>> 'rdfsbase'; did you mean 'rb_erase'? [-Werror=implicit-function-declaration]
task->thread.fsbase = rdfsbase();
^~~~
rb_erase
arch/x86/kernel/process.h:102:15: error: 'struct thread_struct' has no
member named 'gsbase'
task->thread.gsbase = x86_gsbase_read_cpu_inactive();
^
>> arch/x86/kernel/process.h:102:25: error: implicit declaration of function
>> 'x86_gsbase_read_cpu_inactive' [-Werror=implicit-function-declaration]
task->thread.gsbase = x86_gsbase_read_cpu_inactive();
^~~~
arch/x86/kernel/process.h:104:38: error: 'struct thread_struct' has no
member named 'fsindex'
save_base_legacy(task,
My Dear in the lord
My Dear in the lord My name is Mrs. Mina A. Brunel I am a Norway Citizen who is living in Burkina Faso, I am married to Mr. Brunel Patrice, a politicians who owns a small gold company in Burkina Faso; He died of Leprosy and Radesyge, in year February 2010, During his lifetime he deposited the sum of € 8.5 Million Euro) Eight million, Five hundred thousand Euros in a bank in Ouagadougou the capital city of of Burkina in West Africa. The money was from the sale of his company and death benefits payment and entitlements of my deceased husband by his company. I am sending you this message with heavy tears in my eyes and great sorrow in my heart, and also praying that it will reach you in good health because I am not in good health, I sleep every night without knowing if I may be alive to see the next day. I am suffering from long time cancer and presently I am partially suffering from Leprosy, which has become difficult for me to move around. I was married to my late husband for more than 6 years without having a child and my doctor confided that I have less chance to live, having to know when the cup of death will come, I decided to contact you to claim the fund since I don't have any relation I grew up from an orphanage home. I have decided to donate this money for the support of helping Motherless babies/Less privileged/Widows and churches also to build the house of God because I am dying and diagnosed with cancer for about 3 years ago. I have decided to donate from what I have inherited from my late husband to you for the good work of Almighty God; I will be going in for an operation surgery soon. Now I want you to stand as my next of kin to claim the funds for charity purposes. Because of this money remains unclaimed after my death, the bank executives or the government will take the money as unclaimed fund and maybe use it for selfishness and worthless ventures, I need a very honest person who can claim this money and use it for Charity works, for orphanages, widows and also build schools and churches for less privilege that will be named after my late husband and my name. I need your urgent answer to know if you will be able to execute this project, and I will give you more information on how the fund will be transferred to your bank account or online banking. Thanks Mrs. Mina A. Brunel
Re: [PATCH v4 1/5] block: revert back to synchronous request_queue removal
On 2020-05-08 20:10, Luis Chamberlain wrote: > We revert back to synchronous request_queue removal because asynchronous > removal creates a regression with expected userspace interaction with > several drivers. An example is when removing the loopback driver, one > uses ioctls from userspace to do so, but upon return and if successful, > one expects the device to be removed. Likewise if one races to add another > device the new one may not be added as it is still being removed. This was > expected behavior before and it now fails as the device is still present > and busy still. Moving to asynchronous request_queue removal could have > broken many scripts which relied on the removal to have been completed if > there was no error. Document this expectation as well so that this > doesn't regress userspace again. Reviewed-by: Bart Van Assche
Re: [PATCHES] uaccess simple access_ok() removals
On Sat, May 9, 2020 at 4:41 PM Al Viro wrote: > > Individual patches in followups; if nobody screams - into #for-next > it goes... Looks fine to me, although I only read your commit logs, I didn't verify that what you stated was actually true (ie the whole "only used for xyz" parts). But I'll take your word for it. Particularly the double-underscore versions are getting rare (and presumably some of the other branches you have make it rarer still). Linus
Re: [PATCH v12 00/11] Support ROHM BD99954 charger IC
Hi, On Fri, May 08, 2020 at 06:20:24PM +0100, Mark Brown wrote: > On Fri, May 08, 2020 at 06:38:17PM +0300, Matti Vaittinen wrote: > > Please note that this series should be applied to two trees. Patches > > 1-4 (or 1-5 as suggested by Sebastian) should go to regulator tree. > > Perhaps Mark can provide an immutable branch to Sebastian? Rest of the > > patches can then go to power-supply tree. > > The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: > > Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) > > are available in the Git repository at: > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git > tags/linear-ranges-lib > > for you to fetch changes up to 60ab7f4153b6af461c90d572c31104086b44639f: > > regulator: use linear_ranges helper (2020-05-08 18:18:13 +0100) > > > lib: Add linear ranges helper library and start using it > > Series extracts a "linear ranges" helper out of the regulator > framework. Linear ranges helper is intended to help converting > real-world values to register values when conversion is linear. I > suspect this is useful also for power subsystem and possibly for clk. > > Thanks, merged to power-supply's for-next branch. -- Sebastian > Matti Vaittinen (4): > lib: add linear ranges helpers > lib/test_linear_ranges: add a test for the 'linear_ranges' > power: supply: bd70528: rename linear_range to avoid collision > regulator: use linear_ranges helper > > drivers/power/supply/bd70528-charger.c | 10 +- > drivers/regulator/88pg86x.c | 4 +- > drivers/regulator/88pm800-regulator.c | 4 +- > drivers/regulator/Kconfig | 1 + > drivers/regulator/act8865-regulator.c | 4 +- > drivers/regulator/act8945a-regulator.c | 2 +- > drivers/regulator/arizona-ldo1.c| 2 +- > drivers/regulator/arizona-micsupp.c | 4 +- > drivers/regulator/as3711-regulator.c| 6 +- > drivers/regulator/as3722-regulator.c| 4 +- > drivers/regulator/axp20x-regulator.c| 16 +-- > drivers/regulator/bcm590xx-regulator.c | 8 +- > drivers/regulator/bd70528-regulator.c | 8 +- > drivers/regulator/bd71828-regulator.c | 10 +- > drivers/regulator/bd718x7-regulator.c | 26 ++-- > drivers/regulator/da903x.c | 2 +- > drivers/regulator/helpers.c | 130 - > drivers/regulator/hi6421-regulator.c| 4 +- > drivers/regulator/lochnagar-regulator.c | 4 +- > drivers/regulator/lp873x-regulator.c| 4 +- > drivers/regulator/lp87565-regulator.c | 2 +- > drivers/regulator/lp8788-buck.c | 2 +- > drivers/regulator/max77650-regulator.c | 2 +- > drivers/regulator/mcp16502.c| 4 +- > drivers/regulator/mp8859.c | 2 +- > drivers/regulator/mt6323-regulator.c| 6 +- > drivers/regulator/mt6358-regulator.c| 8 +- > drivers/regulator/mt6380-regulator.c| 6 +- > drivers/regulator/mt6397-regulator.c| 6 +- > drivers/regulator/palmas-regulator.c| 4 +- > drivers/regulator/qcom-rpmh-regulator.c | 2 +- > drivers/regulator/qcom_rpm-regulator.c | 14 +- > drivers/regulator/qcom_smd-regulator.c | 78 +-- > drivers/regulator/rk808-regulator.c | 10 +- > drivers/regulator/s2mps11.c | 14 +- > drivers/regulator/sky81452-regulator.c | 2 +- > drivers/regulator/stpmic1_regulator.c | 18 +-- > drivers/regulator/tps65086-regulator.c | 10 +- > drivers/regulator/tps65217-regulator.c | 4 +- > drivers/regulator/tps65218-regulator.c | 6 +- > drivers/regulator/tps65912-regulator.c | 4 +- > drivers/regulator/twl-regulator.c | 4 +- > drivers/regulator/twl6030-regulator.c | 2 +- > drivers/regulator/wm831x-dcdc.c | 2 +- > drivers/regulator/wm831x-ldo.c | 4 +- > drivers/regulator/wm8350-regulator.c| 2 +- > drivers/regulator/wm8400-regulator.c| 2 +- > include/linux/linear_range.h| 48 +++ > include/linux/regulator/driver.h| 27 +--- > lib/Kconfig | 3 + > lib/Kconfig.debug | 12 ++ > lib/Makefile| 2 + > lib/linear_ranges.c | 241 > > lib/test_linear_ranges.c| 228 ++ > 54 files changed, 768 insertions(+), 266 deletions(-) > create mode 100644 include/linux/linear_range.h > create mode 100644 lib/linear_ranges.c > create mode 100644 lib/test_linear_ranges.c signature.asc Description: PGP signature
[rcu:rcu/next] BUILD SUCCESS 373b78add5ef0c6fd1154304208252dbf0309114
cfc49 rcu: Expedited grace-period sleeps to idle priority 0 0 0 0 825613e73129 rcu-tasks: Convert sleeps to idle priority 0 0 0 0 373b78add5ef fs/btrfs: Add cond_resched() for try_release_extent_mapping( -92 -93+136 +83 ae83d0b416db..373b78add5ef (ALL COMMITS) == elapsed time: 483m configs tested: 104 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig sparcallyesconfig m68k allyesconfig i386 allnoconfig i386defconfig i386 debian-10.3 i386 allyesconfig ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig h8300allmodconfig xtensa defconfig arc defconfig arc allyesconfig microblaze allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allyesconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allyesconfig parisc allmodconfig powerpc defconfig powerpc allyesconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a006-20200510 i386 randconfig-a005-20200510 i386 randconfig-a003-20200510 i386 randconfig-a001-20200510 i386 randconfig-a004-20200510 i386 randconfig-a002-20200510 x86_64 randconfig-a015-20200509 x86_64 randconfig-a014-20200509 x86_64 randconfig-a011-20200509 x86_64 randconfig-a013-20200509 x86_64 randconfig-a012-20200509 x86_64 randconfig-a016-20200509 x86_64 randconfig-a016-20200510 x86_64 randconfig-a012-20200510 x86_64 randconfig-a015-20200510 x86_64 randconfig-a013-20200510 x86_64 randconfig-a014-20200510 x86_64 randconfig-a011-20200510 i386 randconfig-a012-20200510 i386 randconfig-a016-20200510 i386 randconfig-a014-20200510 i386 randconfig-a011-20200510 i386 randconfig-a013-20200510 i386 randconfig-a015-20200510 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig s390 allnoconfig s390 allyesconfig s390 allmodconfig s390defconfig sparc defconfig sparc64 allnoconf
Re: [PATCH 2/2] spi: Add Baikal-T1 System Boot SPI Controller driver
On Fri, May 08, 2020 at 12:37:51PM +0100, Mark Brown wrote:
> On Fri, May 08, 2020 at 12:36:21PM +0300, Serge Semin wrote:
>
> Your CC list on this series is *very* wide - are you sure that this is
> relevant to everyone you're sending things to? Kernel developers often
> get a lot of mail meaning that extra mail can become a bit overwhelming
> and cause things to get lost in the noise.
MIPS folks are here since Baikal-T1 is a MIPS-based chip and this patchset
is a part of the campaign of integrating the SoC support into the kernel.
With Lee and Miquel we discussed the dirmap support in the framework of another
patchset. Rob and devicetree-list are CC'ed due to having the bindings tree
updated. Then a series of folks who recently submitted the biggest updates into
the spi subsystems so might have valuable comments for this driver as well. So
yes, I am sure.
>
> > This SPI-controller is a part of the Baikal-T1 System Controller and
> > is based on the DW APB SSI IP-core, but with very limited resources:
> > no IRQ, no DMA, only a single native chip-select and just 8 bytes Tx/Rx
> > FIFO available. In order to provide a transparent initial boot code
> > execution this controller is also utilized by an vendor-specific block,
> > which provides an CS0 SPI flash direct mapping interface. Since both
> > direct mapping and SPI controller normal utilization are mutual exclusive
> > only a one of these interfaces can be used to access an external SPI
> > slave device. Taking into account the peculiarities of the controller
> > registers and physically mapped SPI flash access, very limited resources
> > and seeing the normal usecase of the controller is to access an external
> > SPI-nor flash, we decided to create a dedicated SPI driver for it.
>
> My overall impression reading this is that there could be a lot more
> reliance on both generic functionality and as Andy suggested the spi-dw
> driver - the headers seem easy for example. As far as I can tell the
> main things this is adding compared to spi-dw are the dirmap code and
> the IRQ disabling around the PIO on the FIFO both of which seem like
> relatively small additions which it should be possible to accomodate
> within spi-dw. For example the driver could have multiple transfer
> functions and pick a different one with the interrupt disabling when
> running on this hardware.
dirmap and IRQs disabling isn't all they have different. Please see my answer to
you in the cover-letter thread and the comments below in this email.
>
> > --- /dev/null
> > +++ b/drivers/spi/spi-bt1-sys.c
> > @@ -0,0 +1,873 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
>
> Please make the entire comment a C++ one so things look a bit cleaner
> and more intentional.
This has been unexpected. It's first time I see such a requirement.
Anyway ok. I'll fix it. Is it ok to have the C-style comments in the header
file?
* It isn't included by any assembly so from this point of view C++ style
* shall also work there.
>
> > + writel(BIT(req->cs), bs->regs + BC_SPI_SER);
> > + if (req->cs_gpiod) {
> > + gpiod_set_value_cansleep(req->cs_gpiod,
> > +!!(bs->cfg.mode & SPI_CS_HIGH));
>
> If you have a GPIO chip select you should just let the core manage it
> through cs_gpiod rather than open coding.
Of course I know this, and normally I would have omitted the GPIO manual
assertion (hopefully soon my hands get to merging the AX99100 driver I've
developed some time ago). The thing is that this Baikal-T1 System SSI device
driver has been initially written before commit 05766050d5bd ("spi: spi-mem:
fallback to using transfers when CS gpios are used"). So asserting GPIO CS had
been required to initiate the SPI memory communications seeing the generic
spi_mem_exec_op() doesn't do this. Manual GPIO manipulation is indeed redundant
for the current SPI-mem op execution procedure.
Secondly the message of that commit states "Devices with chip selects driven
via GPIO are not compatible with the spi-mem operations." I find this statement
questionable, because for instance this device supports memory operations with
GPIO-driven CS. Though in current implementation the driver fallback to using
normal
push-pull IO mode if GPIO CS is utilized as safer one. But even in this case
it's better than splitting the memory operations up into the transfers, which is
developed in the spi_mem_exec_op() method.
So in this matter my question is: how to modify the SPI-mem interface so the
SPI-memory operations would also work with GPIO driven CS? Some additional flag
might work...
Thirdly what about dirmap operations? If we got a GPIO driven CS then due to
lacking any CS manipulation in spi_mem_dirmap_read() method we wouldn't have
been able to make the direct mapping working without manual setting the GPIO.
So the same question here. How to work this around and justify your requirement?
Until the
Re: [PATCH hmm v2 1/5] mm/hmm: make CONFIG_DEVICE_PRIVATE into a select
On Fri, 1 May 2020 15:20:44 -0300 Jason Gunthorpe wrote: > From: Jason Gunthorpe > > There is no reason for a user to select this or not directly - it should > be selected by drivers that are going to use the feature, similar to how > CONFIG_HMM_MIRROR works. > > Currently all drivers provide a feature kconfig that will disable use of > DEVICE_PRIVATE in that driver, allowing users to avoid enabling this if > they don't want the overhead. > I'm not too sure what's going on here, but i386 allmodconfig broke. kernel/resource.c: In function '__request_free_mem_region': kernel/resource.c:1653:28: error: 'PA_SECTION_SHIFT' undeclared (first use in this function); did you mean 'SECTIONS_PGSHIFT'? size = ALIGN(size, 1UL << PA_SECTION_SHIFT); because in current mainline, allmodconfig produces CONFIG_DEVICE_PRIVATE=n but in current linux-next, allmodconfig produces CONFIG_DEVICE_PRIVATE=y. But CONFIG_SPARSEMEM=n so the build breaks. Bisection fingers this commit, but reverting it doesn't seem to fix things. Could you take a look please? I'm seeing this from menuconfig: WARNING: unmet direct dependencies detected for DEVICE_PRIVATE Depends on [n]: ZONE_DEVICE [=n] Selected by [m]: - DRM_NOUVEAU_SVM [=y] && HAS_IOMEM [=y] && DRM_NOUVEAU [=m] && MMU [=y] && STAGING [=y] - TEST_HMM [=m] && RUNTIME_TESTING_MENU [=y] && TRANSPARENT_HUGEPAGE [=y] `select' rather sucks this way - easy to break dependencies. Quite a number of years ago the Kconfig gurus were saying "avoid", but I don't recall the details.
Re: [PATCH net-next 3/4] net: phy: broadcom: add cable test support
On 5/9/2020 3:37 PM, Michael Walle wrote: > Most modern broadcom PHYs support ECD (enhanced cable diagnostics). Add > support for it in the bcm-phy-lib so they can easily be used in the PHY > driver. > > There are two access methods for ECD: legacy by expansion registers and > via the new RDB registers which are exclusive. Provide functions in two > variants where the PHY driver can from. To keep things simple for now, > we just switch the register access to expansion registers in the RDB > variant for now. On the flipside, we have to keep a bus lock to prevent > any other non-legacy access on the PHY. > > Signed-off-by: Michael Walle Reviewed-by: Florian Fainelli Thanks for dealing with the legacy expansion vs. RDB access method, this looks really nice, now I just need to test it on a variety of devices :) -- Florian
Re: [PATCH net-next 4/4] net: phy: bcm54140: add cable diagnostics support
On 5/9/2020 3:37 PM, Michael Walle wrote: > Use the generic cable tester functions from bcm-phy-lib to add cable > tester support. > > 100m cable, A/B/C/D open: > Cable test started for device eth0. > Cable test completed for device eth0. > Pair: Pair A, result: Open Circuit > Pair: Pair B, result: Open Circuit > Pair: Pair C, result: Open Circuit > Pair: Pair D, result: Open Circuit > Pair: Pair A, fault length: 106.60m > Pair: Pair B, fault length: 103.32m > Pair: Pair C, fault length: 104.96m > Pair: Pair D, fault length: 106.60m > > 1m cable, A/B connected, pair C shorted, D open: > Cable test started for device eth0. > Cable test completed for device eth0. > Pair: Pair A, result: OK > Pair: Pair B, result: OK > Pair: Pair C, result: Short within Pair > Pair: Pair D, result: Open Circuit > Pair: Pair C, fault length: 0.82m > Pair: Pair D, fault length: 1.64m > > 1m cable, A/B connected, pair C shorted with D: > Cable test started for device eth0. > Cable test completed for device eth0. > Pair: Pair A, result: OK > Pair: Pair B, result: OK > Pair: Pair C, result: Short to another pair > Pair: Pair D, result: Short to another pair > Pair: Pair C, fault length: 1.64m > Pair: Pair D, fault length: 1.64m > > The granularity of the length measurement seems to be 82cm. > > Signed-off-by: Michael Walle Reviewed-by: Florian Fainelli -- Florian
Re: [PATCH net-next 4/4] net: phy: bcm54140: add cable diagnostics support
Hi Michael,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on next-20200508]
[cannot apply to net/master linus/master v5.7-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Michael-Walle/net-phy-broadcom-cable-tester-support/20200510-063955
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
2c674bec76d35b75c7c730f863424387c9e9633a
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross
ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
drivers/net/phy/bcm54140.c:834:13: error: 'PHY_POLL_CABLE_TEST' undeclared
here (not in a function)
834 | .flags = PHY_POLL_CABLE_TEST,
| ^~~
drivers/net/phy/bcm54140.c:846:4: error: 'struct phy_driver' has no member
named 'cable_test_start'
846 | .cable_test_start = bcm_phy_cable_test_start_rdb,
|^~~~
>> drivers/net/phy/bcm54140.c:846:23: error: initialization of 'int (*)(struct
>> phy_device *, bool)' {aka 'int (*)(struct phy_device *, _Bool)'} from
>> incompatible pointer type 'int (*)(struct phy_device *)'
>> [-Werror=incompatible-pointer-types]
846 | .cable_test_start = bcm_phy_cable_test_start_rdb,
| ^~~~
drivers/net/phy/bcm54140.c:846:23: note: (near initialization for
'bcm54140_drivers[0].set_loopback')
drivers/net/phy/bcm54140.c:847:4: error: 'struct phy_driver' has no member
named 'cable_test_get_status'
847 | .cable_test_get_status = bcm_phy_cable_test_get_status_rdb,
|^
drivers/net/phy/bcm54140.c:847:28: warning: excess elements in struct
initializer
847 | .cable_test_get_status = bcm_phy_cable_test_get_status_rdb,
|^
drivers/net/phy/bcm54140.c:847:28: note: (near initialization for
'bcm54140_drivers[0]')
cc1: some warnings being treated as errors
vim +846 drivers/net/phy/bcm54140.c
828
829 static struct phy_driver bcm54140_drivers[] = {
830 {
831 .phy_id = PHY_ID_BCM54140,
832 .phy_id_mask= BCM54140_PHY_ID_MASK,
833 .name = "Broadcom BCM54140",
834 .flags = PHY_POLL_CABLE_TEST,
835 .features = PHY_GBIT_FEATURES,
836 .config_init= bcm54140_config_init,
837 .did_interrupt = bcm54140_did_interrupt,
838 .ack_interrupt = bcm54140_ack_intr,
839 .config_intr= bcm54140_config_intr,
840 .probe = bcm54140_probe,
841 .suspend= genphy_suspend,
842 .resume = genphy_resume,
843 .soft_reset = genphy_soft_reset,
844 .get_tunable= bcm54140_get_tunable,
845 .set_tunable= bcm54140_set_tunable,
> 846 .cable_test_start = bcm_phy_cable_test_start_rdb,
847 .cable_test_get_status =
bcm_phy_cable_test_get_status_rdb,
848 },
849 };
850 module_phy_driver(bcm54140_drivers);
851
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH v5 9/9] w1_therm: adding bulk read support to trigger multiple conversion on bus
Adding bulk read support:
Sending a 'trigger' command in the dedicated sysfs entry of bus master
device send a conversion command for all the slaves on the bus. The sysfs
entry is added as soon as at least one device supporting this feature
is detected on the bus.
The behavior of the sysfs reading temperature on the device is as follow:
* If no bulk read pending, trigger a conversion on the device, wait for
the conversion to be done, read the temperature in device RAM
* If a bulk read has been trigger, access directly the device RAM
This behavior is the same on the 2 sysfs entries ('temperature' and
'w1_slave').
Reading the therm_bulk_read sysfs give the status of bulk operations:
* '-1': conversion in progress on at least 1 sensor
* '1': conversion complete but at least one sensor has not been read yet
* '0': no bulk operation. Reading temperature on ecah device will trigger
a conversion
As not all devices support bulk read feature, it has been added in device
family structure.
The attribute is set at master level as soon as a supporting device is
discover. It is removed when the last supported device leave the bus.
The count of supported device is kept with the static counter
bulk_read_device_counter.
A strong pull up is apply on the line if at least one device required it.
The duration of the pull up is the max time required by a device on the
line, which depends on the resolution settings of each device. The strong
pull up could be adjust with the a module parameter.
Updating documentation in Documentation/ABI/testing/sysfs-driver-w1_therm
and Documentation/w1/slaves/w1_therm.rst accordingly.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
.../ABI/testing/sysfs-driver-w1_therm | 36 ++-
Documentation/w1/slaves/w1_therm.rst | 50 +++-
drivers/w1/slaves/w1_therm.c | 247 +-
3 files changed, 318 insertions(+), 15 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index f289520..076659d 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -62,9 +62,16 @@ Date:May 2020
Contact: Akira Shimahara
Description:
(RO) return the temperature in 1/1000 degC.
- Note that the conversion duration depend on the resolution (if
- device support this feature). It takes 94ms in 9bits
- resolution, 750ms for 12bits.
+ * If a bulk read has been triggered, it will directly
+ return the temperature computed when the bulk read
+ occurred, if available. If not yet available, nothing
+ is returned (a debug kernel message is sent), you
+ should retry later on.
+ * If no bulk read has been triggered, it will trigger
+ a conversion and send the result. Note that the
+ conversion duration depend on the resolution (if
+ device support this feature). It takes 94ms in 9bits
+ resolution, 750ms for 12bits.
Users: any user space application which wants to communicate with
w1_term device
@@ -85,4 +92,25 @@ Description:
refer to Documentation/w1/slaves/w1_therm.rst for detailed
information.
Users: any user space application which wants to communicate with
- w1_term device
\ No newline at end of file
+ w1_term device
+
+
+What: /sys/bus/w1/devices/w1_bus_masterXX/therm_bulk_read
+Date: May 2020
+Contact: Akira Shimahara
+Description:
+ (RW) trigger a bulk read conversion. read the status
+ *read*:
+ * '-1': conversion in progress on at least 1 sensor
+ * '1' : conversion complete but at least one sensor
+ value has not been read yet
+ * '0' : no bulk operation. Reading temperature will
+ trigger a conversion on each device
+ *write*: 'trigger': trigger a bulk read on all supporting
+ devices on the bus
+ Note that if a bulk read is sent but one sensor is not read
+ immediately, the next access to temperature on this device
+ will return the temperature measured at the time of issue
+ of the bulk read command (not the current temperature).
+Users: any user space application which wants to communicate with
+ w1_term device
diff --git a/Documentation/w1/slaves/w1_therm.rst
b/Documentation/w1/slaves/w1_therm.rst
index 82e8ffe..7c42f00 100644
--- a/Documentation/w1/slaves/w1_therm.rst
[PATCH v5 7/9] w1_therm: optimizing temperature read timings
Optimizing temperature reading by reducing waiting conversion time
according to device resolution settings, as per device specification.
This is device dependent as not all the devices supports resolution
setting, so it has been added in device family structures.
The process to read the temperature on the device has been adapted in a
new function 'convert_t()', which replace the former 'read_therm()', is
introduce to deal with this timing. Strong pull up is also applied during
the required time, according to device power status needs and
'strong_pullup' module parameter.
'temperature_from_RAM()' function is introduced to get the correct
temperature computation (device dependent) from device RAM data.
An new sysfs entry has been added to ouptut only temperature. The old
entry w1_slave has been kept for compatibility, without changing its
output format.
Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
.../ABI/testing/sysfs-driver-w1_therm | 12 +
drivers/w1/slaves/w1_therm.c | 278 --
2 files changed, 195 insertions(+), 95 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 8b7ee89..6ffd3e3 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -41,6 +41,18 @@ Users: any user space application which wants
to communicate with
w1_term device
+What: /sys/bus/w1/devices/.../temperature
+Date: May 2020
+Contact: Akira Shimahara
+Description:
+ (RO) return the temperature in 1/1000 degC.
+ Note that the conversion duration depend on the resolution (if
+ device support this feature). It takes 94ms in 9bits
+ resolution, 750ms for 12bits.
+Users: any user space application which wants to communicate with
+ w1_term device
+
+
What: /sys/bus/w1/devices/.../w1_slave
Date: May 2020
Contact: Akira Shimahara
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index d3f83f8..2955cae 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -94,6 +94,7 @@ struct w1_therm_family_converter {
u16 reserved;
struct w1_family*f;
int (*convert)(u8 rom[9]);
+ int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
};
@@ -144,6 +145,13 @@ struct therm_info {
*/
static int reset_select_slave(struct w1_slave *sl);
+/** convert_t()
+ * \param sl pointer to the slave to read
+ * \param info pointer to a structure to store the read results
+ * \return 0 if success, -kernel error code otherwise
+ */
+static int convert_t(struct w1_slave *sl, struct therm_info *info);
+
/** read_scratchpad()
* \param sl pointer to the slave to read
* \param info pointer to a structure to store the read results
@@ -201,6 +209,15 @@ static ssize_t w1_slave_store(struct device *device,
static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
+/** \brief A callback function to output the temperature
+ * Main differences with w1_slave :
+ * No hardware check (just read the stored device infos)
+ * No formatting
+ * \return temperature (1/1000°)
+ */
+static ssize_t temperature_show(struct device *device,
+ struct device_attribute *attr, char *buf);
+
/** \brief A callback function to output the power mode of the device
* Once done, it is stored in the sl->family_data to avoid doing the test
* during data read
@@ -235,6 +252,7 @@ static ssize_t eeprom_store(struct device *device,
static DEVICE_ATTR_RW(w1_slave);
static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(temperature);
static DEVICE_ATTR_RO(ext_power);
static DEVICE_ATTR_RW(resolution);
static DEVICE_ATTR_WO(eeprom);
@@ -258,6 +276,7 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+ _attr_temperature.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
@@ -266,6 +285,7 @@ static struct attribute *w1_therm_attrs[] = {
static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
+ _attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
NULL,
@@ -274,6 +294,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
+ _attr_temperature.attr,
_attr_ext_power.attr,
[PATCH v5 8/9] w1_therm: adding alarm sysfs entry
Adding device alarms settings by a dedicated sysfs entry alarms (RW):
read or write TH and TL in the device RAM. Checking devices in alarm
state could be performed using the master search command.
As alarms temperature level are store in a 8 bit register on the device
and are signed values, a safe cast shall be performed using the min and
max temperature that device are able to measure. This is done by
int_to_short inline function.
A 'write_data' field is added in the device structure, to bind the
correct writing function, as some devices may have 2 or 3 bytes RAM.
Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
.../ABI/testing/sysfs-driver-w1_therm | 16 ++
drivers/w1/slaves/w1_therm.c | 158 ++
2 files changed, 174 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 6ffd3e3..f289520 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,19 @@
+What: /sys/bus/w1/devices/.../alarms
+Date: May 2020
+Contact: Akira Shimahara
+Description:
+ (RW) read or write TH and TL (Temperature High an Low) alarms.
+ Values shall be space separated and in the device range
+ (typical -55 degC to 125 degC), if not values will be trimmed
+ to device min/max capabilities. Values are integer as they are
+ stored in a 8bit register in the device. Lowest value is
+ automatically put to TL. Once set, alarms could be search at
+ master level, refer to Documentation/w1/w1_generic.rst for
+ detailed information
+Users: any user space application which wants to communicate with
+ w1_term device
+
+
What: /sys/bus/w1/devices/.../eeprom
Date: May 2020
Contact: Akira Shimahara
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 2955cae..cd65d0b 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -57,6 +57,9 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
#define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
#define EEPROM_CMD_READ "restore" /* cmd for read eeprom sysfs */
+#define MIN_TEMP -55 /* min temperature that can be mesured */
+#define MAX_TEMP 125 /* max temperature that can be mesured */
+
/*---Macros-*/
/* return a pointer on the slave w1_therm_family_converter struct:
@@ -97,6 +100,7 @@ struct w1_therm_family_converter {
int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
+ int (*write_data)(struct w1_slave *sl, const u8 *data);
};
/**
@@ -248,6 +252,18 @@ static ssize_t resolution_store(struct device *device,
static ssize_t eeprom_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
+/** \brief A callback function to set the alarms level
+ * \param device represents the master device
+ */
+static ssize_t alarms_store(struct device *device,
+ struct device_attribute *attr, const char *buf, size_t size);
+
+/** \brief A callback function to get the alarms level
+ * \return Low and High alarm, separate by one space
+ */
+static ssize_t alarms_show(struct device *device,
+ struct device_attribute *attr, char *buf);
+
/*-Attributes declarations--*/
static DEVICE_ATTR_RW(w1_slave);
@@ -256,6 +272,7 @@ static DEVICE_ATTR_RO(temperature);
static DEVICE_ATTR_RO(ext_power);
static DEVICE_ATTR_RW(resolution);
static DEVICE_ATTR_WO(eeprom);
+static DEVICE_ATTR_RW(alarms);
/*Interface Functions declaration---*/
@@ -280,6 +297,7 @@ static struct attribute *w1_therm_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+ _attr_alarms.attr,
NULL,
};
@@ -288,6 +306,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
_attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
+ _attr_alarms.attr,
NULL,
};
@@ -298,6 +317,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+ _attr_alarms.attr,
NULL,
};
@@ -543,6 +563,7 @@ static struct w1_therm_family_converter w1_therm_families[]
= {
.get_conversion_time= w1_DS18S20_convert_time,
.set_resolution
[PATCH v5 6/9] w1_therm: adding eeprom sysfs entry
Adding eeprom sysfs entry (WO) to trigger either device EEPROM save
(by writing 'save' in the sysfs), either device EEPROM restore (by writing
'restore' in the sysfs). All the RAM of the device will be save/restore,
whatever its size (actually 2 or 3 bytes): resolution config register and
alarms level will be save/restore.
The driver implement 2 hardware functions to access device RAM:
* copy_scratchpad
* recall_scratchpad
They act according to device specifications.
As EEPROM operations are not device dependent (all w1_therm can perform
EEPROM read/write operation following the same protocol), it is removed
from device families structures.
Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
.../ABI/testing/sysfs-driver-w1_therm | 14 ++
drivers/w1/slaves/w1_therm.c | 176 --
2 files changed, 133 insertions(+), 57 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 7ed95e9..8b7ee89 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,17 @@
+What: /sys/bus/w1/devices/.../eeprom
+Date: May 2020
+Contact: Akira Shimahara
+Description:
+ (WO) writing that file will either trigger a save of the
+ device data to its embedded EEPROM, either restore data
+ embedded in device EEPROM. Be aware that devices support
+ limited EEPROM writing cycles (typical 50k)
+ * 'save': save device RAM to EEPROM
+ * 'restore': restore EEPROM data in device RAM
+Users: any user space application which wants to communicate with
+ w1_term device
+
+
What: /sys/bus/w1/devices/.../ext_power
Date: May 2020
Contact: Akira Shimahara
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 159bfc1..d3f83f8 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -42,12 +42,21 @@
static int w1_strong_pullup = 1;
module_param_named(strong_pullup, w1_strong_pullup, int, 0);
+/* This command should be in public header w1.h but is not */
+#define W1_RECALL_EEPROM 0xB8
+
/* Nb of try for an operation */
#define W1_THERM_MAX_TRY 5
/* ms delay to retry bus mutex */
#define W1_THERM_RETRY_DELAY 20
+/* delay in ms to write in EEPROM */
+#define W1_THERM_EEPROM_WRITE_DELAY10
+
+#define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
+#define EEPROM_CMD_READ "restore" /* cmd for read eeprom sysfs */
+
/*---Macros-*/
/* return a pointer on the slave w1_therm_family_converter struct:
@@ -87,7 +96,6 @@ struct w1_therm_family_converter {
int (*convert)(u8 rom[9]);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
- int (*eeprom)(struct device *device);
};
/**
@@ -151,6 +159,19 @@ static int read_scratchpad(struct w1_slave *sl, struct
therm_info *info);
*/
static int write_scratchpad(struct w1_slave *sl, const u8 *data, u8 nb_bytes);
+/** copy_scratchpad() - Copy the content of scratchpad in device EEPROM
+ * \param sl slave involved
+ * \return 0 if success, -kernel error code otherwise
+ */
+static int copy_scratchpad(struct w1_slave *sl);
+
+/** recall_eeprom()
+ * \brief retrieve EEPROM data to device RAM
+ * \param sl slave involved
+ * \return 0 if success, -kernel error code otherwise
+ */
+static int recall_eeprom(struct w1_slave *sl);
+
/** read_powermode()
* \brief ask the device to get its power mode {external, parasite}
* \param sl slave to be interrogated
@@ -204,12 +225,19 @@ static ssize_t resolution_show(struct device *device,
static ssize_t resolution_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
+/** \brief A callback function to let the user read/write device EEPROM
+ * \param check EEPROM_CMD_WRITE & EEPROM_CMD_READ macros
+ */
+static ssize_t eeprom_store(struct device *device,
+ struct device_attribute *attr, const char *buf, size_t size);
+
/*-Attributes declarations--*/
static DEVICE_ATTR_RW(w1_slave);
static DEVICE_ATTR_RO(w1_seq);
static DEVICE_ATTR_RO(ext_power);
static DEVICE_ATTR_RW(resolution);
+static DEVICE_ATTR_WO(eeprom);
/*Interface Functions declaration---*/
@@ -232,12 +260,14 @@ static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
+
[PATCH v5 5/9] w1_therm: adding resolution sysfs entry
Adding resolution sysfs entry (RW) to get or set the device resolution
Write values are managed as follow:
* '9..12': resolution to set in bit
* Anything else: do nothing
Read values are :
* '9..12': device resolution in bit
* '-xx': xx is kernel error when reading the resolution
Only supported devices will show the sysfs entry. A new family has been
created for DS18S20 devices as they do not implement resolution feature.
The resolution of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the resolution of each device, which could
be used later to determine the required conversion duration (resolution
dependent).
The resolution is re evaluate each time a user read or write the sysfs
entry.
To avoid looping through the w1_therm_families at run time, the pointer
'specific_functions' is set up to the correct 'w1_therm_family_converter'
when the slave is added (which mean when it is discovered by the master).
This initialization is done by a helper function
'device_family(struct w1_slave *sl)', and a dedicated macro
'SLAVE_SPECIFIC_FUNC(sl)' allow the access to the specific function of the
slave device.
'read_scratchpad' and 'write_scratchpad' are the hardware functions to
access the device RAM, as per protocol specification.
It cancel the former 'precision' functions, which was only set and never
read (so not stored in the device struct).
Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
.../ABI/testing/sysfs-driver-w1_therm | 17 +
drivers/w1/slaves/w1_therm.c | 436 ++
2 files changed, 356 insertions(+), 97 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 99d73ee..7ed95e9 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -10,6 +10,23 @@ Users: any user space application which wants
to communicate with
w1_term device
+What: /sys/bus/w1/devices/.../resolution
+Date: May 2020
+Contact: Akira Shimahara
+Description:
+ (RW) get or set the device resolution (on supported devices,
+ if not, this entry is not present). Note that the resolution
+ will be changed only in device RAM, so it will be cleared when
+ power is lost. Trigger a 'save' to EEPROM command to keep
+ values after power-on. Read or write are :
+ * '9..12': device resolution in bit
+ or resolution to set in bit
+ * '-xx': xx is kernel error when reading the resolution
+ * Anything else: do nothing
+Users: any user space application which wants to communicate with
+ w1_term device
+
+
What: /sys/bus/w1/devices/.../w1_slave
Date: May 2020
Contact: Akira Shimahara
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index ab15cca..159bfc1 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -50,12 +50,24 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
/*---Macros-*/
+/* return a pointer on the slave w1_therm_family_converter struct:
+ * always test family data existence before
+ */
+#define SLAVE_SPECIFIC_FUNC(sl) \
+ (((struct w1_therm_family_data *)(sl->family_data))->specific_functions)
+
/* return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
* always test family data existence before
*/
#define SLAVE_POWERMODE(sl) \
(((struct w1_therm_family_data *)(sl->family_data))->external_powered)
+/* return the resolution in bit of the sl slave : <0 unknown
+ * always test family data existence before
+ */
+#define SLAVE_RESOLUTION(sl) \
+ (((struct w1_therm_family_data *)(sl->family_data))->resolution)
+
/* return the address of the refcnt in the family data */
#define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -73,7 +85,8 @@ struct w1_therm_family_converter {
u16 reserved;
struct w1_family*f;
int (*convert)(u8 rom[9]);
- int (*precision)(struct device *device, int val);
+ int (*set_resolution)(struct w1_slave *sl, int val);
+ int (*get_resolution)(struct w1_slave *sl);
int (*eeprom)(struct device *device);
};
@@ -91,6 +104,8 @@ struct w1_therm_family_data {
uint8_t rom[9];
[PATCH v5 4/9] w1_therm: adding ext_power sysfs entry
Adding ext_power sysfs entry (RO). Return the power status of the device:
- 0: device parasite powered
- 1: device externally powered
- xx: xx is kernel error
The power status of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the power state of each device, which could
be used later to determine the required strong pull up to apply on the
line.
The power status is re evaluate each time the sysfs ext_power read by
a user.
The hardware function 'read_powermode(struct w1_slave *sl)' act just as
per device specifications, sending W1_READ_PSUPPLY command on the bus,
and issue a read time slot, reading only one bit.
A helper function 'bool bus_mutex_lock(struct mutex *lock)' is introduced.
It try to aquire the bus mutex several times (W1_THERM_MAX_TRY), waiting
W1_THERM_RETRY_DELAY between two attempt.
Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
.../ABI/testing/sysfs-driver-w1_therm | 12 ++
drivers/w1/slaves/w1_therm.c | 129 ++
2 files changed, 141 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 4a85663..99d73ee 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,15 @@
+What: /sys/bus/w1/devices/.../ext_power
+Date: May 2020
+Contact: Akira Shimahara
+Description:
+ (RO) return the power status by asking the device
+ * '0': device parasite powered
+ * '1': device externally powered
+ * '-xx': xx is kernel error when reading power status
+Users: any user space application which wants to communicate with
+ w1_term device
+
+
What: /sys/bus/w1/devices/.../w1_slave
Date: May 2020
Contact: Akira Shimahara
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 4dd139b..ab15cca 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -42,8 +42,20 @@
static int w1_strong_pullup = 1;
module_param_named(strong_pullup, w1_strong_pullup, int, 0);
+/* Nb of try for an operation */
+#define W1_THERM_MAX_TRY 5
+
+/* ms delay to retry bus mutex */
+#define W1_THERM_RETRY_DELAY 20
+
/*---Macros-*/
+/* return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
+ * always test family data existence before
+ */
+#define SLAVE_POWERMODE(sl) \
+ (((struct w1_therm_family_data *)(sl->family_data))->external_powered)
+
/* return the address of the refcnt in the family data */
#define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -78,6 +90,7 @@ struct w1_therm_family_converter {
struct w1_therm_family_data {
uint8_t rom[9];
atomic_t refcnt;
+ int external_powered;
};
/**
@@ -108,6 +121,15 @@ struct therm_info {
*/
static int reset_select_slave(struct w1_slave *sl);
+/** read_powermode()
+ * \brief ask the device to get its power mode {external, parasite}
+ * \param sl slave to be interrogated
+ * \return 0 parasite powered device
+ * 1 externally powered device
+ * <0 kernel error code
+ */
+static int read_powermode(struct w1_slave *sl);
+
/*---Interface sysfs declaration*/
/** \brief A callback function to output the temperature Old way
@@ -128,10 +150,21 @@ static ssize_t w1_slave_store(struct device *device,
static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
+/** \brief A callback function to output the power mode of the device
+ * Once done, it is stored in the sl->family_data to avoid doing the test
+ * during data read
+ * \return0 : device parasite powered
+ * 1 : device externally powered
+ * -xx : xx is kernel error code
+ */
+static ssize_t ext_power_show(struct device *device,
+ struct device_attribute *attr, char *buf);
+
/*-Attributes declarations--*/
static DEVICE_ATTR_RW(w1_slave);
static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(ext_power);
/*Interface Functions declaration---*/
@@ -152,12 +185,14 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+ _attr_ext_power.attr,
NULL,
};
static struct attribute
[PATCH v5 3/9] w1_therm: adding sysfs-driver-w1_therm doc
Adding a sysfs-driver-w1_therm documentation file in Documentation/ABI/testing. It describe the onlys sysfs entry of w1_therm module, based on Documentation/w1/slaves/w1_therm.rst Signed-off-by: Akira Shimahara --- Changes in v5: - All patch serie in one .c file - Correcting some comments - adding include Documentation/ABI/testing/sysfs-driver-w1_therm | 17 + 1 file changed, 17 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm b/Documentation/ABI/testing/sysfs-driver-w1_therm new file mode 100644 index 000..4a85663 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-w1_therm @@ -0,0 +1,17 @@ +What: /sys/bus/w1/devices/.../w1_slave +Date: May 2020 +Contact: Akira Shimahara +Description: + (RW) return the temperature in 1/1000 degC. + *read*: return 2 lines with the hexa output data sent on the + bus, return the CRC check and temperature in 1/1000 degC + *write* : + * '0' : save the 2 or 3 bytes to the device EEPROM + (i.e. TH, TL and config register) + * '9..12' : set the device resolution in RAM + (if supported) + * Anything else: do nothing + refer to Documentation/w1/slaves/w1_therm.rst for detailed + information. +Users: any user space application which wants to communicate with + w1_term device \ No newline at end of file -- 2.26.2
[PATCH v5 2/9] w1_therm: fix reset_select_slave during discovery
Fix reset_select_slave issue during devices discovery by the master on
bus. The w1_reset_select_slave() from w1_io.c, which was previously used,
assume that if the slave count is 1 there is only one slave attached on
the bus. This is not always true. For example when discovering devices,
when the first device is discover by the bus master, its slave count is
1, but some other slaves may be on the bus.
In that case instead of adressing command to the attached slave the
master throw a SKIP ROM command so that all slaves attached on the bus
will answer simultenaously causing data collision.
A dedicated reset_select_slave() function is implemented here,
it always perform an adressing to each slave using the MATCH ROM
command.
Signed-off-by: Akira Shimahara
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
drivers/w1/slaves/w1_therm.c | 45 ++--
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index f7147b2..4dd139b 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
@@ -92,6 +93,21 @@ struct therm_info {
u8 verdict;
};
+/*-Hardware Functions declaration---*/
+
+/**
+ * reset_select_slave() - reset and select a slave
+ * \brief Resets the bus and select the slave by sending a ROM MATCH cmd
+ * w1_reset_select_slave() from w1_io.c could not be used here because
+ * it sent a SKIP ROM command if only one device is on the line.
+ * At the beginning of the such process, sl->master->slave_count is 1 even if
+ * more devices are on the line, causing collision on the line.
+ * The w1 master lock must be held.
+ * \param sl the slave to select
+ * \return 0 if success, negative kernel error code otherwise
+ */
+static int reset_select_slave(struct w1_slave *sl);
+
/*---Interface sysfs declaration*/
/** \brief A callback function to output the temperature Old way
@@ -305,7 +321,7 @@ static inline int w1_DS18B20_precision(struct device
*device, int val)
while (max_trying--) {
crc = 0;
- if (!w1_reset_select_slave(sl)) {
+ if (!reset_select_slave(sl)) {
int count = 0;
/* read values to only alter precision bits */
@@ -318,7 +334,7 @@ static inline int w1_DS18B20_precision(struct device
*device, int val)
if (rom[8] == crc) {
rom[4] = (rom[4] & ~mask) | (precision_bits &
mask);
- if (!w1_reset_select_slave(sl)) {
+ if (!reset_select_slave(sl)) {
w1_write_8(dev, W1_WRITE_SCRATCHPAD);
w1_write_8(dev, rom[2]);
w1_write_8(dev, rom[3]);
@@ -440,6 +456,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
/*---Hardware Functions-*/
+/* Safe version of reser_select_slave - avoid using the one in w_io.c */
+static int reset_select_slave(struct w1_slave *sl)
+{
+ u8 match[9] = { W1_MATCH_ROM, };
+ u64 rn = le64_to_cpu(*((u64 *)>reg_num));
+
+ if (w1_reset_bus(sl->master))
+ return -ENODEV;
+
+ memcpy([1], , 8);
+ w1_write_block(sl->master, match, 9);
+
+ return 0;
+}
+
static ssize_t read_therm(struct device *device,
struct w1_slave *sl, struct therm_info *info)
{
@@ -467,7 +498,7 @@ static ssize_t read_therm(struct device *device,
info->verdict = 0;
info->crc = 0;
- if (!w1_reset_select_slave(sl)) {
+ if (!reset_select_slave(sl)) {
int count = 0;
unsigned int tm = 750;
unsigned long sleep_rem;
@@ -475,7 +506,7 @@ static ssize_t read_therm(struct device *device,
w1_write_8(dev, W1_READ_PSUPPLY);
external_power = w1_read_8(dev);
- if (w1_reset_select_slave(sl))
+ if (reset_select_slave(sl))
continue;
/* 750ms strong pullup (or delay) after the convert */
@@ -505,7 +536,7 @@ static ssize_t read_therm(struct device *device,
}
}
- if (!w1_reset_select_slave(sl)) {
+ if (!reset_select_slave(sl)) {
w1_write_8(dev, W1_READ_SCRATCHPAD);
count = w1_read_block(dev, info->rom, 9);
@@ -557,7 +588,7 @@ static inline int w1_therm_eeprom(struct device *device)
[PATCH v5 1/9] w1_therm: adding code comments and code reordering
Adding code comments to split code in dedicated parts. After the global
declarations (defines, macros and function declarations), code is organized
as follow :
- Device and family dependent structures and functions
- Interfaces functions
- Helpers functions
- Hardware functions
- Sysfs interface functions
Signed-off-by: Akira Shimahara
---
Main motivation on the first patch of this serie is to clean up the code,
document it and reorder it to prepare the next patches, which are clearer
after this.
One main point is to keep all device/family dependent code gather at the
beginning to ease adding new devices if required.
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding include
drivers/w1/slaves/w1_therm.c | 403 ---
1 file changed, 237 insertions(+), 166 deletions(-)
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..f7147b2 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -41,55 +41,99 @@
static int w1_strong_pullup = 1;
module_param_named(strong_pullup, w1_strong_pullup, int, 0);
+/*---Macros-*/
+
+/* return the address of the refcnt in the family data */
+#define THERM_REFCNT(family_data) \
+ (&((struct w1_therm_family_data *)family_data)->refcnt)
+
+/*--Structs-*/
+
+/**
+ * struct w1_therm_family_converter
+ * \brief Used to bind standard function call
+ * to device specific function
+ * it could be routed to NULL if device don't support feature
+ */
+struct w1_therm_family_converter {
+ u8 broken;
+ u16 reserved;
+ struct w1_family*f;
+ int (*convert)(u8 rom[9]);
+ int (*precision)(struct device *device, int val);
+ int (*eeprom)(struct device *device);
+};
+
+/**
+ * struct w1_therm_family_data
+ * \param rom data
+ * \param refcnt ref count
+ * \param external_powered
+ * 1 device powered externally,
+ * 0 device parasite powered,
+ * -x error or undefined
+ * \param resolution resolution in bit of the device, refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
- sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
- GFP_KERNEL);
- if (!sl->family_data)
- return -ENOMEM;
- atomic_set(THERM_REFCNT(sl->family_data), 1);
- return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
- int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
- while (refcnt) {
- msleep(1000);
- refcnt = atomic_read(THERM_REFCNT(sl->family_data));
- }
- kfree(sl->family_data);
- sl->family_data = NULL;
-}
+/*---Interface sysfs declaration*/
+/** \brief A callback function to output the temperature Old way
+ * read temperature and return the result in the sys file
+ * This has been kept for compatibility
+ */
static ssize_t w1_slave_show(struct device *device,
struct device_attribute *attr, char *buf);
+/** \brief A callback function to set the resolution Old way
+ * This has been kept for compatibility
+ * \param 0, it write config in the EEPROM
+ * \param 9..12, it set the resolution in the RAM
+ */
static ssize_t w1_slave_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
+/*-Attributes declarations--*/
+
static DEVICE_ATTR_RW(w1_slave);
static DEVICE_ATTR_RO(w1_seq);
+/*Interface Functions declaration---*/
+
+/** w1_therm_add_slave() - Called each time a search discover a new device
+ * \brief used to initialized slave (family datas)
+ * \param sl slave just discovered
+ * \return 0 - If success, negative kernel code otherwise
+ */
+static int w1_therm_add_slave(struct w1_slave *sl);
+
+/** w1_therm_remove_slave() - Called each time a slave is removed
+ * \brief used to free memory
+ * \param sl slave to be removed
+ */
+static void w1_therm_remove_slave(struct w1_slave *sl);
+
+/*--Family attributes---*/
+
static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
NULL,
@@ -101,6 +145,8 @@ static struct attribute *w1_ds28ea00_attrs[] = {
NULL,
};
+/*--Attribute groups*/
+
ATTRIBUTE_GROUPS(w1_therm);
ATTRIBUTE_GROUPS(w1_ds28ea00);
@@ -154,6 +200,8 @@ static const struct hwmon_chip_info w1_chip_info = {
#define W1_CHIPINFONULL
#endif
+/*--Family
Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths
On Fri, 8 May 2020, Jakub Kicinski wrote: > On Fri, 8 May 2020 19:25:57 +0200 Christophe JAILLET wrote: > > @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct > > platform_device *pdev) > > struct sonic_local* lp = netdev_priv(dev); > > > > unregister_netdev(dev); > > - dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * > > SONIC_BUS_SCALE(lp->dma_bitmode), > > - lp->descriptors, lp->descriptors_laddr); > > + dma_free_coherent(lp->device, > > + SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode), > > + lp->descriptors, lp->descriptors_laddr); > > free_netdev(dev); > > > > return 0; > > This is a white-space only change, right? Since this is a fix we should > avoid making cleanups which are not strictly necessary. > I think it is harmless if it doesn't create any merge conflicts. Any merge conflict created by the whitespace change would have happened anyway, because all of the changes in this patch are very closely related. That's why I was happy to put a 'reviewed-by' tag on this.
Re: [PATCH net-next 2/4] net: phy: broadcom: add bcm_phy_modify_exp()
On 5/9/2020 3:37 PM, Michael Walle wrote: > Add the convenience function to do a read-modify-write. This has the > additional benefit of saving one write to the selection register. > > Signed-off-by: Michael Walle Reviewed-by: Florian Fainelli -- Florian
[PATCH 07/20] nvram: drop useless access_ok()
From: Al Viro we are using copy_to_user()/memdup_user() anyway Signed-off-by: Al Viro --- drivers/char/nvram.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c index 4667844eee69..8206412d25ba 100644 --- a/drivers/char/nvram.c +++ b/drivers/char/nvram.c @@ -232,8 +232,6 @@ static ssize_t nvram_misc_read(struct file *file, char __user *buf, ssize_t ret; - if (!access_ok(buf, count)) - return -EFAULT; if (*ppos >= nvram_size) return 0; @@ -264,8 +262,6 @@ static ssize_t nvram_misc_write(struct file *file, const char __user *buf, char *tmp; ssize_t ret; - if (!access_ok(buf, count)) - return -EFAULT; if (*ppos >= nvram_size) return 0; -- 2.11.0
[PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok()
From: Al Viro address passed only to copy_from_user() Signed-off-by: Al Viro --- fs/ocfs2/dlmfs/dlmfs.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/fs/ocfs2/dlmfs/dlmfs.c b/fs/ocfs2/dlmfs/dlmfs.c index 1de77f1a600b..a06f19b67d3b 100644 --- a/fs/ocfs2/dlmfs/dlmfs.c +++ b/fs/ocfs2/dlmfs/dlmfs.c @@ -291,9 +291,6 @@ static ssize_t dlmfs_file_write(struct file *filp, if (!count) return 0; - if (!access_ok(buf, count)) - return -EFAULT; - lvb_buf = kmalloc(count, GFP_NOFS); if (!lvb_buf) return -ENOMEM; -- 2.11.0
[PATCH 02/20] fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade...
From: Al Viro address is passed only to put_user() and copy_to_user() Signed-off-by: Al Viro --- fs/fat/dir.c | 4 1 file changed, 4 deletions(-) diff --git a/fs/fat/dir.c b/fs/fat/dir.c index 054acd9fd033..b4ddf48fa444 100644 --- a/fs/fat/dir.c +++ b/fs/fat/dir.c @@ -804,8 +804,6 @@ static long fat_dir_ioctl(struct file *filp, unsigned int cmd, return fat_generic_ioctl(filp, cmd, arg); } - if (!access_ok(d1, sizeof(struct __fat_dirent[2]))) - return -EFAULT; /* * Yes, we don't need this put_user() absolutely. However old * code didn't return the right value. So, app use this value, @@ -844,8 +842,6 @@ static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd, return fat_generic_ioctl(filp, cmd, (unsigned long)arg); } - if (!access_ok(d1, sizeof(struct compat_dirent[2]))) - return -EFAULT; /* * Yes, we don't need this put_user() absolutely. However old * code didn't return the right value. So, app use this value, -- 2.11.0
Re: [PATCH net-next 1/4] net: phy: broadcom: add exp register access methods without buslock
On 5/9/2020 3:37 PM, Michael Walle wrote: > Add helper to read and write expansion registers without taking the mdio > lock. > > Please note, that this changes the semantics of the read and write. > Before there was no lock between selecting the expansion register and > the actual read/write. This may lead to access failures if there are > parallel accesses. Instead take the bus lock during the whole access > cycle. > > Signed-off-by: Michael Walle Reviewed-by: Florian Fainelli -- Florian
[PATCH 03/20] btrfs_ioctl_send(): don't bother with access_ok()
From: Al Viro
we do copy_from_user() on that range anyway
Signed-off-by: Al Viro
---
fs/btrfs/send.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index c5f41bd86765..6a92ecf9eaa2 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -7065,13 +7065,6 @@ long btrfs_ioctl_send(struct file *mnt_file, struct
btrfs_ioctl_send_args *arg)
goto out;
}
- if (!access_ok(arg->clone_sources,
- sizeof(*arg->clone_sources) *
- arg->clone_sources_count)) {
- ret = -EFAULT;
- goto out;
- }
-
if (arg->flags & ~BTRFS_SEND_FLAG_MASK) {
ret = -EINVAL;
goto out;
--
2.11.0
[PATCH 06/20] n_hdlc_tty_read(): remove pointless access_ok()
From: Al Viro
only copy_to_user() is done to the address in question
Signed-off-by: Al Viro
---
drivers/tty/n_hdlc.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 991f49ee4026..b09eac4b6d64 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -423,13 +423,6 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty,
struct file *file,
struct n_hdlc_buf *rbuf;
DECLARE_WAITQUEUE(wait, current);
- /* verify user access to buffer */
- if (!access_ok(buf, nr)) {
- pr_warn("%s(%d) %s() can't verify user buffer\n",
- __FILE__, __LINE__, __func__);
- return -EFAULT;
- }
-
add_wait_queue(>read_wait, );
for (;;) {
--
2.11.0
[PATCH 18/20] usb: get rid of pointless access_ok() calls
From: Al Viro
in all affected cases addresses are passed only to
copy_from()_user or copy_to_user().
Signed-off-by: Al Viro
---
drivers/usb/core/devices.c | 2 --
drivers/usb/core/devio.c| 9 -
drivers/usb/gadget/function/f_hid.c | 6 --
3 files changed, 17 deletions(-)
diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index 44f28a114c2b..94b6fa6e585e 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -598,8 +598,6 @@ static ssize_t usb_device_read(struct file *file, char
__user *buf,
return -EINVAL;
if (nbytes <= 0)
return 0;
- if (!access_ok(buf, nbytes))
- return -EFAULT;
mutex_lock(_bus_idr_lock);
/* print devices for all busses */
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 6833c918abce..544769807ab8 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1127,11 +1127,6 @@ static int proc_control(struct usb_dev_state *ps, void
__user *arg)
ctrl.bRequestType, ctrl.bRequest, ctrl.wValue,
ctrl.wIndex, ctrl.wLength);
if (ctrl.bRequestType & 0x80) {
- if (ctrl.wLength && !access_ok(ctrl.data,
- ctrl.wLength)) {
- ret = -EINVAL;
- goto done;
- }
pipe = usb_rcvctrlpipe(dev, 0);
snoop_urb(dev, NULL, pipe, ctrl.wLength, tmo, SUBMIT, NULL, 0);
@@ -1216,10 +1211,6 @@ static int proc_bulk(struct usb_dev_state *ps, void
__user *arg)
}
tmo = bulk.timeout;
if (bulk.ep & 0x80) {
- if (len1 && !access_ok(bulk.data, len1)) {
- ret = -EINVAL;
- goto done;
- }
snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, NULL, 0);
usb_unlock_device(dev);
diff --git a/drivers/usb/gadget/function/f_hid.c
b/drivers/usb/gadget/function/f_hid.c
index f3816a5c861e..df671acdd464 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -252,9 +252,6 @@ static ssize_t f_hidg_read(struct file *file, char __user
*buffer,
if (!count)
return 0;
- if (!access_ok(buffer, count))
- return -EFAULT;
-
spin_lock_irqsave(>read_spinlock, flags);
#define READ_COND (!list_empty(>completed_out_req))
@@ -339,9 +336,6 @@ static ssize_t f_hidg_write(struct file *file, const char
__user *buffer,
unsigned long flags;
ssize_t status = -ENOMEM;
- if (!access_ok(buffer, count))
- return -EFAULT;
-
spin_lock_irqsave(>write_spinlock, flags);
#define WRITE_COND (!hidg->write_pending)
--
2.11.0
[PATCH 13/20] drivers/crypto/ccp/sev-dev.c: get rid of pointless access_ok()
From: Al Viro
Contrary to the comments, those do *NOT* verify anything about
writability of memory, etc.
In all cases addresses are passed only to copy_to_user().
Signed-off-by: Al Viro
---
drivers/crypto/ccp/sev-dev.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 896f190b9a50..7f97164cbafb 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -371,8 +371,7 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp,
bool writable)
goto cmd;
/* allocate a physically contiguous buffer to store the CSR blob */
- if (!access_ok(input.address, input.length) ||
- input.length > SEV_FW_BLOB_MAX_SIZE) {
+ if (input.length > SEV_FW_BLOB_MAX_SIZE) {
ret = -EFAULT;
goto e_free;
}
@@ -609,12 +608,6 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp)
if (copy_from_user(, (void __user *)argp->data, sizeof(input)))
return -EFAULT;
- /* Check if we have write access to the userspace buffer */
- if (input.address &&
- input.length &&
- !access_ok(input.address, input.length))
- return -EFAULT;
-
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -730,15 +723,13 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd
*argp, bool writable)
goto cmd;
/* Allocate a physically contiguous buffer to store the PDH blob. */
- if ((input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE) ||
- !access_ok(input.pdh_cert_address, input.pdh_cert_len)) {
+ if (input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE) {
ret = -EFAULT;
goto e_free;
}
/* Allocate a physically contiguous buffer to store the cert chain
blob. */
- if ((input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE) ||
- !access_ok(input.cert_chain_address, input.cert_chain_len)) {
+ if (input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE) {
ret = -EFAULT;
goto e_free;
}
--
2.11.0
[PATCH v6 17/18] mtd: Support kmsg dumper based on pstore/blk
From: WeiXiong Liao
This introduces mtdpstore, which is similar to mtdoops but more
powerful. It uses pstore/blk, and aims to store panic and oops logs to
a flash partition, where pstore can later read back and present as files
in the mounted pstore filesystem.
To make mtdpstore work, the "blkdev" of pstore/blk should be set
as MTD device name or MTD device number. For more details, see
Documentation/admin-guide/pstore-blk.rst
This solves a number of issues:
- Work duplication: both of pstore and mtdoops do the same job storing
panic/oops log. They have very similar logic, registering to kmsg
dumper and storing logs to several chunks one by one.
- Layer violations: drivers should provides methods instead of polices.
MTD should provide read/write/erase operations, and allow a higher
level drivers to provide the chunk management, kmsg dump
configuration, etc.
- Missing features: pstore provides many additional features, including
presenting the logs as files, logging dump time and count, and
supporting other frontends like pmsg, console, etc.
Signed-off-by: WeiXiong Liao
Link:
https://lore.kernel.org/r/1585126506-18635-12-git-send-email-liaoweixi...@allwinnertech.com
Signed-off-by: Kees Cook
---
Documentation/admin-guide/pstore-blk.rst | 9 +-
drivers/mtd/Kconfig | 10 +
drivers/mtd/Makefile | 1 +
drivers/mtd/mtdpstore.c | 564 +++
fs/pstore/platform.c | 22 +-
5 files changed, 583 insertions(+), 23 deletions(-)
create mode 100644 drivers/mtd/mtdpstore.c
diff --git a/Documentation/admin-guide/pstore-blk.rst
b/Documentation/admin-guide/pstore-blk.rst
index faf9991879aa..b2a8ab51b0ed 100644
--- a/Documentation/admin-guide/pstore-blk.rst
+++ b/Documentation/admin-guide/pstore-blk.rst
@@ -43,9 +43,9 @@ blkdev
~~
The block device to use. Most of the time, it is a partition of block device.
-It's required for pstore/blk.
+It's required for pstore/blk. It is also used for MTD device.
-It accepts the following variants:
+It accepts the following variants for block device:
1. device number in hexadecimal represents itself; no
leading 0x, for example b302.
@@ -64,6 +64,11 @@ It accepts the following variants:
partition with a known unique id.
#. : major and minor number of the device separated by a colon.
+It accepts the following variants for MTD device:
+
+1. MTD device name. "pstore" is recommended.
+#. MTD device number.
+
kmsg_size
~
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 42d401ea60ee..6ddab796216d 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -170,6 +170,16 @@ config MTD_OOPS
buffer in a flash partition where it can be read back at some
later point.
+config MTD_PSTORE
+ tristate "Log panic/oops to an MTD buffer based on pstore"
+ depends on PSTORE_BLK
+ help
+ This enables panic and oops messages to be logged to a circular
+ buffer in a flash partition where it can be read back as files after
+ mounting pstore filesystem.
+
+ If unsure, say N.
+
config MTD_SWAP
tristate "Swap on MTD device support"
depends on MTD && SWAP
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 56cc60ccc477..593d0593a038 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_RFD_FTL) += rfd_ftl.o
obj-$(CONFIG_SSFDC)+= ssfdc.o
obj-$(CONFIG_SM_FTL) += sm_ftl.o
obj-$(CONFIG_MTD_OOPS) += mtdoops.o
+obj-$(CONFIG_MTD_PSTORE) += mtdpstore.o
obj-$(CONFIG_MTD_SWAP) += mtdswap.o
nftl-objs := nftlcore.o nftlmount.o
diff --git a/drivers/mtd/mtdpstore.c b/drivers/mtd/mtdpstore.c
new file mode 100644
index ..56d4951c404a
--- /dev/null
+++ b/drivers/mtd/mtdpstore.c
@@ -0,0 +1,564 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define dev_fmt(fmt) "mtdoops-pstore: " fmt
+
+#include
+#include
+#include
+#include
+#include
+
+static struct mtdpstore_context {
+ int index;
+ struct pstore_blk_info info;
+ struct psblk_device dev;
+ struct mtd_info *mtd;
+ unsigned long *rmmap; /* removed bit map */
+ unsigned long *usedmap; /* used bit map */
+ /*
+* used for panic write
+* As there are no block_isbad for panic case, we should keep this
+* status before panic to ensure panic_write not failed.
+*/
+ unsigned long *badmap; /* bad block bit map */
+} oops_cxt;
+
+static int mtdpstore_block_isbad(struct mtdpstore_context *cxt, loff_t off)
+{
+ int ret;
+ struct mtd_info *mtd = cxt->mtd;
+ u64 blknum = div_u64(off, mtd->erasesize);
+
+ if (test_bit(blknum, cxt->badmap))
+ return true;
+ ret = mtd_block_isbad(mtd, off);
+ if (ret < 0) {
+ dev_err(>dev, "mtd_block_isbad
[PATCH 15/20] drm_read(): get rid of pointless access_ok()
From: Al Viro address is passed only to copy_to_user() Signed-off-by: Al Viro --- drivers/gpu/drm/drm_file.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index eb009d3ab48f..6a1f6c802415 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -569,9 +569,6 @@ ssize_t drm_read(struct file *filp, char __user *buffer, struct drm_device *dev = file_priv->minor->dev; ssize_t ret; - if (!access_ok(buffer, count)) - return -EFAULT; - ret = mutex_lock_interruptible(_priv->event_read_lock); if (ret) return ret; -- 2.11.0
[PATCH v6 16/18] pstore/blk: Support non-block storage devices
From: WeiXiong Liao
Add support for non-block devices (e.g. MTD). A non-block driver calls
pstore_blk_register_device() to register iself.
In addition, pstore/zone is updated to handle non-block devices,
where an erase must be done before a write. Without this, there is no
way to remove records stored to an MTD.
Signed-off-by: WeiXiong Liao
Link:
https://lore.kernel.org/r/1585126506-18635-11-git-send-email-liaoweixi...@allwinnertech.com
Signed-off-by: Kees Cook
---
Documentation/admin-guide/pstore-blk.rst | 17 -
fs/pstore/blk.c | 96 +---
fs/pstore/zone.c | 8 +-
include/linux/pstore_blk.h | 37 +
include/linux/pstore_zone.h | 6 ++
5 files changed, 115 insertions(+), 49 deletions(-)
diff --git a/Documentation/admin-guide/pstore-blk.rst
b/Documentation/admin-guide/pstore-blk.rst
index bef8c7436721..faf9991879aa 100644
--- a/Documentation/admin-guide/pstore-blk.rst
+++ b/Documentation/admin-guide/pstore-blk.rst
@@ -7,8 +7,8 @@ Introduction
pstore block (pstore/blk) is an oops/panic logger that writes its logs to a
-block device before the system crashes. You can get these log files by
-mounting pstore filesystem like::
+block device and non-block device before the system crashes. You can get
+these log files by mounting pstore filesystem like::
mount -t pstore pstore /sys/fs/pstore
@@ -24,8 +24,8 @@ Configurations for user determine how pstore/blk works, such
as pmsg_size,
kmsg_size and so on. All of them support both Kconfig and module parameters,
but module parameters have priority over Kconfig.
-Configurations for driver are all about block device, such as total_size
-of block device and read/write operations.
+Configurations for driver are all about block device and non-block device,
+such as total_size of block device and read/write operations.
Configurations for user
---
@@ -152,6 +152,15 @@ driver uses ``register_pstore_blk`` to register to
pstore/blk.
.. kernel-doc:: fs/pstore/blk.c
:identifiers: register_pstore_blk
+A non-block device driver uses ``register_pstore_device`` with
+``struct psblk_device`` to register to pstore/blk.
+
+.. kernel-doc:: fs/pstore/blk.c
+ :identifiers: register_pstore_device
+
+.. kernel-doc:: include/linux/pstore_blk.h
+ :identifiers: psblk_device
+
Compression and header
--
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index 134c5e0c67c1..3e67bd4557ea 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -104,55 +104,23 @@ static struct bdev_info {
_##name_; \
})
-/**
- * struct psblk_device - back-end pstore/blk driver structure.
- *
- * @total_size: The total size in bytes pstore/blk can use. It must be greater
- * than 4096 and be multiple of 4096.
- * @flags: Refer to macro starting with PSTORE_FLAGS defined in
- * linux/pstore.h. It means what front-ends this device support.
- * Zero means all backends for compatible.
- * @read: The general read operation. Both of the function parameters
- * @size and @offset are relative value to bock device (not the
- * whole disk).
- * On success, the number of bytes should be returned, others
- * means error.
- * @write: The same as @read, but the following error number:
- * -EBUSY means try to write again later.
- * -ENOMSG means to try next zone.
- * @panic_write:The write operation only used for panic case. It's optional
- * if you do not care panic log. The parameters are relative
- * value to storage.
- * On success, the number of bytes should be returned, others
- * excluding -ENOMSG mean error. -ENOMSG means to try next zone.
- */
-struct psblk_device {
- unsigned long total_size;
- unsigned int flags;
- psz_read_op read;
- psz_write_op write;
- psz_write_op panic_write;
-};
-
-static int psblk_register_do(struct psblk_device *dev)
+static int __register_pstore_device(struct psblk_device *dev)
{
int ret;
- if (!dev || !dev->total_size || !dev->read || !dev->write)
+ if (WARN_ON(!mutex_is_locked(_blk_lock)))
return -EINVAL;
- mutex_lock(_blk_lock);
+ if (!dev || !dev->total_size || !dev->read || !dev->write)
+ return -EINVAL;
/* someone already registered before */
- if (pstore_zone_info) {
- mutex_unlock(_blk_lock);
+ if (pstore_zone_info)
return -EBUSY;
- }
+
pstore_zone_info = kzalloc(sizeof(struct pstore_zone_info), GFP_KERNEL);
- if (!pstore_zone_info) {
- mutex_unlock(_blk_lock);
+ if (!pstore_zone_info)
return -ENOMEM;
- }
/* zero means not limit on which backends to
[PATCH v6 18/18] pstore/blk: Introduce "best_effort" mode
In order to use arbitrary block devices as a pstore backend, provide a
new module param named "best_effort", which will allow using any block
device, even if it has not provided a panic_write callback.
Signed-off-by: Kees Cook
---
fs/pstore/blk.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index 3e67bd4557ea..b7c33ef4c646 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -51,6 +51,10 @@ static long ftrace_size = -1;
module_param(ftrace_size, long, 0400);
MODULE_PARM_DESC(ftrace_size, "ftrace size in kbytes");
+static bool best_effort;
+module_param(best_effort, bool, 0400);
+MODULE_PARM_DESC(best_effort, "use best effort to write (i.e. do not require
storage driver pstore support, default: off)");
+
/*
* blkdev - the block device to use for pstore storage
*
@@ -388,7 +392,7 @@ static int __register_pstore_blk(unsigned int major,
unsigned int flags,
return PTR_ERR(binfo);
/* only allow driver matching the @blkdev */
- if (!binfo->devt || MAJOR(binfo->devt) != major) {
+ if (!binfo->devt || (!best_effort && MAJOR(binfo->devt) != major)) {
pr_debug("invalid major %u (expect %u)\n",
major, MAJOR(binfo->devt));
return -ENODEV;
@@ -532,6 +536,19 @@ int pstore_blk_usr_info(struct pstore_blk_info *info)
}
EXPORT_SYMBOL_GPL(pstore_blk_usr_info);
+static int __init pstore_blk_init(void)
+{
+ int ret = 0;
+
+ mutex_lock(_blk_lock);
+ if (!pstore_zone_info && best_effort && blkdev[0])
+ ret = __register_pstore_blk(0, 0, NULL);
+ mutex_unlock(_blk_lock);
+
+ return ret;
+}
+late_initcall(pstore_blk_init);
+
static void __exit pstore_blk_exit(void)
{
mutex_lock(_blk_lock);
--
2.20.1
[PATCH 16/20] efi_test: get rid of pointless access_ok()
From: Al Viro
really, people - get_user(), copy_from_user(), memdup_user(), etc.
all fail if access_ok() does.
Signed-off-by: Al Viro
---
drivers/firmware/efi/test/efi_test.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/firmware/efi/test/efi_test.c
b/drivers/firmware/efi/test/efi_test.c
index 7baf48c01e72..ddf9eae396fe 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -70,9 +70,6 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t
__user *src,
return 0;
}
- if (!access_ok(src, 1))
- return -EFAULT;
-
buf = memdup_user(src, len);
if (IS_ERR(buf)) {
*dst = NULL;
@@ -91,9 +88,6 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t
__user *src,
static inline int
get_ucs2_strsize_from_user(efi_char16_t __user *src, size_t *len)
{
- if (!access_ok(src, 1))
- return -EFAULT;
-
*len = user_ucs2_strsize(src);
if (*len == 0)
return -EFAULT;
@@ -118,9 +112,6 @@ copy_ucs2_from_user(efi_char16_t **dst, efi_char16_t __user
*src)
{
size_t len;
- if (!access_ok(src, 1))
- return -EFAULT;
-
len = user_ucs2_strsize(src);
if (len == 0)
return -EFAULT;
@@ -142,9 +133,6 @@ copy_ucs2_to_user_len(efi_char16_t __user *dst,
efi_char16_t *src, size_t len)
if (!src)
return 0;
- if (!access_ok(dst, 1))
- return -EFAULT;
-
return copy_to_user(dst, src, len);
}
--
2.11.0
[PATCH 12/20] omapfb: get rid of pointless access_ok() calls
From: Al Viro address is passed only to copy_to_user() Signed-off-by: Al Viro --- drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c index 56995f44e76d..f40be68d5aac 100644 --- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c @@ -482,9 +482,6 @@ static int omapfb_memory_read(struct fb_info *fbi, if (!display || !display->driver->memory_read) return -ENOENT; - if (!access_ok(mr->buffer, mr->buffer_size)) - return -EFAULT; - if (mr->w > 4096 || mr->h > 4096) return -EINVAL; -- 2.11.0
[PATCH 17/20] lpfc_debugfs: get rid of pointless access_ok()
From: Al Viro
No, you do NOT need to "protect copy from user" that way.
Incidentally, your userland ABI stinks. I understand that you
wanted to accept "reset" and "reset\n" as equivalent, but I suspect
that accepting "reset this, you !@^!@!" had been an accident.
Nothing to do about that now - it is a userland ABI...
Signed-off-by: Al Viro
---
drivers/scsi/lpfc/lpfc_debugfs.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 8a6e02aa553f..5a754fb5f854 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -2166,10 +2166,6 @@ lpfc_debugfs_lockstat_write(struct file *file, const
char __user *buf,
char *pbuf;
int i;
- /* Protect copy from user */
- if (!access_ok(buf, nbytes))
- return -EFAULT;
-
memset(mybuf, 0, sizeof(mybuf));
if (copy_from_user(mybuf, buf, nbytes))
@@ -2621,10 +2617,6 @@ lpfc_debugfs_multixripools_write(struct file *file,
const char __user *buf,
if (nbytes > 64)
nbytes = 64;
- /* Protect copy from user */
- if (!access_ok(buf, nbytes))
- return -EFAULT;
-
memset(mybuf, 0, sizeof(mybuf));
if (copy_from_user(mybuf, buf, nbytes))
@@ -2787,10 +2779,6 @@ lpfc_debugfs_scsistat_write(struct file *file, const
char __user *buf,
char mybuf[6] = {0};
int i;
- /* Protect copy from user */
- if (!access_ok(buf, nbytes))
- return -EFAULT;
-
if (copy_from_user(mybuf, buf, (nbytes >= sizeof(mybuf)) ?
(sizeof(mybuf) - 1) : nbytes))
return -EFAULT;
--
2.11.0
[PATCH 08/20] cm4000_cs.c cmm_ioctl(): get rid of pointless access_ok()
From: Al Viro
copy_to_user()/copy_from_user() for everything
Signed-off-by: Al Viro
---
drivers/char/pcmcia/cm4000_cs.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 4edb4174a1e2..89681f07bc78 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -1404,7 +1404,6 @@ static long cmm_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
unsigned int iobase = dev->p_dev->resource[0]->start;
struct inode *inode = file_inode(filp);
struct pcmcia_device *link;
- int size;
int rc;
void __user *argp = (void __user *)arg;
#ifdef CM4000_DEBUG
@@ -1441,19 +1440,6 @@ static long cmm_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
DEBUGP(4, dev, "iocnr mismatch\n");
goto out;
}
- size = _IOC_SIZE(cmd);
- rc = -EFAULT;
- DEBUGP(4, dev, "iocdir=%.4x iocr=%.4x iocw=%.4x iocsize=%d cmd=%.4x\n",
- _IOC_DIR(cmd), _IOC_READ, _IOC_WRITE, size, cmd);
-
- if (_IOC_DIR(cmd) & _IOC_READ) {
- if (!access_ok(argp, size))
- goto out;
- }
- if (_IOC_DIR(cmd) & _IOC_WRITE) {
- if (!access_ok(argp, size))
- goto out;
- }
rc = 0;
switch (cmd) {
--
2.11.0
[PATCH 11/20] amifb: get rid of pointless access_ok() calls
From: Al Viro addresses passed only to get_user() and put_user() Signed-off-by: Al Viro --- drivers/video/fbdev/amifb.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/video/fbdev/amifb.c b/drivers/video/fbdev/amifb.c index 20e03e00b66d..6062104f3afb 100644 --- a/drivers/video/fbdev/amifb.c +++ b/drivers/video/fbdev/amifb.c @@ -1855,8 +1855,6 @@ static int ami_get_var_cursorinfo(struct fb_var_cursorinfo *var, var->yspot = par->crsr.spot_y; if (size > var->height * var->width) return -ENAMETOOLONG; - if (!access_ok(data, size)) - return -EFAULT; delta = 1 << par->crsr.fmode; lspr = lofsprite + (delta << 1); if (par->bplcon0 & BPC0_LACE) @@ -1935,8 +1933,6 @@ static int ami_set_var_cursorinfo(struct fb_var_cursorinfo *var, return -EINVAL; if (!var->height) return -EINVAL; - if (!access_ok(data, var->width * var->height)) - return -EFAULT; delta = 1 << fmode; lofsprite = shfsprite = (u_short *)spritememory; lspr = lofsprite + (delta << 1); -- 2.11.0
[PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
From: Al Viro address is passed only to get_user() Signed-off-by: Al Viro --- security/tomoyo/common.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 1b467381986f..f93f8acd05f7 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -2662,8 +2662,6 @@ ssize_t tomoyo_write_control(struct tomoyo_io_buffer *head, if (!head->write) return -EINVAL; - if (!access_ok(buffer, buffer_len)) - return -EFAULT; if (mutex_lock_interruptible(>io_sem)) return -EINTR; head->read_user_buf_avail = 0; -- 2.11.0
[PATCH 09/20] drivers/fpga/dfl-fme-pr.c: get rid of pointless access_ok()
From: Al Viro followed by copy_from_user() Signed-off-by: Al Viro --- drivers/fpga/dfl-fme-pr.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/fpga/dfl-fme-pr.c b/drivers/fpga/dfl-fme-pr.c index a233a53db708..1194c0e850e0 100644 --- a/drivers/fpga/dfl-fme-pr.c +++ b/drivers/fpga/dfl-fme-pr.c @@ -97,10 +97,6 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg) return -EINVAL; } - if (!access_ok((void __user *)(unsigned long)port_pr.buffer_address, - port_pr.buffer_size)) - return -EFAULT; - /* * align PR buffer per PR bandwidth, as HW ignores the extra padding * data automatically. -- 2.11.0
[PATCH 04/20] FIEMAP: don't bother with access_ok()
From: Al Viro we use copy_to_user() on that thing anyway (and always had). Signed-off-by: Al Viro --- fs/ext4/ioctl.c | 5 - fs/ioctl.c | 5 - 2 files changed, 10 deletions(-) diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index bfc1281fc4cb..a0afd0338722 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -784,11 +784,6 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg) fieinfo.fi_extents_max = fiemap.fm_extent_count; fieinfo.fi_extents_start = ufiemap->fm_extents; - if (fiemap.fm_extent_count != 0 && - !access_ok(fieinfo.fi_extents_start, - fieinfo.fi_extents_max * sizeof(struct fiemap_extent))) - return -EFAULT; - if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC) filemap_write_and_wait(inode->i_mapping); diff --git a/fs/ioctl.c b/fs/ioctl.c index 282d45be6f45..f8181e2c2168 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -215,11 +215,6 @@ static int ioctl_fiemap(struct file *filp, struct fiemap __user *ufiemap) fieinfo.fi_extents_max = fiemap.fm_extent_count; fieinfo.fi_extents_start = ufiemap->fm_extents; - if (fiemap.fm_extent_count != 0 && - !access_ok(fieinfo.fi_extents_start, - fieinfo.fi_extents_max * sizeof(struct fiemap_extent))) - return -EFAULT; - if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC) filemap_write_and_wait(inode->i_mapping); -- 2.11.0
[PATCH 14/20] via-pmu: don't bother with access_ok()
From: Al Viro we are using copy_to_user() for actual copying Signed-off-by: Al Viro --- drivers/macintosh/via-pmu.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c index 83eb05bf85ff..8450d7c008d0 100644 --- a/drivers/macintosh/via-pmu.c +++ b/drivers/macintosh/via-pmu.c @@ -2184,8 +2184,6 @@ pmu_read(struct file *file, char __user *buf, if (count < 1 || !pp) return -EINVAL; - if (!access_ok(buf, count)) - return -EFAULT; spin_lock_irqsave(>lock, flags); add_wait_queue(>wait, ); -- 2.11.0
