Re: [PATCH v6 02/25] objtool: Add a pass for generating __mcount_loc
On Wed, Oct 14, 2020 at 08:21:15PM +0200, Peter Zijlstra wrote: > On Wed, Oct 14, 2020 at 06:50:04PM +0200, Ingo Molnar wrote: > > Meh, adding --mcount as an option to 'objtool check' was a valid hack for a > > prototype patchset, but please turn this into a proper subcommand, just > > like 'objtool orc' is. > > > > 'objtool check' should ... keep checking. :-) > > No, no subcommands. orc being a subcommand was a mistake. Yup, it gets real awkward when trying to combine subcommands. I proposed a more logical design: https://lkml.kernel.org/r/20201002141303.hyl72to37wudoi66@treble -- Josh
[PATCH 1/2] mips: boot compressed: preprocess linker script
Preprocess vmlinuz (self-decompressing kernel ELF) linker script to avoid using ld -Ttext $(address) https://lkml.kernel.org/lkml/20200413153453.zi4jvu3c4ul23...@google.com/ Signed-off-by: John Thomson --- arch/mips/boot/compressed/.gitignore | 1 + arch/mips/boot/compressed/Makefile | 8 ++-- arch/mips/boot/compressed/{ld.script => vmlinuz.lds.S} | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) rename arch/mips/boot/compressed/{ld.script => vmlinuz.lds.S} (96%) diff --git a/arch/mips/boot/compressed/.gitignore b/arch/mips/boot/compressed/.gitignore index d358395614c..1c367a2efb9 100644 --- a/arch/mips/boot/compressed/.gitignore +++ b/arch/mips/boot/compressed/.gitignore @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only ashldi3.c bswapsi.c +vmlinuz.lds diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile index 6e56caef69f..49d1adceade 100644 --- a/arch/mips/boot/compressed/Makefile +++ b/arch/mips/boot/compressed/Makefile @@ -96,11 +96,15 @@ UIMAGE_LOADADDR = $(VMLINUZ_LOAD_ADDRESS) vmlinuzobjs-y += $(obj)/piggy.o +targets += vmlinuz.lds +$(obj)/vmlinuz.lds: $(obj)/calc_vmlinuz_load_addr $(obj)/vmlinux.bin +CPPFLAGS_vmlinuz.lds = -DVMLINUZ_LOAD_ADDRESS="$(VMLINUZ_LOAD_ADDRESS)" + quiet_cmd_zld = LD $@ - cmd_zld = $(LD) $(KBUILD_LDFLAGS) -Ttext $(VMLINUZ_LOAD_ADDRESS) -T $< $(vmlinuzobjs-y) -o $@ + cmd_zld = $(LD) $(KBUILD_LDFLAGS) -T $< $(vmlinuzobjs-y) -o $@ quiet_cmd_strip = STRIP $@ cmd_strip = $(STRIP) -s $@ -vmlinuz: $(src)/ld.script $(vmlinuzobjs-y) $(obj)/calc_vmlinuz_load_addr +vmlinuz: $(obj)/vmlinuz.lds $(vmlinuzobjs-y) $(obj)/calc_vmlinuz_load_addr $(call cmd,zld) $(call cmd,strip) diff --git a/arch/mips/boot/compressed/ld.script b/arch/mips/boot/compressed/vmlinuz.lds.S similarity index 96% rename from arch/mips/boot/compressed/ld.script rename to arch/mips/boot/compressed/vmlinuz.lds.S index 2ed08fbef8e..890c31c55c1 100644 --- a/arch/mips/boot/compressed/ld.script +++ b/arch/mips/boot/compressed/vmlinuz.lds.S @@ -14,7 +14,7 @@ PHDRS { SECTIONS { /* Text and read-only data */ - /* . = VMLINUZ_LOAD_ADDRESS; */ + . = VMLINUZ_LOAD_ADDRESS; .text : { *(.text) *(.rodata) -- 2.28.0
[PATCH 2/2] mips: boot compressed: add support for vlinuz ELF DTB
For legacy bootloader devices that do not support DTB, and only support booting ELF, or have issues booting large ELF files. vmlinux (objcopy to bytecode then compressed for vmlinuz) requires MIPS_RAW_APPENDED_DTB, then vmlinuz may use MIPS_ELF_APPENDED_DTB_VMLINUZ, and insert the DTB into the ELF: objcopy --update-section .appended_dtb=.dtb vmlinuz Signed-off-by: John Thomson --- arch/mips/Kconfig | 21 + arch/mips/boot/compressed/decompress.c | 4 arch/mips/boot/compressed/vmlinuz.lds.S | 9 + 3 files changed, 34 insertions(+) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 8f328298f8c..2749d46be9e 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -3098,6 +3098,27 @@ choice if you don't intend to always append a DTB. endchoice +config MIPS_ELF_APPENDED_DTB_VMLINUZ + bool "vmlinuz (ELF self-decompressing kernel) appended DTB support" + depends on MIPS_RAW_APPENDED_DTB && SYS_SUPPORTS_ZBOOT + help + With this option, the vmlinuz (self-decompressing kernel ELF binary) + boot code will look for a device tree binary (DTB|FDT) included in + the vmlinux ELF section .appended_dtb. By default it is empty and + the DTB can be appended using binutils command + objcopy: + + objcopy --update-section .appended_dtb=.dtb vmlinuz + + This is meant as a backward compatiblity convenience for those + systems with a bootloader that can't be upgraded to accommodate + the documented boot protocol using a device tree. + + vmlinuz uses the compressed vmlinux.bin, thus vmlinux must use + MIPS_RAW_APPENDED_DTB to expect a DTB at the end-of-uncompressed + kernel location. vmlinuz copies the DTB to this location after kernel + decompression. + choice prompt "Kernel command line type" if !CMDLINE_OVERRIDE default MIPS_CMDLINE_FROM_DTB if USE_OF && !ATH79 && !MACH_INGENIC && \ diff --git a/arch/mips/boot/compressed/decompress.c b/arch/mips/boot/compressed/decompress.c index 88f5d637b1c..cd1a47c69e3 100644 --- a/arch/mips/boot/compressed/decompress.c +++ b/arch/mips/boot/compressed/decompress.c @@ -33,7 +33,11 @@ extern void puthex(unsigned long long val); #define puthex(val) do {} while (0) #endif +#ifdef CONFIG_MIPS_ELF_APPENDED_DTB_VMLINUZ +unsigned char __section(.appended_dtb) __appended_dtb[0x10]; +#else extern char __appended_dtb[]; +#endif void error(char *x) { diff --git a/arch/mips/boot/compressed/vmlinuz.lds.S b/arch/mips/boot/compressed/vmlinuz.lds.S index 890c31c55c1..f5a35b60059 100644 --- a/arch/mips/boot/compressed/vmlinuz.lds.S +++ b/arch/mips/boot/compressed/vmlinuz.lds.S @@ -31,9 +31,18 @@ SECTIONS CONSTRUCTORS . = ALIGN(16); } + +#ifdef CONFIG_MIPS_ELF_APPENDED_DTB_VMLINUZ + /* keep the empty unreferenced ELF DTB section */ + .appended_dtb : { + *(.appended_dtb) + KEEP(*(.appended_dtb)) + } +#else __appended_dtb = .; /* leave space for appended DTB */ . += 0x10; +#endif /* _APPENDED_DTB */ _edata = .; /* End of data section */ -- 2.28.0
Re: [PATCH v6 2/2] PCI/ERR: Split the fatal and non-fatal error recovery handling
On 10/15/20 6:55 AM, Ethan Zhao wrote: On Wed, Oct 14, 2020 at 5:00 PM Kuppuswamy Sathyanarayanan wrote: Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") merged fatal and non-fatal error recovery paths, and also made recovery code depend on hotplug handler for "remove affected device + rescan" support. But this change also complicated the error recovery path and which in turn led to the following issues. 1. We depend on hotplug handler for removing the affected devices/drivers on DLLSC LINK down event (on DPC event trigger) and DPC handler for handling the error recovery. Since both handlers operate on same set of affected devices, it leads to race condition, which in turn leads to NULL pointer exceptions or error recovery failures.You can find more details about this issue in following link. https://lore.kernel.org/linux-pci/20201007113158.48933-1-haifeng.z...@intel.com/T/#t 2. For non-hotplug capable devices fatal (DPC) error recovery is currently broken. Current fatal error recovery implementation relies on PCIe hotplug (pciehp) handler for detaching and re-enumerating the affected devices/drivers. So when dealing with non-hotplug capable devices, recovery code does not restore the state of the affected devices correctly. You can find more details about this issue in the following links. https://lore.kernel.org/linux-pci/20200527083130.4137-1-zhiqiang@nxp.com/ https://lore.kernel.org/linux-pci/12115.1588207324@famine/ https://lore.kernel.org/linux-pci/0e6f89cd6b9e4a72293cc90fafe93487d7c2d295.158584.git.sathyanarayanan.kuppusw...@linux.intel.com/ In order to fix the above two issues, we should stop relying on hotplug handler for cleaning the affected devices/drivers and let error recovery handler own this functionality. So this patch reverts Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") and re-introduce the "remove affected device + rescan" functionality in fatal error recovery handler. Also holding pci_lock_rescan_remove() will prevent the race between hotplug and DPC handler. Fixes: bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") Signed-off-by: Kuppuswamy Sathyanarayanan Reviewed-by: Sinan Kaya --- Changes since v4: * Added new interfaces for error recovery (pcie_do_fatal_recovery() and pcie_do_nonfatal_recovery()). Changes since v5: * Fixed static/non-static declartion issue. Documentation/PCI/pci-error-recovery.rst | 47 ++-- Documentation/PCI/pcieaer-howto.rst | 2 +- drivers/pci/pci.h| 5 +- drivers/pci/pcie/aer.c | 10 ++-- drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/edr.c | 2 +- drivers/pci/pcie/err.c | 70 ++-- 7 files changed, 94 insertions(+), 44 deletions(-) diff --git a/Documentation/PCI/pci-error-recovery.rst b/Documentation/PCI/pci-error-recovery.rst index 84ceebb08cac..830c8af5838b 100644 --- a/Documentation/PCI/pci-error-recovery.rst +++ b/Documentation/PCI/pci-error-recovery.rst @@ -115,7 +115,7 @@ The actual steps taken by a platform to recover from a PCI error event will be platform-dependent, but will follow the general sequence described below. -STEP 0: Error Event +STEP 0: Error Event: ERR_NONFATAL --- A PCI bus error is detected by the PCI hardware. On powerpc, the slot is isolated, in that all I/O is blocked: all reads return 0x, @@ -160,10 +160,10 @@ particular, if the platform doesn't isolate slots), and recovery proceeds to STEP 2 (MMIO Enable). If any driver requested a slot reset (by returning PCI_ERS_RESULT_NEED_RESET), -then recovery proceeds to STEP 4 (Slot Reset). +then recovery proceeds to STEP 3 (Slot Reset). If the platform is unable to recover the slot, the next step -is STEP 6 (Permanent Failure). +is STEP 5 (Permanent Failure). .. note:: @@ -198,7 +198,7 @@ reset or some such, but not restart operations. This callback is made if all drivers on a segment agree that they can try to recover and if no automatic link reset was performed by the HW. If the platform can't just re-enable IOs without a slot reset or a link reset, it will not call this callback, and -instead will have gone directly to STEP 3 (Link Reset) or STEP 4 (Slot Reset) +instead will have gone directly to STEP 3 (Slot Reset) .. note:: @@ -233,18 +233,12 @@ The driver should return one of the following result codes: The next step taken depends on the results returned by the drivers. If all drivers returned PCI_ERS_RESULT_RECOVERED, then the platform -proceeds to either STEP3 (Link Reset) or to STEP 5 (Resume Operations). +proceeds to STEP 4 (Resume Operations). If any driver returned PCI_ERS_RESULT_NEED_RESET, then the platform -proceeds to STEP 4 (Slot Reset) +proceeds to STEP 3 (Slot Reset) -STEP 3: Link Reset --- -The platform resets the link. This is a PCI-Express specific step -and
Re: [PATCH] NFS: Fix mode bits and nlink count for v4 referral dirs
On Thu, Oct 15, 2020 at 7:38 PM Chuck Lever wrote: > > > > > On Oct 15, 2020, at 9:59 AM, Trond Myklebust > > wrote: > > > > On Thu, 2020-10-15 at 09:36 -0400, Chuck Lever wrote: > >>> On Oct 15, 2020, at 8:06 AM, Trond Myklebust < > >>> tron...@hammerspace.com> wrote: > >>> > >>> On Thu, 2020-10-15 at 00:39 +0530, Ashish Sangwan wrote: > On Wed, Oct 14, 2020 at 11:47 PM Trond Myklebust > wrote: > > On Tue, 2020-10-06 at 08:14 -0700, Ashish Sangwan wrote: > >> Request for mode bits and nlink count in the > >> nfs4_get_referral > >> call > >> and if server returns them use them instead of hard coded > >> values. > >> > >> CC: sta...@vger.kernel.org > >> Signed-off-by: Ashish Sangwan > >> --- > >> fs/nfs/nfs4proc.c | 20 +--- > >> 1 file changed, 17 insertions(+), 3 deletions(-) > >> > >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > >> index 6e95c85fe395..efec05c5f535 100644 > >> --- a/fs/nfs/nfs4proc.c > >> +++ b/fs/nfs/nfs4proc.c > >> @@ -266,7 +266,9 @@ const u32 nfs4_fs_locations_bitmap[3] = { > >> | FATTR4_WORD0_FSID > >> | FATTR4_WORD0_FILEID > >> | FATTR4_WORD0_FS_LOCATIONS, > >> - FATTR4_WORD1_OWNER > >> + FATTR4_WORD1_MODE > >> + | FATTR4_WORD1_NUMLINKS > >> + | FATTR4_WORD1_OWNER > >> | FATTR4_WORD1_OWNER_GROUP > >> | FATTR4_WORD1_RAWDEV > >> | FATTR4_WORD1_SPACE_USED > >> @@ -7594,16 +7596,28 @@ nfs4_listxattr_nfs4_user(struct inode > >> *inode, > >> char *list, size_t list_len) > >> */ > >> static void nfs_fixup_referral_attributes(struct nfs_fattr > >> *fattr) > >> { > >> + bool fix_mode = true, fix_nlink = true; > >> + > >> if (!(((fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID) > >> || > >>(fattr->valid & NFS_ATTR_FATTR_FILEID)) && > >> (fattr->valid & NFS_ATTR_FATTR_FSID) && > >> (fattr->valid & NFS_ATTR_FATTR_V4_LOCATIONS))) > >> return; > >> > >> + if (fattr->valid & NFS_ATTR_FATTR_MODE) > >> + fix_mode = false; > >> + if (fattr->valid & NFS_ATTR_FATTR_NLINK) > >> + fix_nlink = false; > >> fattr->valid |= NFS_ATTR_FATTR_TYPE | > >> NFS_ATTR_FATTR_MODE | > >> NFS_ATTR_FATTR_NLINK | > >> NFS_ATTR_FATTR_V4_REFERRAL; > >> - fattr->mode = S_IFDIR | S_IRUGO | S_IXUGO; > >> - fattr->nlink = 2; > >> + > >> + if (fix_mode) > >> + fattr->mode = S_IFDIR | S_IRUGO | S_IXUGO; > >> + else > >> + fattr->mode |= S_IFDIR; > >> + > >> + if (fix_nlink) > >> + fattr->nlink = 2; > >> } > >> > >> static int _nfs4_proc_fs_locations(struct rpc_clnt *client, > >> struct > >> inode *dir, > > > > NACK to this patch. The whole point is that if the server has a > > referral, then it is not going to give us any attributes other > > than > > the > > ones we're already asking for because it may not even have a > > real > > directory. The client is required to fake up an inode, hence > > the > > existing code. > > Hi Trond, thanks for reviewing the patch! > Sorry but I didn't understand the reason to NACK it. Could you > please > elaborate your concern? > These are the current attributes we request from the server on a > referral: > FATTR4_WORD0_CHANGE > > FATTR4_WORD0_SIZE > > FATTR4_WORD0_FSID > > FATTR4_WORD0_FILEID > > FATTR4_WORD0_FS_LOCATIONS, > FATTR4_WORD1_OWNER > > FATTR4_WORD1_OWNER_GROUP > > FATTR4_WORD1_RAWDEV > > FATTR4_WORD1_SPACE_USED > > FATTR4_WORD1_TIME_ACCESS > > FATTR4_WORD1_TIME_METADATA > > FATTR4_WORD1_TIME_MODIFY > > FATTR4_WORD1_MOUNTED_ON_FILEID, > > So you are suggesting that it's ok to ask for SIZE, OWNER, OWNER > GROUP, SPACE USED, TIMESTAMPs etc but not ok to ask for mode bits > and > numlinks? > >>> > >>> No. We shouldn't be asking for any of that information for a > >>> referral > >>> because the server isn't supposed to return any values for it. > >>> > >>> Chuck and Anna, what's the deal with commit c05cefcc7241? That > >>> appears > >>> to have changed the original code to speculatively assume that the > >>> server will violate RFC5661 Section 11.3.1 and/or RFC7530 Section > >>> 8.3.1. > >> > >> The commit is an attempt to address the many complaints we've had > >> about the ugly appearance of referral anchors. The strange "special" > >> default values made the client appear to be broken, and was confusing > >> to some. I consider this to be a UX issue: the information displayed > >> in this case is not meant to be factual, but rather to prevent the > >> user concluding that something is wrong. > >> > >> I'm not attached to this particular
Re: [PATCH v3 2/2] vhost-vdpa: fix page pinning leakage in error path
On 10/15/2020 6:11 AM, Michael S. Tsirkin wrote: On Thu, Oct 15, 2020 at 02:15:32PM +0800, Jason Wang wrote: On 2020/10/14 上午7:42, si-wei liu wrote: So what I suggest is to fix the pinning leakage first and do the possible optimization on top (which is still questionable to me). OK. Unfortunately, this was picked and got merged in upstream. So I will post a follow up patch set to 1) revert the commit to the original __get_free_page() implementation, and 2) fix the accounting and leakage on top. Will it be fine? Fine. Thanks Fine by me too. Thanks, Michael & Jason. I will post the fix shortly. Stay tuned. -Siwei
Re: [PATCH v2 4/9] rpmsg: Move rpmsg_hr and rpmsg_ns_msg to header file
On Thu, Oct 15, 2020 at 10:33:25AM +0200, Arnaud POULIQUEN wrote: > Hi Mathieu, > > On 10/14/20 1:25 AM, Mathieu Poirier wrote: > > Move structures rpmsg_hdr and rpmsg_ns_msg to their own header file > > so that they can be used by other entities. > > > > Signed-off-by: Mathieu Poirier > > --- > > drivers/rpmsg/virtio_rpmsg_bus.c | 58 ++ > > include/linux/rpmsg_ns.h | 62 > > include/uapi/linux/rpmsg.h | 3 ++ > > 3 files changed, 67 insertions(+), 56 deletions(-) > > create mode 100644 include/linux/rpmsg_ns.h > > > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c > > b/drivers/rpmsg/virtio_rpmsg_bus.c > > index 793fe924671f..85f2acc4ed9f 100644 > > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > > @@ -19,7 +19,7 @@ > > #include > > #include > > #include > > -#include > > +#include > > #include > > #include > > #include > > @@ -27,6 +27,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "rpmsg_internal.h" > > > > @@ -70,58 +71,6 @@ struct virtproc_info { > > struct rpmsg_endpoint *ns_ept; > > }; > > > > -/* The feature bitmap for virtio rpmsg */ > > -#define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */ > > - > > -/** > > - * struct rpmsg_hdr - common header for all rpmsg messages > > - * @src: source address > > - * @dst: destination address > > - * @reserved: reserved for future use > > - * @len: length of payload (in bytes) > > - * @flags: message flags > > - * @data: @len bytes of message payload data > > - * > > - * Every message sent(/received) on the rpmsg bus begins with this header. > > - */ > > -struct rpmsg_hdr { > > - __rpmsg32 src; > > - __rpmsg32 dst; > > - __rpmsg32 reserved; > > - __rpmsg16 len; > > - __rpmsg16 flags; > > - u8 data[]; > > -} __packed; > > - > > -/** > > - * struct rpmsg_ns_msg - dynamic name service announcement message > > - * @name: name of remote service that is published > > - * @addr: address of remote service that is published > > - * @flags: indicates whether service is created or destroyed > > - * > > - * This message is sent across to publish a new service, or announce > > - * about its removal. When we receive these messages, an appropriate > > - * rpmsg channel (i.e device) is created/destroyed. In turn, the ->probe() > > - * or ->remove() handler of the appropriate rpmsg driver will be invoked > > - * (if/as-soon-as one is registered). > > - */ > > -struct rpmsg_ns_msg { > > - char name[RPMSG_NAME_SIZE]; > > - __rpmsg32 addr; > > - __rpmsg32 flags; > > -} __packed; > > - > > -/** > > - * enum rpmsg_ns_flags - dynamic name service announcement flags > > - * > > - * @RPMSG_NS_CREATE: a new remote service was just created > > - * @RPMSG_NS_DESTROY: a known remote service was just destroyed > > - */ > > -enum rpmsg_ns_flags { > > - RPMSG_NS_CREATE = 0, > > - RPMSG_NS_DESTROY= 1, > > -}; > > - > > /** > > * @vrp: the remote processor this channel belongs to > > */ > > @@ -162,9 +111,6 @@ struct virtio_rpmsg_channel { > > */ > > #define RPMSG_RESERVED_ADDRESSES (1024) > > > > -/* Address 53 is reserved for advertising remote services */ > > -#define RPMSG_NS_ADDR (53) > > - > > static void virtio_rpmsg_destroy_ept(struct rpmsg_endpoint *ept); > > static int virtio_rpmsg_send(struct rpmsg_endpoint *ept, void *data, int > > len); > > static int virtio_rpmsg_sendto(struct rpmsg_endpoint *ept, void *data, int > > len, > > diff --git a/include/linux/rpmsg_ns.h b/include/linux/rpmsg_ns.h > > new file mode 100644 > > index ..3d836b8580b2 > > --- /dev/null > > +++ b/include/linux/rpmsg_ns.h > > @@ -0,0 +1,62 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +#ifndef _LINUX_RPMSG_NS_H > > +#define _LINUX_RPMSG_NS_H > > + > > +#include > > +#include > > +#include > > + > > +/** > > + * struct rpmsg_hdr - common header for all rpmsg messages > > + * @src: source address > > + * @dst: destination address > > + * @reserved: reserved for future use > > + * @len: length of payload (in bytes) > > + * @flags: message flags > > + * @data: @len bytes of message payload data > > + * > > + * Every message sent(/received) on the rpmsg bus begins with this header. > > + */ > > +struct rpmsg_hdr { > > + __rpmsg32 src; > > + __rpmsg32 dst; > > + __rpmsg32 reserved; > > + __rpmsg16 len; > > + __rpmsg16 flags; > > + u8 data[]; > > +} __packed; > > This structure is not related to the rpmsg ns service but to the rpmsg bus. > If this structure has to be exposed to rpmsg client should be in rpmsg.h, but > Is there a need to expose it for now? > I suppose that it is for vhost...As the need will depends on the > implementation, > I would suggest leaving it internally and expose only if needed, in the > related series. > I also thought about moving rpmsg_hdr to rpmsg.h but decided against
Re: [PATCH v1 02/29] virtio-mem: simplify calculation in virtio_mem_mb_state_prepare_next_mb()
> We actually need one byte less (next_mb_id is exclusive, first_mb_id is > inclusive). Simplify. > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Pankaj Gupta > Signed-off-by: David Hildenbrand > --- > drivers/virtio/virtio_mem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c > index a1f5bf7a571a..670b3faf412d 100644 > --- a/drivers/virtio/virtio_mem.c > +++ b/drivers/virtio/virtio_mem.c > @@ -257,8 +257,8 @@ static enum virtio_mem_mb_state > virtio_mem_mb_get_state(struct virtio_mem *vm, > */ > static int virtio_mem_mb_state_prepare_next_mb(struct virtio_mem *vm) > { > - unsigned long old_bytes = vm->next_mb_id - vm->first_mb_id + 1; > - unsigned long new_bytes = vm->next_mb_id - vm->first_mb_id + 2; > + unsigned long old_bytes = vm->next_mb_id - vm->first_mb_id; > + unsigned long new_bytes = old_bytes + 1; Maybe we can avoid new_bytes & old_bytes variables, instead use single variable. Can later be used with PFN_UP/PFN_DOWN. > int old_pages = PFN_UP(old_bytes); > int new_pages = PFN_UP(new_bytes); > uint8_t *new_mb_state;
[GIT PULL] perf tools changes for v5.10
Hi Linus, Please consider pulling, Best regards, - Arnaldo The following changes since commit fb0155a09b0224a7147cb07a4ce6034c8d29667f: Merge tag 'nfs-for-5.9-3' of git://git.linux-nfs.org/projects/trondmy/linux-nfs (2020-09-28 11:05:56 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-tools-for-v5.10-2020-10-15 for you to fetch changes up to 744aec4df2c5b4d12af26a57d8858af2f59ef3d0: perf c2c: Update documentation for metrics reorganization (2020-10-15 12:02:12 -0300) perf tools changes for v5.10: 1st batch - cgroup improvements for 'perf stat', allowing for compact specification of events and cgroups in the command line. - Support per thread topdown metrics in 'perf stat'. - Support sample-read topdown metric group in 'perf record' - Show start of latency in addition to its start in 'perf sched latency'. - Add min, max to 'perf script' futex-contention output, in addition to avg. - Allow usage of 'perf_event_attr->exclusive' attribute via the new ':e' event modifier. - Add 'snapshot' command to 'perf record --control', using it with Intel PT. - Support FIFO file names as alternative options to 'perf record --control'. - Introduce branch history "streams", to compare 'perf record' runs with 'perf diff' based on branch records and report hot streams. - Support PE executable symbol tables using libbfd, to profile, for instance, wine binaries. - Add filter support for option 'perf ftrace -F/--funcs'. - Allow configuring the 'disassembler_style' 'perf annotate' knob via 'perf config' - Update CascadelakeX and SkylakeX JSON vendor events files. - Add support for parsing perchip/percore JSON vendor events. - Add power9 hv_24x7 core level metric events. - Add L2 prefetch, ITLB instruction fetch hits JSON events for AMD zen1. - Enable Family 19h users by matching Zen2 AMD vendor events. - Use debuginfod in 'perf probe' when required debug files not found locally. - Display negative tid in non-sample events in 'perf script'. - Make GTK2 support opt-in - Add build test with GTK+ - Add missing -lzstd to the fast path feature detection - Add scripts to auto generate 'mmap', 'mremap' string<->id tables for use in 'perf trace'. - Show python test script in verbose mode. - Fix uncore metric expressions - Msan uninitialized use fixes. - Use condition variables in 'perf bench numa' - Autodetect python3 binary in systems without python2. - Support md5 build ids in addition to sha1. - Add build id 'perf test' regression test. - Fix printable strings in python3 scripts. - Fix off by ones in 'perf trace' in arches using libaudit. - Fix JSON event code for events referencing std arch events. - Introduce 'perf test' shell script for Arm CoreSight testing. - Add rdtsc() for Arm64 for used in the PERF_RECORD_TIME_CONV metadata event and in 'perf test tsc'. - 'perf c2c' improvements: Add "RMT Load Hit" metric, "Total Stores", fixes and documentation update. - Fix usage of reloc_sym in 'perf probe' when using both kallsyms and debuginfo files. - Do not print 'Metric Groups:' unnecessarily in 'perf list' - Refcounting fixes in the event parsing code. - Add expand cgroup event 'perf test' entry. - Fix out of bounds CPU map access when handling armv8_pmu events in 'perf stat'. - Add build-id injection 'perf bench' benchmark. - Enter namespace when reading build-id in 'perf inject'. - Do not load map/dso when injecting build-id speeding up the 'perf inject' process. - Add --buildid-all option to avoid processing all samples, just the mmap metadata events. - Add feature test to check if libbfd has buildid support - Add 'perf test' entry for PE binary format support. - Fix typos in power8 PMU vendor events JSON files. - Hide libtraceevent non API functions. Signed-off-by: Arnaldo Carvalho de Melo Test results in the signed tag. Adrian Hunter (9): perf tools: Consolidate --control option parsing into one function perf tools: Handle read errors from ctl_fd perf tools: Use AsciiDoc formatting for --control option documentation perf tools: Add FIFO file names as alternative options to --control perf record: Add 'snapshot' control command perf intel-pt: Document snapshot control command perf tools: Consolidate close_control_option()'s into one function perf script: Display negative tid in non-sample events perf intel-pt: Fix "context_switch event has no tid" error Andi Kleen (4): perf stat: Support new per thread TopDown metrics perf tools: Add documentation for topdown metrics perf tools: Add support for exclusive groups/events perf intel-pt: Improve PT documentation slightly Arnaldo Carvalho de Melo (16): tools features: Add feature test to check if libbfd has buildid support perf annotate: Allow configuring the
Re: 5.10-rc0: build error in ipi.c
On Thu 2020-10-15 20:41:32, Marc Zyngier wrote: > On 2020-10-15 18:18, Pavel Machek wrote: > > Hi! > > > > > > > I'm getting build problems in 5.10-rc0 in config for n900. ARM board. > > > > > > > > > > CONFIG_SMP=y > > > > > CONFIG_SMP_ON_UP=y > > > > > > On its own, this doesn't break anything with multi_v7_defconfig. > > > > I sent config off-list. Let me know if it does not arrive or if you > > need more info. > > Try this for size: > > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig > index 10a5aff4eecc..db923e0da162 100644 > --- a/kernel/irq/Kconfig > +++ b/kernel/irq/Kconfig > @@ -81,6 +81,7 @@ config IRQ_FASTEOI_HIERARCHY_HANDLERS > > # Generic IRQ IPI support > config GENERIC_IRQ_IPI > + select IRQ_DOMAIN_HIERARCHY > bool > > # Generic MSI interrupt support > My OCD prevents me from doing that! :-)... select needs to be moved line below, for consistency. And yes, this fixes it for me. Tested-by: Pavel Machek Pavel -- http://www.livejournal.com/~pavelmachek signature.asc Description: PGP signature
Re: [Cocci] [PATCH V3] coccinelle: iterators: Add for_each_child.cocci script
On Thu, 15 Oct 2020, Sumera Priyadarsini wrote: > While iterating over child nodes with the for_each functions, if > control is transferred from the middle of the loop, as in the case > of a break or return or goto, there is no decrement in the > reference counter thus ultimately resulting in a memory leak. > > Add this script to detect potential memory leaks caused by > the absence of of_node_put() before break, goto, or, return > statements which transfer control outside the loop. > > Signed-off-by: Sumera Priyadarsini Applied, thanks. julia > > > Changes in V2: > - Add options --include-headers and --no-includes > - Add 'when forall` to rules for break and goto > > Changes in V3: > - Add return case > --- > .../coccinelle/iterators/for_each_child.cocci | 358 ++ > 1 file changed, 358 insertions(+) > create mode 100644 scripts/coccinelle/iterators/for_each_child.cocci > > diff --git a/scripts/coccinelle/iterators/for_each_child.cocci > b/scripts/coccinelle/iterators/for_each_child.cocci > new file mode 100644 > index ..bc394615948e > --- /dev/null > +++ b/scripts/coccinelle/iterators/for_each_child.cocci > @@ -0,0 +1,358 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// Adds missing of_node_put() before return/break/goto statement within a > for_each iterator for child nodes. > +//# False positives can be due to function calls within the for_each > +//# loop that may encapsulate an of_node_put. > +/// > +// Confidence: High > +// Copyright: (C) 2020 Sumera Priyadarsini > +// URL: http://coccinelle.lip6.fr > +// Options: --no-includes --include-headers > + > +virtual patch > +virtual context > +virtual org > +virtual report > + > +@r@ > +local idexpression n; > +expression e1,e2; > +iterator name for_each_node_by_name, for_each_node_by_type, > +for_each_compatible_node, for_each_matching_node, > +for_each_matching_node_and_match, for_each_child_of_node, > +for_each_available_child_of_node, for_each_node_with_property; > +iterator i; > +statement S; > +expression list [n1] es; > +@@ > + > +( > +( > +for_each_node_by_name(n,e1) S > +| > +for_each_node_by_type(n,e1) S > +| > +for_each_compatible_node(n,e1,e2) S > +| > +for_each_matching_node(n,e1) S > +| > +for_each_matching_node_and_match(n,e1,e2) S > +| > +for_each_child_of_node(e1,n) S > +| > +for_each_available_child_of_node(e1,n) S > +| > +for_each_node_with_property(n,e1) S > +) > +& > +i(es,n,...) S > +) > + > +@ruleone depends on patch && !context && !org && !report@ > + > +local idexpression r.n; > +iterator r.i,i1; > +expression e; > +expression list [r.n1] es; > +statement S; > +@@ > + > + i(es,n,...) { > + ... > +( > + of_node_put(n); > +| > + e = n > +| > + return n; > +| > + i1(...,n,...) S > +| > +- return of_node_get(n); > ++ return n; > +| > ++ of_node_put(n); > +? return ...; > +) > + ... when any > + } > + > +@ruletwo depends on patch && !context && !org && !report@ > + > +local idexpression r.n; > +iterator r.i,i1,i2; > +expression e,e1; > +expression list [r.n1] es; > +statement S,S2; > +@@ > + > + i(es,n,...) { > + ... > +( > + of_node_put(n); > +| > + e = n > +| > + i1(...,n,...) S > +| > ++ of_node_put(n); > +? break; > +) > + ... when any > + } > +... when != n > +when strict > +when forall > +( > + n = e1; > +| > +?i2(...,n,...) S2 > +) > + > +@rulethree depends on patch && !context && !org && !report exists@ > + > +local idexpression r.n; > +iterator r.i,i1,i2; > +expression e,e1; > +identifier l; > +expression list [r.n1] es; > +statement S,S2; > +@@ > + > + i(es,n,...) { > + ... > +( > + of_node_put(n); > +| > + e = n > +| > + i1(...,n,...) S > +| > ++ of_node_put(n); > +? goto l; > +) > + ... when any > + } > +... when exists > +l: ... when != n > + when strict > + when forall > +( > + n = e1; > +| > +?i2(...,n,...) S2 > +) > + > +// > > + > +@ruleone_context depends on !patch && (context || org || report) exists@ > +statement S; > +expression e; > +expression list[r.n1] es; > +iterator r.i, i1; > +local idexpression r.n; > +position j0, j1; > +@@ > + > + i@j0(es,n,...) { > + ... > +( > + of_node_put(n); > +| > + e = n > +| > + return n; > +| > + i1(...,n,...) S > +| > + return @j1 ...; > +) > + ... when any > + } > + > +@ruleone_disj depends on !patch && (context || org || report)@ > +expression list[r.n1] es; > +iterator r.i; > +local idexpression r.n; > +position ruleone_context.j0, ruleone_context.j1; > +@@ > + > +* i@j0(es,n,...) { > + ... > +*return @j1...; > + ... when any > + } > + > +@ruletwo_context depends on !patch && (context || org || report) exists@ > +statement S, S2; > +expression e, e1; > +expression list[r.n1] es; > +iterator r.i, i1, i2; > +local idexpression r.n; > +position j0, j2; > +@@ > + > + i@j0(es,n,...) { > + ... > +( > + of_node_put(n); > +| > + e = n > +| > + i1(...,n,...) S > +| > +
Re: [PATCH v1 11/29] virtio-mem: use "unsigned long" for nr_pages when fake onlining/offlining
> No harm done, but let's be consistent. > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Pankaj Gupta > Signed-off-by: David Hildenbrand > --- > drivers/virtio/virtio_mem.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c > index cb2e8f254650..00d1cfca4713 100644 > --- a/drivers/virtio/virtio_mem.c > +++ b/drivers/virtio/virtio_mem.c > @@ -766,7 +766,7 @@ static int virtio_mem_memory_notifier_cb(struct > notifier_block *nb, > * (via generic_online_page()) using PageDirty(). > */ > static void virtio_mem_set_fake_offline(unsigned long pfn, > - unsigned int nr_pages, bool onlined) > + unsigned long nr_pages, bool onlined) > { > for (; nr_pages--; pfn++) { > struct page *page = pfn_to_page(pfn); > @@ -785,7 +785,7 @@ static void virtio_mem_set_fake_offline(unsigned long pfn, > * (via generic_online_page()), clear PageDirty(). > */ > static void virtio_mem_clear_fake_offline(unsigned long pfn, > - unsigned int nr_pages, bool onlined) > + unsigned long nr_pages, bool > onlined) > { > for (; nr_pages--; pfn++) { > struct page *page = pfn_to_page(pfn); > @@ -800,10 +800,10 @@ static void virtio_mem_clear_fake_offline(unsigned long > pfn, > * Release a range of fake-offline pages to the buddy, effectively > * fake-onlining them. > */ > -static void virtio_mem_fake_online(unsigned long pfn, unsigned int nr_pages) > +static void virtio_mem_fake_online(unsigned long pfn, unsigned long nr_pages) > { > const unsigned long max_nr_pages = MAX_ORDER_NR_PAGES; > - int i; > + unsigned long i; > > /* > * We are always called at least with MAX_ORDER_NR_PAGES Reviewed-by: Pankaj Gupta
Re: [PATCH v1 06/29] virtio-mem: generalize virtio_mem_owned_mb()
> Avoid using memory block ids. Rename it to virtio_mem_contains_range(). > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Pankaj Gupta > Signed-off-by: David Hildenbrand > --- > drivers/virtio/virtio_mem.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c > index 6bbd1cfd10d3..821143db14fe 100644 > --- a/drivers/virtio/virtio_mem.c > +++ b/drivers/virtio/virtio_mem.c > @@ -500,12 +500,13 @@ static bool virtio_mem_overlaps_range(struct virtio_mem > *vm, > } > > /* > - * Test if a virtio-mem device owns a memory block. Can be called from > + * Test if a virtio-mem device contains a given range. Can be called from > * (notifier) callbacks lockless. > */ > -static bool virtio_mem_owned_mb(struct virtio_mem *vm, unsigned long mb_id) > +static bool virtio_mem_contains_range(struct virtio_mem *vm, uint64_t start, > + uint64_t size) > { > - return mb_id >= vm->first_mb_id && mb_id <= vm->last_mb_id; > + return start >= vm->addr && start + size <= vm->addr + > vm->region_size; > } > > static int virtio_mem_notify_going_online(struct virtio_mem *vm, > @@ -800,7 +801,7 @@ static void virtio_mem_online_page_cb(struct page *page, > unsigned int order) > */ > rcu_read_lock(); > list_for_each_entry_rcu(vm, _mem_devices, next) { > - if (!virtio_mem_owned_mb(vm, mb_id)) > + if (!virtio_mem_contains_range(vm, addr, PFN_PHYS(1 << > order))) > continue; > > sb_id = virtio_mem_phys_to_sb_id(vm, addr); Looks good. Reviewed-by: Pankaj Gupta
Re: [PATCH v1 08/29] virtio-mem: drop last_mb_id
> No longer used, let's drop it. > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Pankaj Gupta > Signed-off-by: David Hildenbrand > --- > drivers/virtio/virtio_mem.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c > index 37a0e338ae4a..5c93f8a65eba 100644 > --- a/drivers/virtio/virtio_mem.c > +++ b/drivers/virtio/virtio_mem.c > @@ -84,8 +84,6 @@ struct virtio_mem { > > /* Id of the first memory block of this device. */ > unsigned long first_mb_id; > - /* Id of the last memory block of this device. */ > - unsigned long last_mb_id; > /* Id of the last usable memory block of this device. */ > unsigned long last_usable_mb_id; > /* Id of the next memory bock to prepare when needed. */ > @@ -1689,8 +1687,6 @@ static int virtio_mem_init(struct virtio_mem *vm) > vm->first_mb_id = virtio_mem_phys_to_mb_id(vm->addr - 1 + >memory_block_size_bytes()); > vm->next_mb_id = vm->first_mb_id; > - vm->last_mb_id = virtio_mem_phys_to_mb_id(vm->addr + > -vm->region_size) - 1; > > dev_info(>vdev->dev, "start address: 0x%llx", vm->addr); > dev_info(>vdev->dev, "region size: 0x%llx", vm->region_size); Reviewed-by: Pankaj Gupta
Re: For review: seccomp_user_notif(2) manual page
On Thu, Oct 15, 2020 at 1:24 PM Michael Kerrisk (man-pages) wrote: > On 9/30/20 5:53 PM, Jann Horn wrote: > > On Wed, Sep 30, 2020 at 1:07 PM Michael Kerrisk (man-pages) > > wrote: > >> I knew it would be a big ask, but below is kind of the manual page > >> I was hoping you might write [1] for the seccomp user-space notification > >> mechanism. Since you didn't (and because 5.9 adds various new pieces > >> such as SECCOMP_ADDFD_FLAG_SETFD and SECCOMP_IOCTL_NOTIF_ADDFD > >> that also will need documenting [2]), I did :-). But of course I may > >> have made mistakes... [...] > >>3. The supervisor process will receive notification events on the > >> listening file descriptor. These events are returned as > >> structures of type seccomp_notif. Because this structure and > >> its size may evolve over kernel versions, the supervisor must > >> first determine the size of this structure using the sec‐ > >> comp(2) SECCOMP_GET_NOTIF_SIZES operation, which returns a > >> structure of type seccomp_notif_sizes. The supervisor allo‐ > >> cates a buffer of size seccomp_notif_sizes.seccomp_notif bytes > >> to receive notification events. In addition,the supervisor > >> allocates another buffer of size seccomp_notif_sizes.sec‐ > >> comp_notif_resp bytes for the response (a struct sec‐ > >> comp_notif_resp structure) that it will provide to the kernel > >> (and thus the target process). > >> > >>4. The target process then performs its workload, which includes > >> system calls that will be controlled by the seccomp filter. > >> Whenever one of these system calls causes the filter to return > >> the SECCOMP_RET_USER_NOTIF action value, the kernel does not > >> execute the system call; instead, execution of the target > >> process is temporarily blocked inside the kernel and a notifi‐ > > > > where "blocked" refers to the interruptible, restartable kind - if the > > child receives a signal with an SA_RESTART signal handler in the > > meantime, it'll leave the syscall, go through the signal handler, then > > restart the syscall again and send the same request to the supervisor > > again. so the supervisor may see duplicate syscalls. > > So, I partially demonstrated what you describe here, for two example > system calls (epoll_wait() and pause()). But I could not exactly > demonstrate things as I understand you to be describing them. (So, > I'm not sure whether I have not understood you correctly, or > if things are not exactly as you describe them.) > > Here's a scenario (A) that I tested: > > 1. Target installs seccomp filters for a blocking syscall >(epoll_wait() or pause(), both of which should never restart, >regardless of SA_RESTART) > 2. Target installs SIGINT handler with SA_RESTART > 3. Supervisor is sleeping (i.e., is not blocked in >SECCOMP_IOCTL_NOTIF_RECV operation). > 4. Target makes a blocking system call (epoll_wait() or pause()). > 5. SIGINT gets delivered to target; handler gets called; >***and syscall gets restarted by the kernel*** > > That last should never happen, of course, and is a result of the > combination of both the user-notify filter and the SA_RESTART flag. > If one or other is not present, then the system call is not > restarted. > > So, as you note below, the UAPI gets broken a little. > > However, from your description above I had understood that > something like the following scenario (B) could occur: > > 1. Target installs seccomp filters for a blocking syscall >(epoll_wait() or pause(), both of which should never restart, >regardless of SA_RESTART) > 2. Target installs SIGINT handler with SA_RESTART > 3. Supervisor performs SECCOMP_IOCTL_NOTIF_RECV operation (which >blocks). > 4. Target makes a blocking system call (epoll_wait() or pause()). > 5. Supervisor gets seccomp user-space notification (i.e., >SECCOMP_IOCTL_NOTIF_RECV ioctl() returns > 6. SIGINT gets delivered to target; handler gets called; >and syscall gets restarted by the kernel > 7. Supervisor performs another SECCOMP_IOCTL_NOTIF_RECV operation >which gets another notification for the restarted system call. > > However, I don't observe such behavior. In step 6, the syscall > does not get restarted by the kernel, but instead returns -1/EINTR. > Perhaps I have misconstructed my experiment in the second case, or > perhaps I've misunderstood what you meant, or is it possibly the > case that things are not quite as you said? user@vm:~/test/seccomp-notify-interrupt$ cat seccomp-notify-interrupt.c #define _GNU_SOURCE #include #include #include #include #include #include #include #include #include #include #include #include #include #include #include struct { int seccomp_fd; } *shared; static void handle_signal(int sig, siginfo_t *info,
Re: [PATCH v6 22/25] x86/asm: annotate indirect jumps
On Thu, Oct 15, 2020 at 12:22:16PM +0200, Peter Zijlstra wrote: > On Thu, Oct 15, 2020 at 01:23:41AM +0200, Jann Horn wrote: > > > It would probably be good to keep LTO and non-LTO builds in sync about > > which files are subjected to objtool checks. So either you should be > > removing the OBJECT_FILES_NON_STANDARD annotations for anything that > > is linked into the main kernel (which would be a nice cleanup, if that > > is possible), > > This, I've had to do that for a number of files already for the limited > vmlinux.o passes we needed for noinstr validation. Getting rid of OBJECT_FILES_NON_STANDARD is indeed the end goal, though I'm not sure how practical that will be for some of the weirder edge case. On a related note, I have some old crypto cleanups which need dusting off. -- Josh
Re: [PATCH v4 0/5] Speed up mremap on large regions
On Wed, 14 Oct 2020 00:53:05 +, Kalesh Singh wrote: > This is a repost of the mremap speed up patches, adding Kirill's > Acked-by's (from a separate discussion). The previous versions are > posted at: > v1 - https://lore.kernel.org/r/20200930222130.4175584-1-kaleshsi...@google.com > v2 - https://lore.kernel.org/r/20201002162101.665549-1-kaleshsi...@google.com > v3 - http://lore.kernel.org/r/20201005154017.474722-1-kaleshsi...@google.com > > [...] Applied just the arm64 PMD patch to arm64 (for-next/core), thanks! [1/1] arm64: mremap speedup - Enable HAVE_MOVE_PMD https://git.kernel.org/arm64/c/45544eee9606 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev
Re: [PATCH v7] coccinelle: api: add kfree_mismatch script
[See below for a comment] On Mon, 3 Aug 2020, Denis Efremov wrote: > Check that alloc and free types of functions match each other. > > Signed-off-by: Denis Efremov > --- > Changes in v2: > - Lines are limited to 80 characters where possible > - Confidence changed from High to Medium because of >fs/btrfs/send.c:1119 false-positive > - __vmalloc_area_node() explicitly excluded from analysis >instead of !(file in "mm/vmalloc.c") condition > Changes in v3: > - prints style in org && report modes changed for python2 > Changes in v4: > - missing msg argument to print_todo fixed > Changes in v5: > - fix position p in kfree rule > - move @kok and @v positions in choice rule after the arguments > - remove kvmalloc suggestions > Changes in v6: > - more asterisks added in context mode > - second @kok added to the choice rule > Changes in v7: > - file renamed to kfree_mismatch.cocci > - python function relevant() removed > - additional rule for filtering free positions added > - btrfs false-positive fixed > - confidence level changed to high > - kvfree_switch rule added > - names for position variables changed to @a (alloc) and @f (free) > > scripts/coccinelle/api/kfree_mismatch.cocci | 229 > 1 file changed, 229 insertions(+) > create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci > > diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci > b/scripts/coccinelle/api/kfree_mismatch.cocci > new file mode 100644 > index ..9e9ef9fd7a25 > --- /dev/null > +++ b/scripts/coccinelle/api/kfree_mismatch.cocci > @@ -0,0 +1,229 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Check that kvmalloc'ed memory is freed by kfree functions, > +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree > +/// functions. > +/// > +// Confidence: High > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > + > +virtual patch > +virtual report > +virtual org > +virtual context > + > +@alloc@ > +expression E, E1; > +position kok, vok; > +@@ > + > +( > + if (...) { > +... > +E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| > + kmalloc_node\|kzalloc_node\|kmalloc_array\| > + kmalloc_array_node\|kcalloc_node\)(...)@kok > +... > + } else { > +... > +E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > +... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > +kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... when != E = E1 > + when any > + if (E == NULL) { > +... > +E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > +... > + } > +) > + > +@free@ > +expression E; > +position fok; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > +kvmalloc_array\)(...) > + ... > + kvfree(E)@fok > + > +@vfree depends on !patch@ > +expression E; > +position a != alloc.kok; > +position f != free.fok; > +@@ > + > +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > +* kcalloc_node\)(...)@a > + ... when != if (...) { ... E = > \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); > ... } > + when != is_vmalloc_addr(E) > + when any > +* \(vfree\|vfree_atomic\|kvfree\)(E)@f > + > +@depends on patch exists@ > +expression E; > +position a != alloc.kok; > +position f != free.fok; > +@@ > + > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > +kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > +kcalloc_node\)(...)@a > + ... when != if (...) { ... E = > \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); > ... } > + when != is_vmalloc_addr(E) > + when any > +- \(vfree\|vfree_atomic\|kvfree\)(E)@f > ++ kfree(E) > + > +@kfree depends on !patch@ > +expression E; > +position a != alloc.vok; > +position f != free.fok; > +@@ > + > +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > +* __vmalloc_node_range\|__vmalloc_node\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|kvfree\)(E)@f > + > +@depends on patch exists@ > +expression E; > +position a != alloc.vok; > +position f != free.fok; > +@@ > + > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > +
[ANNOUNCE] Git v2.29.0-rc2
A release candidate Git v2.29.0-rc2 is now available for testing at the usual places. The tree has no change since v2.29.0-rc1 The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The following public repositories all have a copy of the 'v2.29.0-rc2' tag and the 'master' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://github.com/gitster/git Thanks. Git 2.29 Release Notes (draft) == Updates since v2.28 --- UI, Workflows & Features * "git help log" has been enhanced by sharing more material from the documentation for the underlying "git rev-list" command. * "git for-each-ref --format=<>" learned %(contents:size). * "git merge" learned to selectively omit " into " at the end of the title of default merge message with merge.suppressDest configuration. * The component to respond to "git fetch" request is made more configurable to selectively allow or reject object filtering specification used for partial cloning. * Stop when "sendmail.*" configuration variables are defined, which could be a mistaken attempt to define "sendemail.*" variables. * The existing backends for "git mergetool" based on variants of vim have been refactored and then support for "nvim" has been added. * "git bisect" learns the "--first-parent" option to find the first breakage along the first-parent chain. * "git log --first-parent -p" showed patches only for single-parent commits on the first-parent chain; the "--first-parent" option has been made to imply "-m". Use "--no-diff-merges" to restore the previous behaviour to omit patches for merge commits. * The commit labels used to explain each side of conflicted hunks placed by the sequencer machinery have been made more readable by humans. * The "--batch-size" option of "git multi-pack-index repack" command is now used to specify that very small packfiles are collected into one until the total size roughly exceeds it. * The recent addition of SHA-256 support is marked as experimental in the documentation. * "git fetch" learned --no-write-fetch-head option to avoid writing the FETCH_HEAD file. * Command line completion (in contrib/) usually omits redundant, deprecated and/or dangerous options from its output; it learned to optionally include all of them. * The output from the "diff" family of the commands had abbreviated object names of blobs involved in the patch, but its length was not affected by the --abbrev option. Now it is. * "git worktree" gained a "repair" subcommand to help users recover after moving the worktrees or repository manually without telling Git. Also, "git init --separate-git-dir" no longer corrupts administrative data related to linked worktrees. * The "--format=" option to the "for-each-ref" command and friends learned a few more tricks, e.g. the ":short" suffix that applies to "objectname" now also can be used for "parent", "tree", etc. * "git worktree add" learns that the "-d" is a synonym to "--detach" option to create a new worktree without being on a branch. * "format-patch --range-diff= ..HEAD" has been taught not to ignore when is a single version. * "add -p" now allows editing paths that were only added in intent. * The 'meld' backend of the "git mergetool" learned to give the underlying 'meld' the '--auto-merge' option, which would help reduce the amount of text that requires manual merging. * "git for-each-ref" and friends that list refs used to allow only one --merged or --no-merged to filter them; they learned to take combination of both kind of filtering. * "git maintenance", a "git gc"'s big brother, has been introduced to take care of more repository maintenance tasks, not limited to the object database cleaning. * "git receive-pack" that accepts requests by "git push" learned to outsource most of the ref updates to the new "proc-receive" hook. * "git push" that wants to be atomic and wants to send push certificate learned not to prepare and sign the push certificate when it fails the local check (hence due to atomicity it is known that no certificate is needed). * "git commit-graph write" learned to limit the number of bloom filters that are computed from scratch with the --max-new-filters option. * The transport protocol v2 has become the default again. * The installation procedure learned to optionally omit "git-foo" executable files for each 'foo' built-in subcommand, which are only required by old timers that still rely on the age old promise that prepending "git --exec-path" output to PATH early in their script will keep the "git-foo" calls they wrote working. * The command line completion (in contrib/) learned that
Re: [PATCH 0/3] IRQ stack support for ARM
On Thu, Oct 8, 2020 at 1:30 AM Russell King - ARM Linux admin wrote: > > On Thu, Oct 08, 2020 at 12:45:30PM +0530, Maninder Singh wrote: > > Observed Stack Overflow on 8KB kernel stack on ARM specially > > incase on network interrupts, which results in undeterministic behaviour. > > So there is need for per cpu dedicated IRQ stack for ARM. > > > > As ARm does not have extra co-processor register > > to save thread info pointer, IRQ stack will be at some > > performance cost, so code is under CONFIG_IRQ_STACK. > > > > and we don't have much knowledge and set up for CLANG > > and ARM_UNWIND, so dependency added for both cases. > > > > Tested patch set with QEMU for latest kernel > > and 4.1 kernel for ARM target with same patch set. > > You need to investigate and show where and why this is happening. My > guess is you have a network driver that uses a lot of kernel stack > space, which itself would be a bug. > > Note that there are compiler versions out there that mis-optimise and > eat stack space - the kernel build should be warning if a function > uses a large amount of stack. For tracking down those not-super-helpful compiler warnings, I wrote a tool where if you rebuild with debug info, and give it the object file and string of the function the compiler warned about it will parse the DWARF to tell you the size of each local variable, and if it came from an inline frame. Generally, it's possible to stack allocate something that's way too big; instead those should be allocated on the heap. https://github.com/ClangBuiltLinux/frame-larger-than (I haven't had time to sit down and use it to resolve all outstanding issues, but it has worked well for me in the past) -- Thanks, ~Nick Desaulniers
Re: Re: [PATCH v2] arm64: dts: allwinner: h6: add eMMC voltage property for Beelink GS1
Dne četrtek, 15. oktober 2020 ob 11:35:44 CEST je Maxime Ripard napisal(a): > On Tue, Oct 13, 2020 at 11:27:33PM +0200, Jernej Škrabec wrote: > > Dne petek, 09. oktober 2020 ob 09:36:51 CEST je Maxime Ripard napisal(a): > > > On Thu, Oct 08, 2020 at 10:00:06PM +0200, Clément Péron wrote: > > > > Hi Maxime, > > > > > > > > Adding linux-sunxi and Jernej Skrabec to this discussion. > > > > > > > > On Thu, 8 Oct 2020 at 17:10, Maxime Ripard wrote: > > > > > > > > > > Hi Clément, > > > > > > > > > > On Mon, Oct 05, 2020 at 08:47:19PM +0200, Clément Péron wrote: > > > > > > On Mon, 5 Oct 2020 at 11:21, Maxime Ripard wrote: > > > > > > > > > > > > > > Hi Clément, > > > > > > > > > > > > > > On Sat, Oct 03, 2020 at 11:20:01AM +0200, Clément Péron wrote: > > > > > > > > Sunxi MMC driver can't distinguish at runtime what's the I/O > > voltage > > > > > > > > for HS200 mode. > > > > > > > > > > > > > > Unfortunately, that's not true (or at least, that's not related to > > your patch). > > > > > > > > > > > > > > > Add a property in the device-tree to notify MMC core about this > > > > > > > > configuration. > > > > > > > > > > > > > > > > Fixes: 089bee8dd119 ("arm64: dts: allwinner: h6: Introduce Beelink > > GS1 board") > > > > > > > > Signed-off-by: Clément Péron > > > > > > > > --- > > > > > > > > arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + > > > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink- > > gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts > > > > > > > > index 049c21718846..3f20d2c9 100644 > > > > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts > > > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts > > > > > > > > @@ -145,6 +145,7 @@ { > > > > > > > > vqmmc-supply = <_bldo2>; > > > > > > > > non-removable; > > > > > > > > cap-mmc-hw-reset; > > > > > > > > + mmc-hs200-1_8v; > > > > > > > > bus-width = <8>; > > > > > > > > status = "okay"; > > > > > > > > }; > > > > > > > > > > > > > > I'm not really sure what you're trying to fix here, but as far as MMC > > > > > > > goes, eMMC's can support io voltage of 3.3, 1.8 and 1.2V. Modes up > > until > > > > > > > HS DDR (50MHz in DDR) will use an IO voltage of 3.3V, higher speed > > modes > > > > > > > (HS200 and HS400) supporting 1.8V and 1.2V. > > > > > > > > > > > > Some users report that the eMMC is not working properly on their > > > > > > Beelink GS1 boards. > > > > > > > > > > > > > The mmc-hs200-1_8v property states that the MMC controller supports > > the > > > > > > > HS200 mode at 1.8V. Now, I can only assume that since BLDO2 is set > > up at > > > > > > > 1.8V then otherwise, the MMC core will rightfully decide to use the > > > > > > > highest supported mode. In this case, since the driver sets it, it > > would > > > > > > > be HS-DDR at 3.3V, which won't work with that fixed regulator. > > > > > > > > > > > > > > I can only assume that enabling HS200 at 1.8V only fixes the issue > > you > > > > > > > have because otherwise it would use HS-DDR at 3.3V, ie not actually > > > > > > > fixing the issue but sweeping it under the rug. > > > > > > > > > > > > > > Trying to add mmc-ddr-1_8v would be a good idea > > > > > > > > > > > > Thanks for the explanation, this is indeed the correct one. > > > > > > So It looks like the SDIO controller has an issue on some boards when > > > > > > using HS-DDR mode. > > > > > > > > > > > > Is this patch acceptable with the proper commit log? > > > > > > > > > > If HS-DDR works, yes, but I assume it doesn't? > > > > > > > > After discussing with Jernej about this issue, I understood that: > > > > - Automatic delay calibration is not implemented > > > > - We also miss some handling of DDR related bits in control register > > > > > > > > So none of H5/H6 boards should actually work. > > > > (Some 'lucky' boards seem to work enough to switch to HS200 mode...) > > > > > > > > To "fix" this the H5 disable the HS-DDR mode in sunxi mmc driver : > > > > https://github.com/torvalds/linux/blob/master/drivers/mmc/host/sunxi-mmc.c#L1409 > > > > > > I find it suspicious that some boards would have traces not good enough > > > for HS-DDR (50MHz in DDR) but would work fine in HS200 (200MHz in SDR). > > > If there's some mismatch on the traces, it will only be worse in HS200. > > > > FYI, similar situation is also with Tanix TX6 board. Mine works well in HS-DDR > > mode, but some people reported that it doesn't work for them. The only > > possible difference could be different eMMC IC. I'll try to confirm that. > > > > Anyway, I did some tests on OrangePi 3 board which also have eMMC. Both modes > > (HS-DDR and HS200) are supported and work well. > > The Orange Pi 3 has an HS400-capable eMMC ?! > > That's really good news, I've initially done the HS200 support on a > custom board I had to give
Did you get my mail?
-- Hello. I am still waiting to hear from you regarding my proposal. Thanks
RE: [PATCH] compiler.h: Clarify comment about the need for barrier_data()
From: Arvind Sankar > Sent: 15 October 2020 19:14 > > Be clear about @ptr vs the variable that @ptr points to, and add some > more details as to why the special barrier_data() macro is required. > > Signed-off-by: Arvind Sankar > --- > include/linux/compiler.h | 33 ++--- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 93035d7fee0d..d8cee7c8968d 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -86,17 +86,28 @@ void ftrace_likely_update(struct ftrace_likely_data *f, > int val, > > #ifndef barrier_data > /* > - * This version is i.e. to prevent dead stores elimination on @ptr > - * where gcc and llvm may behave differently when otherwise using > - * normal barrier(): while gcc behavior gets along with a normal > - * barrier(), llvm needs an explicit input variable to be assumed > - * clobbered. The issue is as follows: while the inline asm might > - * access any memory it wants, the compiler could have fit all of > - * @ptr into memory registers instead, and since @ptr never escaped > - * from that, it proved that the inline asm wasn't touching any of > - * it. This version works well with both compilers, i.e. we're telling > - * the compiler that the inline asm absolutely may see the contents > - * of @ptr. See also: https://llvm.org/bugs/show_bug.cgi?id=15495 > + * This version is to prevent dead stores elimination on @ptr where gcc and > + * llvm may behave differently when otherwise using normal barrier(): while > gcc > + * behavior gets along with a normal barrier(), llvm needs an explicit input > + * variable to be assumed clobbered. > + * > + * Its primary use is in implementing memzero_explicit(), which is used for > + * clearing temporary data that may contain secrets. > + * > + * The issue is as follows: while the inline asm might access any memory it > + * wants, the compiler could have fit all of the variable that @ptr points to > + * into registers instead, and if @ptr never escaped from the function, it > + * proved that the inline asm wasn't touching any of it. gcc only eliminates > + * dead stores if the variable was actually allocated in registers, but llvm > + * reasons that the variable _could_ have been in registers, so the inline > asm > + * can't reliably access it anyway, and eliminates dead stores even if the > + * variable is actually in memory. I think I'd just say something like: Although the compiler must assume a "memory" clobber may affect all memory, local variables (on stack) cannot actually be visible to the asm unless their address has been passed to an external function. So the compiler may assume such variables cannot be affected by a normal asm volatile(::"memory") barrier(). Passing the address of the local variables to the asm barrier is enough to tell the compiler that the asm can 'see' the variables (and spill anything held in registers to the stack) so that the "memory" clobber has the expected effect. This is necessary to get llvm to do a memset() of on-stack data at the end of a function to clear memory that contains secrets. David > + * > + * This version works well with both compilers, i.e. we're telling the > compiler > + * that the inline asm absolutely may see the contents of the variable > pointed > + * to by @ptr. > + * > + * See also: https://llvm.org/bugs/show_bug.cgi?id=15495#c5 > */ > # define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory") > #endif > -- > 2.26.2 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: > On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > > Fix the space in the middle in below entry. > > > > > > memry||memory > > [] > > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > > [] > > > @@ -885,7 +885,7 @@ meetign||meeting > > > memeory||memory > > > memmber||member > > > memoery||memory > > > -memry ||memory > > > +memry||memory > > > > No. Don't post a bad patch, assume > > it's applied and then post a fix to > > the bad patch as v2. > > > > Send a single clean patch. > > > > Not sure what you mean...could you elaborate...don't know what is going on..> You sent a patch with a defect You sent a V2 patch that just corrects the defect in the first patch. Instead send a v3 patch without any defect.
Re: linux-next: build failure after merge of the hmm tree
Hi all, On Mon, 12 Oct 2020 15:19:48 +1100 Stephen Rothwell wrote: > > On Tue, 6 Oct 2020 13:41:20 -0300 Jason Gunthorpe wrote: > > > > On Tue, Oct 06, 2020 at 08:35:08PM +1100, Stephen Rothwell wrote: > > > Hi all, > > > > > > After merging the hmm tree, today's linux-next build (arm > > > multi_v7_defconfig) failed like this: > > > > > > > > > Caused by commit > > > > > > 07da1223ec93 ("lib/scatterlist: Add support in dynamic allocation of SG > > > table from pages") > > > > > > interacting with commit > > > > > > 707d561f77b5 ("drm: allow limiting the scatter list size.") > > > > > > from the drm tree. > > > > > > I have added the following merge fix patch > > > > > > From: Stephen Rothwell > > > Date: Tue, 6 Oct 2020 20:22:51 +1100 > > > Subject: [PATCH] lib/scatterlist: merge fix for "drm: allow limiting the > > > scatter list size." > > > > > > Signed-off-by: Stephen Rothwell > > > drivers/gpu/drm/drm_prime.c | 9 ++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > > index 11fe9ff76fd5..83ac901b65a2 100644 > > > +++ b/drivers/gpu/drm/drm_prime.c > > > @@ -807,6 +807,7 @@ struct sg_table *drm_prime_pages_to_sg(struct > > > drm_device *dev, > > > struct page **pages, unsigned int > > > nr_pages) > > > { > > > struct sg_table *sg = NULL; > > > + struct scatterlist *sl; > > > size_t max_segment = 0; > > > int ret; > > > > > > @@ -820,11 +821,13 @@ struct sg_table *drm_prime_pages_to_sg(struct > > > drm_device *dev, > > > max_segment = dma_max_mapping_size(dev->dev); > > > if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT) > > > max_segment = SCATTERLIST_MAX_SEGMENT; > > > - ret = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, > > > + sl = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, > > > nr_pages << PAGE_SHIFT, > > > - max_segment, GFP_KERNEL); > > > - if (ret) > > > + max_segment, NULL, 0, GFP_KERNEL); > > > + if (IS_ERR(sl)) { > > > + ret = PTR_ERR(sl); > > > goto out; > > > + } > > > > > > return sg; > > > out: > > > > This looks OK to me, thanks > > This merge fix patch is now being applied to the merge of the drm tree > since the rdma tree (that is merged before the drm tree) has merged the > hmm tree. This merge fix patch is now being applied to the merge of the rdma tree since the Linus has merged the drm tree. -- Cheers, Stephen Rothwell pgpLAtQQAbSSV.pgp Description: OpenPGP digital signature
[v4.9..v5.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4
From: Luiz Augusto von Dentz E0 is not allowed with Level 4: BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: '128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened' SC enabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) Secure Connections (Host Support) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with AES-CCM (0x02) SC disabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used < HCI Command: Disconnect (0x01|0x0006) plen 3 Handle: 256 Reason: Authentication Failure (0x05) Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5) Cc: sta...@vger.kernel.org # 4.9..5.4 --- include/net/bluetooth/hci_core.h | 10 ++ net/bluetooth/hci_conn.c | 17 + net/bluetooth/hci_event.c| 20 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 144f580556f9..3cd232cf29c6 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1329,11 +1329,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) else encrypt = 0x01; - if (conn->sec_level == BT_SECURITY_SDP) - conn->sec_level = BT_SECURITY_LOW; + if (!status) { + if (conn->sec_level == BT_SECURITY_SDP) + conn->sec_level = BT_SECURITY_LOW; - if (conn->pending_sec_level > conn->sec_level) - conn->sec_level = conn->pending_sec_level; + if (conn->pending_sec_level > conn->sec_level) + conn->sec_level = conn->pending_sec_level; + } mutex_lock(_cb_list_lock); list_for_each_entry(cb, _cb_list, list) { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 87691404d0c6..ee57fa20bac3 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1285,6 +1285,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } +/* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, >flags)) { + bt_dev_err(conn->hdev, + "Invalid security: Missing AES-CCM usage"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, >flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index de4cce5f1bd8..9917b399ddd0 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2974,27 +2974,23 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_CONN_ENCRYPT_PEND, >flags); + /* Check link security requirements are met */ + if (!hci_conn_check_link_mode(conn)) + ev->status = HCI_ERROR_AUTH_FAILURE; + if (ev->status && conn->state == BT_CONNECTED) { if (ev->status == HCI_ERROR_PIN_OR_KEY_MISSING) set_bit(HCI_CONN_AUTH_FAILURE, >flags); + /* Notify upper layers so they can cleanup before +* disconnecting. +*/ + hci_encrypt_cfm(conn, ev->status); hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; } - /* In Secure Connections Only mode, do not allow any connections -* that are not encrypted with AES-CCM using a P-256 authenticated -* combination key. -*/ - if
[v5.8/bluetooth PATCH] Bluetooth: Disconnect if E0 is used for Level 4
From: Luiz Augusto von Dentz E0 is not allowed with Level 4: BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: '128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened' SC enabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) Secure Connections (Host Support) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with AES-CCM (0x02) SC disabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used < HCI Command: Disconnect (0x01|0x0006) plen 3 Handle: 256 Reason: Authentication Failure (0x05) Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5) Cc: sta...@vger.kernel.org # 5.8 --- include/net/bluetooth/hci_core.h | 10 ++ net/bluetooth/hci_conn.c | 17 + net/bluetooth/hci_event.c| 20 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index da3728871e85..78970afa96f9 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1402,11 +1402,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) else encrypt = 0x01; - if (conn->sec_level == BT_SECURITY_SDP) - conn->sec_level = BT_SECURITY_LOW; + if (!status) { + if (conn->sec_level == BT_SECURITY_SDP) + conn->sec_level = BT_SECURITY_LOW; - if (conn->pending_sec_level > conn->sec_level) - conn->sec_level = conn->pending_sec_level; + if (conn->pending_sec_level > conn->sec_level) + conn->sec_level = conn->pending_sec_level; + } mutex_lock(_cb_list_lock); list_for_each_entry(cb, _cb_list, list) { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 307800fd18e6..b99b5c6de55a 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1323,6 +1323,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } +/* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, >flags)) { + bt_dev_err(conn->hdev, + "Invalid security: Missing AES-CCM usage"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, >flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 6c6c9a81bee2..ff38f98988db 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3068,27 +3068,23 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_CONN_ENCRYPT_PEND, >flags); + /* Check link security requirements are met */ + if (!hci_conn_check_link_mode(conn)) + ev->status = HCI_ERROR_AUTH_FAILURE; + if (ev->status && conn->state == BT_CONNECTED) { if (ev->status == HCI_ERROR_PIN_OR_KEY_MISSING) set_bit(HCI_CONN_AUTH_FAILURE, >flags); + /* Notify upper layers so they can cleanup before +* disconnecting. +*/ + hci_encrypt_cfm(conn, ev->status); hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; } - /* In Secure Connections Only mode, do not allow any connections -* that are not encrypted with AES-CCM using a P-256 authenticated -* combination key. -*/ - if (hci_dev_test_flag(hdev,
[v4.9..v5.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm`
From: Patrick Steinhardt Starting with the upgrade to v5.8-rc3, I've noticed I wasn't able to connect to my Bluetooth headset properly anymore. While connecting to the device would eventually succeed, bluetoothd seemed to be confused about the current connection state where the state was flapping hence and forth. Bisecting this issue led to commit 3ca44c16b0dc (Bluetooth: Consolidate encryption handling in hci_encrypt_cfm, 2020-05-19), which refactored `hci_encrypt_cfm` to also handle updating the connection state. The commit in question changed the code to call `hci_connect_cfm` inside `hci_encrypt_cfm` and to change the connection state. But with the conversion, we now only update the connection state if a status was set already. In fact, the reverse should be true: the status should be updated if no status is yet set. So let's fix the isuse by reversing the condition. Fixes: 3ca44c16b0dc ("Bluetooth: Consolidate encryption handling in hci_encrypt_cfm") Signed-off-by: Patrick Steinhardt Acked-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 339ddaa626995bc6218972ca241471f3717cc5f4) Cc: sta...@vger.kernel.org # 4.9..5.4 --- include/net/bluetooth/hci_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index ffd0eedd27ab..144f580556f9 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1314,7 +1314,7 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) __u8 encrypt; if (conn->state == BT_CONFIG) { - if (status) + if (!status) conn->state = BT_CONNECTED; hci_connect_cfm(conn, status); -- 2.27.0
[v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm`
From: Patrick Steinhardt Starting with the upgrade to v5.8-rc3, I've noticed I wasn't able to connect to my Bluetooth headset properly anymore. While connecting to the device would eventually succeed, bluetoothd seemed to be confused about the current connection state where the state was flapping hence and forth. Bisecting this issue led to commit 3ca44c16b0dc (Bluetooth: Consolidate encryption handling in hci_encrypt_cfm, 2020-05-19), which refactored `hci_encrypt_cfm` to also handle updating the connection state. The commit in question changed the code to call `hci_connect_cfm` inside `hci_encrypt_cfm` and to change the connection state. But with the conversion, we now only update the connection state if a status was set already. In fact, the reverse should be true: the status should be updated if no status is yet set. So let's fix the isuse by reversing the condition. Fixes: 3ca44c16b0dc ("Bluetooth: Consolidate encryption handling in hci_encrypt_cfm") Signed-off-by: Patrick Steinhardt Acked-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 339ddaa626995bc6218972ca241471f3717cc5f4) Cc: sta...@vger.kernel.org # 4.4 --- include/net/bluetooth/hci_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 0269a772bfe1..dfa672b6f89d 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1241,7 +1241,7 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) __u8 encrypt; if (conn->state == BT_CONFIG) { - if (status) + if (!status) conn->state = BT_CONNECTED; hci_connect_cfm(conn, status); -- 2.27.0
[v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
From: Luiz Augusto von Dentz This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection state is BT_CONFIG so callers don't have to check the state. Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3) Cc: sta...@vger.kernel.org # 4.4 --- include/net/bluetooth/hci_core.h | 20 ++-- net/bluetooth/hci_event.c| 28 +++- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 7c0c83dfe86e..0269a772bfe1 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1235,10 +1235,26 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) conn->security_cfm_cb(conn, status); } -static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status, - __u8 encrypt) +static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) { struct hci_cb *cb; + __u8 encrypt; + + if (conn->state == BT_CONFIG) { + if (status) + conn->state = BT_CONNECTED; + + hci_connect_cfm(conn, status); + hci_conn_drop(conn); + return; + } + + if (!test_bit(HCI_CONN_ENCRYPT, >flags)) + encrypt = 0x00; + else if (test_bit(HCI_CONN_AES_CCM, >flags)) + encrypt = 0x02; + else + encrypt = 0x01; if (conn->sec_level == BT_SECURITY_SDP) conn->sec_level = BT_SECURITY_LOW; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 03319ab8a7c6..bb9c13506bca 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2479,7 +2479,7 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) ); } else { clear_bit(HCI_CONN_ENCRYPT_PEND, >flags); - hci_encrypt_cfm(conn, ev->status, 0x00); + hci_encrypt_cfm(conn, ev->status); } } @@ -2565,22 +2565,7 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status, conn->enc_key_size = rp->key_size; } - if (conn->state == BT_CONFIG) { - conn->state = BT_CONNECTED; - hci_connect_cfm(conn, 0); - hci_conn_drop(conn); - } else { - u8 encrypt; - - if (!test_bit(HCI_CONN_ENCRYPT, >flags)) - encrypt = 0x00; - else if (test_bit(HCI_CONN_AES_CCM, >flags)) - encrypt = 0x02; - else - encrypt = 0x01; - - hci_encrypt_cfm(conn, 0, encrypt); - } + hci_encrypt_cfm(conn, 0); unlock: hci_dev_unlock(hdev); @@ -2674,14 +2659,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) } notify: - if (conn->state == BT_CONFIG) { - if (!ev->status) - conn->state = BT_CONNECTED; - - hci_connect_cfm(conn, ev->status); - hci_conn_drop(conn); - } else - hci_encrypt_cfm(conn, ev->status, ev->encrypt); + hci_encrypt_cfm(conn, ev->status); unlock: hci_dev_unlock(hdev); -- 2.27.0
[GIT PULL] Kunit fixes update for Linux 5.10-rc1
Hi Linus, Please pull the following Kunit fixes update for Linux 5.10-rc1 This Kunit fixes update consists of several kunit tool bug fixes in flag handling, run outside kernel tree, make errors, and generating results. diff is attached. thanks, -- Shuah The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5: Linux 5.9-rc1 (2020-08-16 13:04:57 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest tags/linux-kselftest-kunit-fixes-5.10-rc1 for you to fetch changes up to 1abdd39f14b25dd2d69096b624a4f86f158a9feb: kunit: tool: fix display of make errors (2020-10-09 14:04:09 -0600) linux-kselftest-kunit-fixes-5.10-rc1 This Kunit fixes update consists of several kunit tool bug fixes in flag handling, run outside kernel tree, make errors, and generating results. Brendan Higgins (3): kunit: tool: fix running kunit_tool from outside kernel tree kunit: tool: fix --alltests flag kunit: tool: handle when .kunit exists but .kunitconfig does not Daniel Latypov (1): kunit: tool: fix display of make errors Heidi Fahim (1): kunit: tool: allow generating test results in JSON tools/testing/kunit/configs/broken_on_uml.config | 1 + tools/testing/kunit/kunit.py | 58 +++--- tools/testing/kunit/kunit_json.py| 63 tools/testing/kunit/kunit_kernel.py | 27 +- tools/testing/kunit/kunit_tool_test.py | 33 + 5 files changed, 154 insertions(+), 28 deletions(-) create mode 100644 tools/testing/kunit/kunit_json.py diff --git a/tools/testing/kunit/configs/broken_on_uml.config b/tools/testing/kunit/configs/broken_on_uml.config index 239b9f03da2c..a7f0603d33f6 100644 --- a/tools/testing/kunit/configs/broken_on_uml.config +++ b/tools/testing/kunit/configs/broken_on_uml.config @@ -39,3 +39,4 @@ # CONFIG_QCOM_CPR is not set # CONFIG_RESET_BRCMSTB_RESCAL is not set # CONFIG_RESET_INTEL_GW is not set +# CONFIG_ADI_AXI_ADC is not set diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 425ef40067e7..ebf5f5763dee 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -17,6 +17,7 @@ from collections import namedtuple from enum import Enum, auto import kunit_config +import kunit_json import kunit_kernel import kunit_parser @@ -30,9 +31,9 @@ KunitBuildRequest = namedtuple('KunitBuildRequest', KunitExecRequest = namedtuple('KunitExecRequest', ['timeout', 'build_dir', 'alltests']) KunitParseRequest = namedtuple('KunitParseRequest', - ['raw_output', 'input_data']) + ['raw_output', 'input_data', 'build_dir', 'json']) KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', - 'build_dir', 'alltests', + 'build_dir', 'alltests', 'json', 'make_options']) KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0] @@ -113,12 +114,22 @@ def parse_tests(request: KunitParseRequest) -> KunitResult: test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS, [], 'Tests not Parsed.') + if request.raw_output: kunit_parser.raw_output(request.input_data) else: test_result = kunit_parser.parse_run_tests(request.input_data) parse_end = time.time() + if request.json: + json_obj = kunit_json.get_json_result( + test_result=test_result, + def_config='kunit_defconfig', + build_dir=request.build_dir, + json_path=request.json) + if request.json == 'stdout': + print(json_obj) + if test_result.status != kunit_parser.TestStatus.SUCCESS: return KunitResult(KunitStatus.TEST_FAILURE, test_result, parse_end - parse_start) @@ -151,7 +162,9 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree, return exec_result parse_request = KunitParseRequest(request.raw_output, - exec_result.result) + exec_result.result, + request.build_dir, + request.json) parse_result = parse_tests(parse_request) run_end = time.time() @@ -195,7 +208,12 @@ def add_exec_opts(parser): def add_parse_opts(parser): parser.add_argument('--raw_output', help='don\'t format output from kernel', action='store_true') - + parser.add_argument('--json', + nargs='?', + help='Stores test results in a JSON, and either ' + 'prints to stdout or saves to file if a ' + 'filename is specified', + type=str, const='stdout', default=None) def main(argv, linux=None): parser = argparse.ArgumentParser( @@ -237,10 +255,16 @@ def main(argv, linux=None): cli_args = parser.parse_args(argv) + if get_kernel_root_path(): +
[v4.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4
From: Luiz Augusto von Dentz E0 is not allowed with Level 4: BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: '128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened' SC enabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) Secure Connections (Host Support) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with AES-CCM (0x02) SC disabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used < HCI Command: Disconnect (0x01|0x0006) plen 3 Handle: 256 Reason: Authentication Failure (0x05) Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5, adjusted to match linux-4.4.y sources.) Cc: sta...@vger.kernel.org # 4.4 --- include/net/bluetooth/hci_core.h | 10 ++ net/bluetooth/hci_conn.c | 17 + net/bluetooth/hci_event.c| 20 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index dfa672b6f89d..5aaf6cdb121a 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1256,11 +1256,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) else encrypt = 0x01; - if (conn->sec_level == BT_SECURITY_SDP) - conn->sec_level = BT_SECURITY_LOW; + if (!status) { + if (conn->sec_level == BT_SECURITY_SDP) + conn->sec_level = BT_SECURITY_LOW; - if (conn->pending_sec_level > conn->sec_level) - conn->sec_level = conn->pending_sec_level; + if (conn->pending_sec_level > conn->sec_level) + conn->sec_level = conn->pending_sec_level; + } mutex_lock(_cb_list_lock); list_for_each_entry(cb, _cb_list, list) { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 114bcf6ea916..2c94e3cd3545 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1173,6 +1173,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } +/* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, >flags)) { + bt_dev_err(conn->hdev, + "Invalid security: Missing AES-CCM usage"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, >flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index bb9c13506bca..f0e6cce921d8 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2612,24 +2612,20 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_CONN_ENCRYPT_PEND, >flags); + /* Check link security requirements are met */ + if (!hci_conn_check_link_mode(conn)) + ev->status = HCI_ERROR_AUTH_FAILURE; + if (ev->status && conn->state == BT_CONNECTED) { + /* Notify upper layers so they can cleanup before +* disconnecting. +*/ + hci_encrypt_cfm(conn, ev->status); hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; } - /* In Secure Connections Only mode, do not allow any connections -* that are not encrypted with AES-CCM using a P-256 authenticated -* combination key. -*/ - if (hci_dev_test_flag(hdev, HCI_SC_ONLY) && - (!test_bit(HCI_CONN_AES_CCM, >flags) || -
[v4.9..v5.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
From: Luiz Augusto von Dentz This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection state is BT_CONFIG so callers don't have to check the state. Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3) Cc: sta...@vger.kernel.org # 4.9..5.4 --- include/net/bluetooth/hci_core.h | 20 ++-- net/bluetooth/hci_event.c| 28 +++- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index b689aceb636b..ffd0eedd27ab 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1308,10 +1308,26 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) conn->security_cfm_cb(conn, status); } -static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status, - __u8 encrypt) +static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) { struct hci_cb *cb; + __u8 encrypt; + + if (conn->state == BT_CONFIG) { + if (status) + conn->state = BT_CONNECTED; + + hci_connect_cfm(conn, status); + hci_conn_drop(conn); + return; + } + + if (!test_bit(HCI_CONN_ENCRYPT, >flags)) + encrypt = 0x00; + else if (test_bit(HCI_CONN_AES_CCM, >flags)) + encrypt = 0x02; + else + encrypt = 0x01; if (conn->sec_level == BT_SECURITY_SDP) conn->sec_level = BT_SECURITY_LOW; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index fd436e5d7b54..de4cce5f1bd8 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2840,7 +2840,7 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) ); } else { clear_bit(HCI_CONN_ENCRYPT_PEND, >flags); - hci_encrypt_cfm(conn, ev->status, 0x00); + hci_encrypt_cfm(conn, ev->status); } } @@ -2925,22 +2925,7 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status, conn->enc_key_size = rp->key_size; } - if (conn->state == BT_CONFIG) { - conn->state = BT_CONNECTED; - hci_connect_cfm(conn, 0); - hci_conn_drop(conn); - } else { - u8 encrypt; - - if (!test_bit(HCI_CONN_ENCRYPT, >flags)) - encrypt = 0x00; - else if (test_bit(HCI_CONN_AES_CCM, >flags)) - encrypt = 0x02; - else - encrypt = 0x01; - - hci_encrypt_cfm(conn, 0, encrypt); - } + hci_encrypt_cfm(conn, 0); unlock: hci_dev_unlock(hdev); @@ -3058,14 +3043,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) } notify: - if (conn->state == BT_CONFIG) { - if (!ev->status) - conn->state = BT_CONNECTED; - - hci_connect_cfm(conn, ev->status); - hci_conn_drop(conn); - } else - hci_encrypt_cfm(conn, ev->status, ev->encrypt); + hci_encrypt_cfm(conn, ev->status); unlock: hci_dev_unlock(hdev); -- 2.27.0
Re: [PATCH] power: reset: POWER_RESET_OCELOT_RESET should depend on Ocelot or Sparx5
Hi, On Wed, Oct 14, 2020 at 03:14:15PM +0200, Geert Uytterhoeven wrote: > To add support for Sparx5, the dependency on MSCC_OCELOT was removed. > However, this increases exposure of the driver question not only to > Sparx5 platforms, but to everyone. Hence re-add the dependency on > MSCC_OCELOT, and extend it with ARCH_SPARX5, to prevent asking the user > about this driver when configuring a kernel without Ocelot and Sparx5 > support. > > Fixes: ec871696b7776767 ("power: reset: ocelot: Add support for Sparx5") > Signed-off-by: Geert Uytterhoeven Thanks, queued. -- Sebastian > drivers/power/reset/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig > index 6361569aacb7eedf..d55b3727e00eb768 100644 > --- a/drivers/power/reset/Kconfig > +++ b/drivers/power/reset/Kconfig > @@ -129,6 +129,7 @@ config POWER_RESET_QCOM_PON > > config POWER_RESET_OCELOT_RESET > bool "Microsemi Ocelot reset driver" > + depends on MSCC_OCELOT || ARCH_SPARX5 || COMPILE_TEST > select MFD_SYSCON > help > This driver supports restart for Microsemi Ocelot SoC and similar. > -- > 2.17.1 > signature.asc Description: PGP signature
Re: [PATCH v4.4/bluetooth 1/2] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
On 15/10/2020 14:44, Hans-Christian Egtvedt (hegtvedt) wrote: > On 15/10/2020 14:02, Greg KH wrote: >> On Thu, Oct 15, 2020 at 11:18:39AM +, Hans-Christian Egtvedt (hegtvedt) >> wrote: >>> On 15/10/2020 11:57, Greg KH wrote: On Thu, Oct 15, 2020 at 09:43:32AM +0200, Hans-Christian Noren Egtvedt wrote: > From: Luiz Augusto von Dentz > --- > AFAICT, fixing CVE 2020-10135 Bluetooth impersonation attacks have been > left out for the 4.4 stable kernel. I cherry picked what I assume are > the appropriate two patches missing from the 4.9 stable kernel. Please > add them to upcoming 4.4 stable releases. Why are you merging 2 commits together? Please provide backports for all stable kernels, if you want to see this in the 4.4.y tree. We can not have someone move from an older tree to a newer one and have a regression. >>> >>> Agreed, I have managed to trick myself into thinking the 4.4.y branch >>> was left out, but I assume these patches are required for all LTS branches. >> >> They are, but if you have copies of them, please feel free to share >> them. > > I will repeat cherry-picking from a clean linux-stable git tree and send > patches, sorry for this noise. > > I see linux-5.8.y has partial patches, while the older branches need the > full series of three commits. I just discovered an additional fix. I sent three iterations. 5.8.y only requires a single cherry-pick. 4.9.y to 5.4.y has non-conflicting cherry-pick from upstream commits. 4.4.y needs to resolve a conflict when cherry-picking. I would still want Luiz to ack that this completes the mitigation for this Bluetooth vulnerability in the stable kernels. -- Best regards, Hans-Christian Noren Egtvedt pEpkey.asc Description: pEpkey.asc
Re: [PATCH 0/3] IRQ stack support for ARM
On 10/15/20 1:59 PM, Nick Desaulniers wrote: > On Thu, Oct 8, 2020 at 1:30 AM Russell King - ARM Linux admin > wrote: >> >> On Thu, Oct 08, 2020 at 12:45:30PM +0530, Maninder Singh wrote: >>> Observed Stack Overflow on 8KB kernel stack on ARM specially >>> incase on network interrupts, which results in undeterministic behaviour. >>> So there is need for per cpu dedicated IRQ stack for ARM. >>> >>> As ARm does not have extra co-processor register >>> to save thread info pointer, IRQ stack will be at some >>> performance cost, so code is under CONFIG_IRQ_STACK. >>> >>> and we don't have much knowledge and set up for CLANG >>> and ARM_UNWIND, so dependency added for both cases. >>> >>> Tested patch set with QEMU for latest kernel >>> and 4.1 kernel for ARM target with same patch set. >> >> You need to investigate and show where and why this is happening. My >> guess is you have a network driver that uses a lot of kernel stack >> space, which itself would be a bug. >> >> Note that there are compiler versions out there that mis-optimise and >> eat stack space - the kernel build should be warning if a function >> uses a large amount of stack. > > For tracking down those not-super-helpful compiler warnings, I wrote a > tool where if you rebuild with debug info, and give it the object file > and string of the function the compiler warned about it will parse the > DWARF to tell you the size of each local variable, and if it came from > an inline frame. Generally, it's possible to stack allocate something > that's way too big; instead those should be allocated on the heap. > https://github.com/ClangBuiltLinux/frame-larger-than > (I haven't had time to sit down and use it to resolve all outstanding > issues, but it has worked well for me in the past) Things get a bit more difficult with the network stack and you easily recurse into functions and blow up the stack size. This is especially true if you have some complex network tunneling or filtering going on. For one, in the 4.1 kernel that appears to have been used as a basis for this work, if you have CONFIG_BPF enabled but not CONFIG_BPF_JIT_ALWAYS_ON, __bpf_prog_run will require about 724 bytes of stack last I measured, that's nearly 10% of the stack that goes away just like that. -- Florian
Re: [PATCH 5/8] x86/clear_page: add clear_page_uncached()
On 2020-10-15 3:35 a.m., Borislav Petkov wrote: On Wed, Oct 14, 2020 at 08:37:44PM -0700, Ankur Arora wrote: I don't disagree but I think the selection of cached/uncached route should be made where we have enough context available to be able to choose to do this. This could be for example, done in mm_populate() or gup where if say the extent is larger than LLC-size, it takes the uncached path. Are there examples where we don't know the size? The case I was thinking of was that clear_huge_page() or faultin_page() would know the size to a page unit, while the higher level function would know the whole extent and could optimize differently based on that. Thanks Ankur
Re: [PATCH net 1/4] ptp: ptp_idt82p33: update to support adjphase
On Thu, Oct 15, 2020 at 07:30:38PM +, Min Li wrote: > When you have time, can you take a look at this change? Thanks Min, I think your series was posted during a time when net-next was closed. Please report the series. Thanks, Richard
Re: [PATCH 2/2] irqchip/sifive-plic: Fix the interrupt was enabled accidentally issue.
On 2020-10-12 14:57, Greentime Hu wrote: In commit 2ca0b460bbcb ("genirq/affinity: Make affinity setting if activated opt-in"), it added irqd_affinity_on_activate() checking in the function irq_set_affinity_deactivated() so it will return false here. In that case, it will call irq_try_set_affinity() -> plic_irq_toggle() which will enable the interrupt to cause the CPU hang. if (irq_set_affinity_deactivated()) return 0; ... irq_try_set_affinity(data, mask, force); [ 919.015783] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 919.020922] rcu: 0-...0: (0 ticks this GP) idle=7d2/1/0x4002 softirq=1424/1424 fqs=105807 [ 919.030295] (detected by 1, t=225825 jiffies, g=1561, q=3496) [ 919.036109] Task dump for CPU 0: [ 919.039321] kworker/0:1 R running task030 2 0x0008 [ 919.046359] Workqueue: events set_brightness_delayed [ 919.051302] Call Trace: [ 919.053738] [] __schedule+0x194/0x4de [ 982.035783] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 982.040923] rcu: 0-...0: (0 ticks this GP) idle=7d2/1/0x4002 softirq=1424/1424 fqs=113325 [ 982.050294] (detected by 1, t=241580 jiffies, g=1561, q=3509) [ 982.056108] Task dump for CPU 0: [ 982.059321] kworker/0:1 R running task030 2 0x0008 [ 982.066359] Workqueue: events set_brightness_delayed [ 982.071302] Call Trace: [ 982.073739] [] __schedule+0x194/0x4de [..] After applying this patch, the CPU hang issue can be fixed. Signed-off-by: Greentime Hu --- drivers/irqchip/irq-sifive-plic.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 4cc8a2657a6d..8a673d9cff69 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -183,10 +183,14 @@ static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) { struct plic_priv *priv = d->host_data; + struct irq_data *irqd; irq_domain_set_info(d, irq, hwirq, _chip, d->host_data, handle_fasteoi_irq, NULL, NULL); irq_set_noprobe(irq); + irqd = irq_get_irq_data(irq); + irqd_set_single_target(irqd); + irqd_set_affinity_on_activate(irqd); irq_set_affinity(irq, >lmask); return 0; } How does this fix anything? The plic driver doesn't have an activate callback, so how does it make any difference? I get the feeling this papers over another issue. M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags
On Thu, 15 Oct 2020 07:32:07 +0200 Mauro Carvalho Chehab wrote: > > That will apply to most (maybe all) of the structures mentioned in this > > file. > > I expected that if the documentation system now automatically recognizes > > 'struct foo', then it would render it in code font even when 'struct foo' > > isn't > > documented. Any particular reason why that isn't the case? Not like I care > > much myself, but it's a bit unexpected and it means this change actually > > makes > > the rendered documentation look worse... > > Yeah, I agree that using monospaced fonts on this case too would > be nice. The C domain actually uses italic monospaced fonts for > broken XREFs. > > I suspect that changing this at automarkup.py would be simple, but > not sure if it would be safe. > > Jon can tell more about that, as he's the author of automarkup, > but I suspect that the reason for the current behavior is to avoid > false-positives. Automarkup has always behaved that way because ... well, because nobody got around to changing it. I don't see any reason not to use a monospace font for such things, just without a link; shouldn't be a problem to do. I'll see if I can't get to it once things stabilize a bit. Thanks, jon
drivers/staging/media/rkvdec/rkvdec.c:967:34: warning: unused variable 'of_rkvdec_match'
Hi Boris, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 93b694d096cc10994c817730d4d50288f9ae3d66 commit: cd33c830448baf7b1e94da72eca069e3e1d050c9 media: rkvdec: Add the rkvdec driver date: 6 months ago config: x86_64-randconfig-a001-20201016 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd33c830448baf7b1e94da72eca069e3e1d050c9 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout cd33c830448baf7b1e94da72eca069e3e1d050c9 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/staging/media/rkvdec/rkvdec.c:967:34: warning: unused variable >> 'of_rkvdec_match' [-Wunused-const-variable] static const struct of_device_id of_rkvdec_match[] = { ^ 1 warning generated. vim +/of_rkvdec_match +967 drivers/staging/media/rkvdec/rkvdec.c 966 > 967 static const struct of_device_id of_rkvdec_match[] = { 968 { .compatible = "rockchip,rk3399-vdec" }, 969 { /* sentinel */ } 970 }; 971 MODULE_DEVICE_TABLE(of, of_rkvdec_match); 972 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [RFC PATCH 0/3] arm64: Implement reliable stack trace
I can't see the original patch. Can the original poster (Mark B?) add me to Cc on the next version? It's also good practice to add lkml as well. That way, those of us not copied can at least find the patch in the archives. live-patch...@vger.kernel.org would also be a good idea for this one. On Thu, Oct 15, 2020 at 04:49:51PM +0100, Mark Brown wrote: > On Thu, Oct 15, 2020 at 03:16:12PM +0100, Mark Rutland wrote: > > On Thu, Oct 15, 2020 at 03:39:37PM +0200, Miroslav Benes wrote: > > > > I'll just copy an excerpt from my notes about the required guarantees. > > > Written by Josh (CCed, he has better idea about the problem than me > > > anyway). > > > > It also needs to: > > > - detect preemption / page fault frames and return an error > > > - only return success if it reaches the end of the task stack; for user > > > tasks, that means the syscall barrier; for kthreads/idle tasks, that > > > means finding a defined thread entry point > > > - make sure it can't get into a recursive loop > > > - make sure each return address is a valid text address > > > - properly detect generated code hacks like function graph tracing and > > > kretprobes > > > " > > > It would be great if we could put something like the above into the > > kernel tree, either under Documentation/ or in a comment somewhere for > > the reliable stacktrace functions. > > Yes, please - the expecations are quite hard to follow at the minute, > implementing it involves quite a bit of guesswork and cargo culting to > figure out what the APIs are supposed to do. Documentation is indeed long overdue. I suppose everyone's looking at me. I can do that, but my bandwidth's limited for at least a few weeks. [ Currently in week 4 of traveling cross-country with a camper ("caravan" in British-speak?), National Lampoon vacation style. ] If by cargo culting, you mean reverse engineering the requirements due to lack of documentation, that's fair. Otherwise, if you see anything that doesn't make sense or that can be improved, let me know. > > AFAICT, existing architectures don't always handle all of the above in > > arch_stack_walk_reliable(). For example, it looks like x86 assumes > > unwiding through exceptions is reliable for !CONFIG_FRAME_POINTER, but I > > think this might not always be true. Why not? What else are the existing arches missing from the above list? > I certainly wouldn't have inferred the list from what's there :/ Fair, presumably because of missing documentation. > The searching for a defined thread entry point for example isn't > entirely visible in the implementations. For now I'll speak only of x86, because I don't quite remember how powerpc does it. For thread entry points, aka the "end" of the stack: - For ORC, the end of the stack is either pt_regs, or -- when unwinding from kthreads, idle tasks, or irqs/exceptions in entry code -- UNWIND_HINT_EMPTY (found by the unwinder's check for orc->end. [ Admittedly the implementation needs to be cleaned up a bit. EMPTY is too broad and needs to be split into UNDEFINED and ENTRY. ] - For frame pointers, by convention, the end of the stack for all tasks is a defined stack offset: end of stack page - sizeof(pt_regs). And yes, all that needs to be documented. -- Josh
Re: [PATCH] irqchip: MST_IRQ should depend on ARCH_MEDIATEK or ARCH_MSTARV7
On Wed, 14 Oct 2020 15:17:03 +0200, Geert Uytterhoeven wrote: > The MStar interrupt controller is only found on MStar, SigmaStar, and > Mediatek SoCs. Hence add dependencies on ARCH_MEDIATEK and > ARCH_MSTARV7, to prevent asking the user about the MStar interrupt > controller driver when configuring a kernel without support for MStar, > SigmaStar, and Mediatek SoCs. Applied to irq/irqchip-next, thanks! [1/1] irqchip/mst: MST_IRQ should depend on ARCH_MEDIATEK or ARCH_MSTARV7 commit: 61b0648d569aca932eab87a67f7ca0ffd3ea2b68 Cheers, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH v2 2/3] ARM: dts: rockchip: veyron: Remove 0 point from brightness-levels
On Wed, Oct 14, 2020 at 7:19 AM Daniel Thompson wrote: > > On Tue, Oct 13, 2020 at 01:01:02AM -0700, Alexandru Stan wrote: > > After the "PWM backlight interpolation adjustments" patches, the > > backlight interpolation works a little differently. The way these > > dts files were working before was relying on a bug (IMHO). > > > > Remove the 0-3 range since otherwise we would have a 252 long > > interpolation that would slowly go between 0 and 3, looking really bad > > in userspace. > > > > We don't need the 0% point, userspace seems to handle this just fine > > because it uses the bl_power property to turn off the display. > > > > Signed-off-by: Alexandru Stan > > Acked-by: Daniel Thompson Thank you! > > Note also shouldn't this be patch 1 of the set. AFAICT it makes sense > whether or not the interpolation algorithm is changed. Yeah, I guess it could be. Sorry I didn't think of it that way before, I'm used to landing things in a group. In particular on veyron I assume it will almost be a noop without having my driver patch (especially with the findings of 0% not being that important). Feel free to land this independently. > > > Daniel. Alexandru Stan (amstan)
Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks
Hi Michael, On 10/14/20 2:36 PM, Michael Auchter wrote: > After updating to v5.9, I've started seeing errors in the kernel log > when using device tree overlays. Specifically, the problem seems to > happen when removing a device tree overlay that contains two devices > with some dependency between them (e.g., a device that provides a clock > and a device that consumes that clock). Removing such an overlay results > in: > > OF: ERROR: memory leak, expected refcount 1 instead of 2, > of_node_get()/of_node_put() unbalanced - destroy > OF: ERROR: memory leak, expected refcount 1 instead of 2, > of_node_get()/of_node_put() unbalanced - destroy > > followed by hitting some REFCOUNT_WARNs in refcount.c > > In the first patch, I've included a unittest that can be used to > reproduce this when built with CONFIG_OF_UNITTEST [1]. > > I believe the issue is caused by the cleanup performed when releasing > the devlink device that's created to represent the dependency between > devices. The devlink device has references to the consumer and supplier > devices, which it drops in device_link_free; the devlink device's > release callback calls device_link_free via call_srcu. > > When the overlay is being removed, all devices are removed, and > eventually the release callback for the devlink device run, and > schedules cleanup using call_srcu. Before device_link_free can and call > put_device on the consumer/supplier, the rest of the overlay removal > process runs, resulting in the error traces above. > > Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait > for any pending device_link_free's to execute before continuing on with > the removal process. > > These patches resolve the issue, but probably not in the best way. In > particular, it seems strange to need to leak details of devlinks into > the device tree overlay code. So, I'd be curious to get some feedback or > hear any other ideas for how to resolve this issue. Thanks for finding the problem, analyzing it, creating a unittest, and creating a fix. I agree with your analysis that there are issues with the implementation of the test and fix. I'll dig into this to see if I can provide some useful improvements. -Frank > > Thanks, > Michael > > 1. Note that this isn't a very good unit test: it will report a "pass" >even if it fails with the aforementioned errors, as these errors >aren't propogated. > > Michael Auchter (3): > of: unittest: add test of overlay with devlinks > driver core: add device_links_barrier > of: dynamic: add device links barrier before detach > > drivers/base/core.c | 10 ++ > drivers/of/dynamic.c| 3 +++ > drivers/of/unittest-data/Makefile | 1 + > drivers/of/unittest-data/overlay_16.dts | 26 + > drivers/of/unittest.c | 16 +++ > include/linux/device.h | 1 + > 6 files changed, 57 insertions(+) > create mode 100644 drivers/of/unittest-data/overlay_16.dts >
Re: [PATCH v2] x86/insn, tools/x86: Fix some potential undefined behavior.
On October 15, 2020 9:12:16 AM PDT, Ian Rogers wrote: >From: Numfor Mbiziwo-Tiapo > >Don't perform unaligned loads in __get_next and __peek_nbyte_next as >these are forms of undefined behavior. > >These problems were identified using the undefined behavior sanitizer >(ubsan) with the tools version of the code and perf test. Part of this >patch was previously posted here: >https://lore.kernel.org/lkml/20190724184512.162887-4-n...@google.com/ > >v2. removes the validate_next check and merges the 2 changes into one >as >requested by Masami Hiramatsu > >Signed-off-by: Ian Rogers >Signed-off-by: Numfor Mbiziwo-Tiapo >--- > arch/x86/lib/insn.c | 4 ++-- > tools/arch/x86/lib/insn.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > >diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c >index 404279563891..be88ab250146 100644 >--- a/arch/x86/lib/insn.c >+++ b/arch/x86/lib/insn.c >@@ -20,10 +20,10 @@ > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > #define __get_next(t, insn) \ >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) >+ ({ t r; memcpy(, insn->next_byte, sizeof(t)); insn->next_byte += >sizeof(t); r; }) > > #define __peek_nbyte_next(t, insn, n) \ >- ({ t r = *(t*)((insn)->next_byte + n); r; }) >+ ({ t r; memcpy(, (insn)->next_byte + n, sizeof(t)); r; }) > > #define get_next(t, insn) \ > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; >__get_next(t, insn); }) >diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c >index 0151dfc6da61..92358c71a59e 100644 >--- a/tools/arch/x86/lib/insn.c >+++ b/tools/arch/x86/lib/insn.c >@@ -20,10 +20,10 @@ > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > #define __get_next(t, insn) \ >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) >+ ({ t r; memcpy(, insn->next_byte, sizeof(t)); insn->next_byte += >sizeof(t); r; }) > > #define __peek_nbyte_next(t, insn, n) \ >- ({ t r = *(t*)((insn)->next_byte + n); r; }) >+ ({ t r; memcpy(, (insn)->next_byte + n, sizeof(t)); r; }) > > #define get_next(t, insn) \ > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; >__get_next(t, insn); }) Wait, what? You are taking about x86-specific code, and on x86 unaligned memory accesses are supported, well-defined, and ubiquitous. This is B.S. at best, and unless the compiler turns the memcpy() right back into what you started with, deleterious for performance. If you have a *very* good reason for this kind of churn, wrap it in the unaligned access macros, but using memcpy() is insane. All you are doing is making the code worse. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [PATCH 5/8] x86/clear_page: add clear_page_uncached()
On 2020-10-15 3:40 a.m., Borislav Petkov wrote: On Wed, Oct 14, 2020 at 08:21:57PM -0700, Ankur Arora wrote: Also, if we did extend clear_page() to take the page-size as parameter we still might not have enough information (ex. a 4K or a 2MB page that clear_page() sees could be part of a GUP of a much larger extent) to decide whether to go uncached or not. clear_page* assumes 4K. All of the lowlevel asm variants do. So adding the size there won't bring you a whole lot. So you'd need to devise this whole thing differently. Perhaps have a clear_pages() helper which decides based on size what to do: uncached clearing or the clear_page() as is now in a loop. I think that'll work well for GB pages, where the clear_pages() helper has enough information to make a decision. But, unless I'm missing something, I'm not sure how that would work for say, a 1GB mm_populate() using 4K pages. The clear_page() (or clear_pages()) in that case would only see the 4K size. But let me think about this more (and look at the callsites as you suggest.) Looking at the callsites would give you a better idea I'd say. Thanks, yeah that's a good idea. Let me go do that. Ankur Thx.
linux-next: manual merge of the wireless-drivers tree with the net tree
Hi all, Today's linux-next merge of the wireless-drivers tree got a conflict in: tools/testing/selftests/net/Makefile between commit: 1a01727676a8 ("selftests: Add VRF route leaking tests") from the net tree and commit: b7cc6d3c5c91 ("selftests: net: Add drop monitor test") from the wireless-drivers (presumably because it has merged part of the net-next tree) tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc tools/testing/selftests/net/Makefile index 3e7fb1e70c77,4773ce72edbd.. --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@@ -19,7 -19,7 +19,8 @@@ TEST_PROGS += txtimestamp.s TEST_PROGS += vrf-xfrm-tests.sh TEST_PROGS += rxtimestamp.sh TEST_PROGS += devlink_port_split.py + TEST_PROGS += drop_monitor_tests.sh +TEST_PROGS += vrf_route_leaking.sh TEST_PROGS_EXTENDED := in_netns.sh TEST_GEN_FILES = socket nettest TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any pgpFGPCzLbkLS.pgp Description: OpenPGP digital signature
[PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
There is a misconfiguration in the bios of the gpio pin used for the interrupt in the T490s. When interrupts are enabled in the tpm_tis driver code this results in an interrupt storm. This was initially reported when we attempted to enable the interrupt code in the tpm_tis driver, which previously wasn't setting a flag to enable it. Due to the reports of the interrupt storm that code was reverted and we went back to polling instead of using interrupts. Now that we know the T490s problem is a firmware issue, add code to check if the system is a T490s and disable interrupts if that is the case. This will allow us to enable interrupts for everyone else. If the user has a fixed bios they can force the enabling of interrupts with tpm_tis.interrupts=1 on the kernel command line. Cc: jar...@kernel.org Cc: Peter Huewe Cc: Jason Gunthorpe Cc: Hans de Goede Reviewed-by: James Bottomley Signed-off-by: Jerry Snitselaar --- drivers/char/tpm/tpm_tis.c | 29 +++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 0b214963539d..4ed6e660273a 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "tpm.h" #include "tpm_tis_core.h" @@ -49,8 +50,8 @@ static inline struct tpm_tis_tcg_phy *to_tpm_tis_tcg_phy(struct tpm_tis_data *da return container_of(data, struct tpm_tis_tcg_phy, priv); } -static bool interrupts = true; -module_param(interrupts, bool, 0444); +static int interrupts = -1; +module_param(interrupts, int, 0444); MODULE_PARM_DESC(interrupts, "Enable interrupts"); static bool itpm; @@ -63,6 +64,28 @@ module_param(force, bool, 0444); MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry"); #endif +static int tpm_tis_disable_irq(const struct dmi_system_id *d) +{ + if (interrupts == -1) { + pr_notice("tpm_tis: %s detected: disabling interrupts.\n", d->ident); + interrupts = 0; + } + + return 0; +} + +static const struct dmi_system_id tpm_tis_dmi_table[] = { + { + .callback = tpm_tis_disable_irq, + .ident = "ThinkPad T490s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), + }, + }, + {} +}; + #if defined(CONFIG_PNP) && defined(CONFIG_ACPI) static int has_hid(struct acpi_device *dev, const char *hid) { @@ -192,6 +215,8 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) int irq = -1; int rc; + dmi_check_system(tpm_tis_dmi_table); + rc = check_acpi_tpm2(dev); if (rc) return rc; -- 2.28.0
[PATCH v2 1/4] block: keyslot-manager: Introduce passthrough keyslot manager
The device mapper may map over devices that have inline encryption capabilities, and to make use of those capabilities, the DM device must itself advertise those inline encryption capabilities. One way to do this would be to have the DM device set up a keyslot manager with a "sufficiently large" number of keyslots, but that would use a lot of memory. Also, the DM device itself has no "keyslots", and it doesn't make much sense to talk about "programming a key into a DM device's keyslot manager", so all that extra memory used to represent those keyslots is just wasted. All a DM device really needs to be able to do is advertise the crypto capabilities of the underlying devices in a coherent manner and expose a way to evict keys from the underlying devices. There are also devices with inline encryption hardware that do not have a limited number of keyslots. One can send a raw encryption key along with a bio to these devices (as opposed to typical inline encryption hardware that require users to first program a raw encryption key into a keyslot, and send the index of that keyslot along with the bio). These devices also only need the same things from the keyslot manager that DM devices need - a way to advertise crypto capabilities and potentially a way to expose a function to evict keys from hardware. So we introduce a "passthrough" keyslot manager that provides a way to represent a keyslot manager that doesn't have just a limited number of keyslots, and for which do not require keys to be programmed into keyslots. DM devices can set up a passthrough keyslot manager in their request queues, and advertise appropriate crypto capabilities based on those of the underlying devices. Blk-crypto does not attempt to program keys into any keyslots in the passthrough keyslot manager. Instead, if/when the bio is resubmitted to the underlying device, blk-crypto will try to program the key into the underlying device's keyslot manager. Signed-off-by: Satya Tangirala --- block/keyslot-manager.c | 41 + include/linux/keyslot-manager.h | 2 ++ 2 files changed, 43 insertions(+) diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c index 35abcb1ec051..5ad476dafeab 100644 --- a/block/keyslot-manager.c +++ b/block/keyslot-manager.c @@ -62,6 +62,11 @@ static inline void blk_ksm_hw_exit(struct blk_keyslot_manager *ksm) pm_runtime_put_sync(ksm->dev); } +static inline bool blk_ksm_is_passthrough(struct blk_keyslot_manager *ksm) +{ + return ksm->num_slots == 0; +} + /** * blk_ksm_init() - Initialize a keyslot manager * @ksm: The keyslot_manager to initialize. @@ -198,6 +203,10 @@ blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm, int err; *slot_ptr = NULL; + + if (blk_ksm_is_passthrough(ksm)) + return BLK_STS_OK; + down_read(>lock); slot = blk_ksm_find_and_grab_keyslot(ksm, key); up_read(>lock); @@ -318,6 +327,16 @@ int blk_ksm_evict_key(struct blk_keyslot_manager *ksm, struct blk_ksm_keyslot *slot; int err = 0; + if (blk_ksm_is_passthrough(ksm)) { + if (ksm->ksm_ll_ops.keyslot_evict) { + blk_ksm_hw_enter(ksm); + err = ksm->ksm_ll_ops.keyslot_evict(ksm, key, -1); + blk_ksm_hw_exit(ksm); + return err; + } + return 0; + } + blk_ksm_hw_enter(ksm); slot = blk_ksm_find_keyslot(ksm, key); if (!slot) @@ -353,6 +372,9 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm) { unsigned int slot; + if (blk_ksm_is_passthrough(ksm)) + return; + /* This is for device initialization, so don't resume the device */ down_write(>lock); for (slot = 0; slot < ksm->num_slots; slot++) { @@ -394,3 +416,22 @@ void blk_ksm_unregister(struct request_queue *q) { q->ksm = NULL; } + +/** + * blk_ksm_init_passthrough() - Init a passthrough keyslot manager + * @ksm: The keyslot manager to init + * + * Initialize a passthrough keyslot manager. + * Called by e.g. storage drivers to set up a keyslot manager in their + * request_queue, when the storage driver wants to manage its keys by itself. + * This is useful for inline encryption hardware that doesn't have the concept + * of keyslots, and for layered devices. + * + * See blk_ksm_init() for more details about the parameters. + */ +void blk_ksm_init_passthrough(struct blk_keyslot_manager *ksm) +{ + memset(ksm, 0, sizeof(*ksm)); + init_rwsem(>lock); +} +EXPORT_SYMBOL_GPL(blk_ksm_init_passthrough); diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h index 18f3f5346843..323e15dd6fa7 100644 --- a/include/linux/keyslot-manager.h +++ b/include/linux/keyslot-manager.h @@ -103,4 +103,6 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm); void
[PATCH v2 0/4] add support for inline encryption to device mapper
This patch series adds support for inline encryption to the device mapper. Patch 1 introduces the "passthrough" keyslot manager. The regular keyslot manager is designed for inline encryption hardware that have only a small fixed number of keyslots. A DM device itself does not actually have only a small fixed number of keyslots - it doesn't actually have any keyslots in the first place, and programming an encryption context into a DM device doesn't make much semantic sense. It is possible for a DM device to set up a keyslot manager with some "sufficiently large" number of keyslots in its request queue, so that upper layers can use the inline encryption capabilities of the DM device's underlying devices, but the memory being allocated for the DM device's keyslots is a waste since they won't actually be used by the DM device. The passthrough keyslot manager solves this issue - when the block layer sees that a request queue has a passthrough keyslot manager, it doesn't attempt to program any encryption context into the keyslot manager. The passthrough keyslot manager only allows the device to expose its inline encryption capabilities, and a way for upper layers to evict keys if necessary. There also exist inline encryption hardware that can handle encryption contexts directly, and allow users to pass them a data request along with the encryption context (as opposed to inline encryption hardware that require users to first program a keyslot with an encryption context, and then require the users to pass the keyslot index with the data request). Such devices can also make use of the passthrough keyslot manager. Patch 2 introduces a private field to struct blk_keyslot_manager that owners of the struct can use for any purpose. The struct blk_keyslot_manager has been embedded within other structures directly (like in struct ufs_hba in drivers/scsi/ufs/ufshcd.h), but we don't want to do that with struct mapped_device. So, the device mapper patches later in this series use the private field to hold a pointer to the associated struct mapped_device, since we can't use container_of() anymore. Patch 3 introduces the changes for inline encryption support for the device mapper. A DM device only exposes the intersection of the crypto capabilities of its underlying devices. This is so that in case a bio with an encryption context is eventually mapped to an underlying device that doesn't support that encryption context, the blk-crypto-fallback's cipher tfms are allocated ahead of time by the call to blk_crypto_start_using_key. Each DM target can now also specify that it "may_passthrough_inline_crypto" to opt-in to supporting passing through the underlying inline encryption capabilities. This flag is needed because it doesn't make much semantic sense for certain targets like dm-crypt to expose the underlying inline encryption capabilities to the upper layers. Again, the DM exposes inline encryption capabilities of the underlying devices only if all of them opt-in to passing through inline encryption support. A DM device's keyslot manager is set up whenever a new table is swapped in. This patch only allows the keyslot manager's capabilities to *expand* because of table changes. Any attempts to load a new table that would cause crypto capabilities to be dropped are rejected. The crypto capabilities of a new table are also verified when the table is loaded (and the load is rejected if crypto capabilities will be dropped because of the new table), but the keyslot manager for the DM device is only modified when the table is actually swapped in. This patch also only exposes the intersection of the underlying device's capabilities, which has the effect of causing en/decryption of a bio to fall back to the kernel crypto API (if the fallback is enabled) whenever any of the underlying devices doesn't support the encryption context of the bio - it might be possible to make the bio only fall back to the kernel crypto API if the bio's target underlying device doesn't support the bio's encryption context, but the use case may be uncommon enough in the first place not to warrant worrying about it right now. Patch 4 makes some DM targets opt-in to passing through inline encryption support. It does not (yet) try to enable this option with dm-raid, since users can "hot add" disks to a raid device, which makes this not completely straightforward (we'll need to ensure that any "hot added" disks must have a superset of the inline encryption capabilities of the rest of the disks in the raid device, due to the way Patch 2 of this series works). Changes v1 => v2: - Introduce private field to struct blk_keyslot_manager - Allow the DM keyslot manager to expand its crypto capabilities if the table is changed. - Make DM reject table changes that would otherwise cause crypto capabilities to be dropped. - Allocate the DM device's keyslot manager only when at least one crypto capability is supported (since a NULL value for q->ksm
[PATCH v2 3/4] dm: add support for passing through inline crypto support
Update the device-mapper core to support exposing the inline crypto support of the underlying device(s) through the device-mapper device. This works by creating a "passthrough keyslot manager" for the dm device, which declares support for encryption settings which all underlying devices support. When a supported setting is used, the bio cloning code handles cloning the crypto context to the bios for all the underlying devices. When an unsupported setting is used, the blk-crypto fallback is used as usual. Crypto support on each underlying device is ignored unless the corresponding dm target opts into exposing it. This is needed because for inline crypto to semantically operate on the original bio, the data must not be transformed by the dm target. Thus, targets like dm-linear can expose crypto support of the underlying device, but targets like dm-crypt can't. (dm-crypt could use inline crypto itself, though.) When a key is evicted from the dm device, it is evicted from all underlying devices. A DM device's table can only be changed if the "new" inline encryption capabilities are a superset of the "old" inline encryption capabilities. Attempts to make changes to the table that result in some inline encryption capability becoming no longer supported will be rejected. Co-developed-by: Eric Biggers Signed-off-by: Eric Biggers Signed-off-by: Satya Tangirala --- block/blk-crypto.c | 1 + block/keyslot-manager.c | 89 + drivers/md/dm-ioctl.c | 8 ++ drivers/md/dm.c | 217 +++- drivers/md/dm.h | 19 +++ include/linux/device-mapper.h | 6 + include/linux/keyslot-manager.h | 17 +++ 7 files changed, 356 insertions(+), 1 deletion(-) diff --git a/block/blk-crypto.c b/block/blk-crypto.c index 5da43f0973b4..c2be8f15006c 100644 --- a/block/blk-crypto.c +++ b/block/blk-crypto.c @@ -409,3 +409,4 @@ int blk_crypto_evict_key(struct request_queue *q, */ return blk_crypto_fallback_evict_key(key); } +EXPORT_SYMBOL_GPL(blk_crypto_evict_key); diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c index 5ad476dafeab..e16e4a074765 100644 --- a/block/keyslot-manager.c +++ b/block/keyslot-manager.c @@ -416,6 +416,95 @@ void blk_ksm_unregister(struct request_queue *q) { q->ksm = NULL; } +EXPORT_SYMBOL_GPL(blk_ksm_unregister); + +/** + * blk_ksm_intersect_modes() - restrict supported modes by child device + * @parent: The keyslot manager for parent device + * @child: The keyslot manager for child device, or NULL + * + * Clear any crypto mode support bits in @parent that aren't set in @child. + * If @child is NULL, then all parent bits are cleared. + * + * Only use this when setting up the keyslot manager for a layered device, + * before it's been exposed yet. + */ +void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent, +const struct blk_keyslot_manager *child) +{ + if (child) { + unsigned int i; + + parent->max_dun_bytes_supported = + min(parent->max_dun_bytes_supported, + child->max_dun_bytes_supported); + for (i = 0; i < ARRAY_SIZE(child->crypto_modes_supported); +i++) { + parent->crypto_modes_supported[i] &= + child->crypto_modes_supported[i]; + } + } else { + parent->max_dun_bytes_supported = 0; + memset(parent->crypto_modes_supported, 0, + sizeof(parent->crypto_modes_supported)); + } +} +EXPORT_SYMBOL_GPL(blk_ksm_intersect_modes); + +/** + * blk_ksm_is_superset() - Check if a KSM supports a superset of crypto modes + *and DUN bytes that another KSM supports. + * @ksm_superset: The KSM that we want to verify is a superset + * @ksm_subset: The KSM that we want to verify is a subset + * + * Return: True if @ksm_superset supports a superset of the crypto modes and DUN + *bytes that @ksm_subset supports. + */ +bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset, +struct blk_keyslot_manager *ksm_subset) +{ + int i; + + if (!ksm_subset) + return true; + + if (!ksm_superset) + return false; + + for (i = 0; i < ARRAY_SIZE(ksm_superset->crypto_modes_supported); i++) { + if (ksm_subset->crypto_modes_supported[i] & + (~ksm_superset->crypto_modes_supported[i])) { + return false; + } + } + + if (ksm_subset->max_dun_bytes_supported > + ksm_superset->max_dun_bytes_supported) { + return false; + } + + return true; +} +EXPORT_SYMBOL_GPL(blk_ksm_is_superset); + +/** + * blk_ksm_update_capabilities() - Update the restrictions of a KSM to those of + *
[PATCH v2 4/4] dm: enable may_passthrough_inline_crypto on some targets
dm-linear and dm-flakey obviously can pass through inline crypto support. Co-developed-by: Eric Biggers Signed-off-by: Eric Biggers Signed-off-by: Satya Tangirala --- drivers/md/dm-flakey.c | 1 + drivers/md/dm-linear.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c index a2cc9e45cbba..655286dacc35 100644 --- a/drivers/md/dm-flakey.c +++ b/drivers/md/dm-flakey.c @@ -253,6 +253,7 @@ static int flakey_ctr(struct dm_target *ti, unsigned int argc, char **argv) ti->num_discard_bios = 1; ti->per_io_data_size = sizeof(struct per_bio_data); ti->private = fc; + ti->may_passthrough_inline_crypto = true; return 0; bad: diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c index 00774b5d7668..345e22b9be5d 100644 --- a/drivers/md/dm-linear.c +++ b/drivers/md/dm-linear.c @@ -62,6 +62,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv) ti->num_secure_erase_bios = 1; ti->num_write_same_bios = 1; ti->num_write_zeroes_bios = 1; + ti->may_passthrough_inline_crypto = true; ti->private = lc; return 0; -- 2.29.0.rc1.297.gfa9743e501-goog
Re: [linux-stable-rc:linux-5.4.y 665/2391] drivers/android/binder.c:3776: Error: unrecognized keyword/register name `l.lwz
+openrisc folks On Thu, Oct 15, 2020 at 11:28 PM kernel test robot wrote: > tree: > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-5.4.y > head: 85b0841aab15c12948af951d477183ab3df7de14 > commit: c5665cafbedd2e2a523fe933e452391a02d3adb3 [665/2391] binder: Prevent > context manager from incrementing ref 0 > config: openrisc-randconfig-r002-20201014 (attached as .config) > compiler: or1k-linux-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/commit/?id=c5665cafbedd2e2a523fe933e452391a02d3adb3 > git remote add linux-stable-rc > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > git fetch --no-tags linux-stable-rc linux-5.4.y > git checkout c5665cafbedd2e2a523fe933e452391a02d3adb3 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=openrisc > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >drivers/android/binder.c: Assembler messages: > >> drivers/android/binder.c:3776: Error: unrecognized keyword/register name > >> `l.lwz ?ap,4(r25)' >drivers/android/binder.c:3781: Error: unrecognized keyword/register name > `l.addi ?ap,r0,0' binder is basically doing this: u64 data_ptr; if (get_user(data_ptr, (u64 __user *)ptr)) return -EFAULT; and GCC complains that that doesn't turn into valid assembly on openrisc, where get_user() of size 8 expands into this: #define __get_user_asm2(x, addr, err) \ { \ unsigned long long __gu_tmp;\ __asm__ __volatile__( \ "1: l.lwz %1,0(%2)\n" \ "2: l.lwz %H1,4(%2)\n" \ "3:\n" \ ".section .fixup,\"ax\"\n" \ "4: l.addi %0,r0,%3\n" \ " l.addi %1,r0,0\n" \ " l.addi %H1,r0,0\n" \ " l.j 3b\n" \ " l.nop\n"\ ".previous\n" \ ".section __ex_table,\"a\"\n" \ " .align 2\n" \ " .long 1b,4b\n" \ " .long 2b,4b\n" \ ".previous" \ : "=r"(err), "="(__gu_tmp)\ : "r"(addr), "i"(-EFAULT), "0"(err)); \ (x) = (__typeof__(*(addr)))(\ (__typeof__((x)-(x)))__gu_tmp); \ } and apparently the "l.lwz %H1,4(%2)" and "l.addi %H1,r0,0" don't turn into valid assembly when %H1 expands to "?ap"? I don't know anything about OpenRISC, but this seems like it's probably an issue in the get_user() implementation.
[PATCH v2 2/4] block: add private field to struct keyslot_manager
Add a (void *) pointer to struct keyslot_manager that the owner of the struct can use for any purpose it wants. Right now, the struct keyslot_manager is expected to be embedded directly into other structs (and the owner of the keyslot_manager would use container_of() to access any other data the owner needs). However, this might take up more space than is acceptable, and it would be better to be able to add only a pointer to a struct keyslot_manager into other structs rather than embed the entire struct directly. But container_of() can't be used when only the pointer to the keyslot_manager is embded. The primary motivation of this patch is to get around that issue. Signed-off-by: Satya Tangirala --- include/linux/keyslot-manager.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h index 323e15dd6fa7..37f1022b256f 100644 --- a/include/linux/keyslot-manager.h +++ b/include/linux/keyslot-manager.h @@ -59,6 +59,9 @@ struct blk_keyslot_manager { /* Device for runtime power management (NULL if none) */ struct device *dev; + /* Private data for owner */ + void *priv; + /* Here onwards are *private* fields for internal keyslot manager use */ unsigned int num_slots; -- 2.29.0.rc1.297.gfa9743e501-goog
Re: linux-next: manual merge of the usb tree with the pci tree
Hi all, On Mon, 21 Sep 2020 15:18:07 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the usb tree got a conflict in: > > drivers/pci/controller/pcie-brcmstb.c > > between commit: > > 1cf1b0a6dd95 ("PCI: brcmstb: Add bcm7278 register info") > > from the pci tree and commit: > > f48cc509c935 ("Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when > present"") > > from the usb tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > > diff --cc drivers/pci/controller/pcie-brcmstb.c > index 6e7aa82a54a3,bac63d04297f.. > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@@ -1213,8 -929,6 +1211,7 @@@ static int brcm_pcie_probe(struct platf > { > struct device_node *np = pdev->dev.of_node, *msi_np; > struct pci_host_bridge *bridge; > - struct device_node *fw_np; > +const struct pcie_cfg_data *data; > struct brcm_pcie *pcie; > int ret; > This is now a conflict between the pci tree and Linus' tree. -- Cheers, Stephen Rothwell pgpTHoKAh_Xc4.pgp Description: OpenPGP digital signature
Re: [PATCH v2] x86/insn, tools/x86: Fix some potential undefined behavior.
On Thu, Oct 15, 2020 at 2:35 PM wrote: > > On October 15, 2020 9:12:16 AM PDT, Ian Rogers wrote: > >From: Numfor Mbiziwo-Tiapo > > > >Don't perform unaligned loads in __get_next and __peek_nbyte_next as > >these are forms of undefined behavior. > > > >These problems were identified using the undefined behavior sanitizer > >(ubsan) with the tools version of the code and perf test. Part of this > >patch was previously posted here: > >https://lore.kernel.org/lkml/20190724184512.162887-4-n...@google.com/ > > > >v2. removes the validate_next check and merges the 2 changes into one > >as > >requested by Masami Hiramatsu > > > >Signed-off-by: Ian Rogers > >Signed-off-by: Numfor Mbiziwo-Tiapo > >--- > > arch/x86/lib/insn.c | 4 ++-- > > tools/arch/x86/lib/insn.c | 4 ++-- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > >diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c > >index 404279563891..be88ab250146 100644 > >--- a/arch/x86/lib/insn.c > >+++ b/arch/x86/lib/insn.c > >@@ -20,10 +20,10 @@ > > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > > > #define __get_next(t, insn) \ > >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) > >+ ({ t r; memcpy(, insn->next_byte, sizeof(t)); insn->next_byte += > >sizeof(t); r; }) > > > > #define __peek_nbyte_next(t, insn, n) \ > >- ({ t r = *(t*)((insn)->next_byte + n); r; }) > >+ ({ t r; memcpy(, (insn)->next_byte + n, sizeof(t)); r; }) > > > > #define get_next(t, insn) \ > > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; > >__get_next(t, insn); }) > >diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c > >index 0151dfc6da61..92358c71a59e 100644 > >--- a/tools/arch/x86/lib/insn.c > >+++ b/tools/arch/x86/lib/insn.c > >@@ -20,10 +20,10 @@ > > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > > > #define __get_next(t, insn) \ > >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) > >+ ({ t r; memcpy(, insn->next_byte, sizeof(t)); insn->next_byte += > >sizeof(t); r; }) > > > > #define __peek_nbyte_next(t, insn, n) \ > >- ({ t r = *(t*)((insn)->next_byte + n); r; }) > >+ ({ t r; memcpy(, (insn)->next_byte + n, sizeof(t)); r; }) > > > > #define get_next(t, insn) \ > > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; > >__get_next(t, insn); }) > > Wait, what? > > You are taking about x86-specific code, and on x86 unaligned memory accesses > are supported, well-defined, and ubiquitous. On why this is undefined behavior: https://lore.kernel.org/lkml/CAP-5=fU2XBoOa2=00vcuwyqsluzmsmzuxy63zjt9rz-nj+v...@mail.gmail.com/ > This is B.S. at best, and unless the compiler turns the memcpy() right back > into what you started with, deleterious for performance. On performance, the memcpys are fixed size and so lowered to loads on x86 by any reasonable compiler. See the thread above. > If you have a *very* good reason for this kind of churn, wrap it in the > unaligned access macros, but using memcpy() is insane. All you are doing is > making the code worse. The decoder is a shared code and using unaligned macros makes life hard for the other users of the code. Memcpy is the "standard" workaround for this kind of undefined behavior. https://lore.kernel.org/lkml/e4269cb2-d8e6-da26-6afd-a9df72d4b...@intel.com/ For motivation, beyond just having perf be sanitizer clean, see discussion here: https://lore.kernel.org/lkml/CAP-5=fuosgy3naztsbf3ylepabss7opsxlkcx36xkezzm34...@mail.gmail.com/ https://lore.kernel.org/lkml/160208761921.7002.1321765913567405137.tip-bot2@tip-bot2/ Thanks, Ian > -- > Sent from my Android device with K-9 Mail. Please excuse my brevity.
[tip:master] BUILD SUCCESS 3c095241232f26d9ca68df129cd6fa4e95bbfc31
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master branch HEAD: 3c095241232f26d9ca68df129cd6fa4e95bbfc31 Merge branch 'linus' elapsed time: 724m configs tested: 145 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm sama5_defconfig sparc sparc32_defconfig armmulti_v5_defconfig mipsgpr_defconfig arm pxa168_defconfig powerpc mpc834x_mds_defconfig arm cm_x300_defconfig arm zx_defconfig powerpc mpc8560_ads_defconfig powerpc tqm8555_defconfig arm alldefconfig mipsjmr3927_defconfig arm colibri_pxa300_defconfig armxcep_defconfig alpha defconfig powerpc eiger_defconfig mips ip32_defconfig powerpc motionpro_defconfig mips malta_defconfig arm lpc32xx_defconfig arm simpad_defconfig arm at91_dt_defconfig armlart_defconfig pariscgeneric-64bit_defconfig x86_64 alldefconfig riscv defconfig riscvnommu_k210_defconfig mips ip28_defconfig i386 alldefconfig arcvdk_hs38_defconfig powerpc pmac32_defconfig powerpc sbc8548_defconfig sh ul2_defconfig mips sb1250_swarm_defconfig armzeus_defconfig powerpc ep88xc_defconfig sh alldefconfig powerpc cm5200_defconfig arm collie_defconfig mips tb0287_defconfig powerpc mgcoge_defconfig arm tct_hammer_defconfig armvexpress_defconfig powerpc tqm8540_defconfig arm nhk8815_defconfig arm lubbock_defconfig sparc sparc64_defconfig arm pxa255-idp_defconfig s390 zfcpdump_defconfig sh se7705_defconfig powerpc mpc834x_itxgp_defconfig arm ep93xx_defconfig armspear3xx_defconfig powerpc mpc885_ads_defconfig sh polaris_defconfig powerpc powernv_defconfig shecovec24-romimage_defconfig powerpc ppc6xx_defconfig armmagician_defconfig shmigor_defconfig arm viper_defconfig sh sh7710voipgw_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20201015 i386 randconfig-a006-20201015 i386 randconfig-a001-20201015 i386
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Thu, Sep 24, 2020 at 09:40:22AM -0400, Mike Snitzer wrote: > On Thu, Sep 24 2020 at 3:48am -0400, > Satya Tangirala wrote: > > > On Wed, Sep 23, 2020 at 09:21:03PM -0400, Mike Snitzer wrote: > > > On Wed, Sep 09 2020 at 7:44pm -0400, > > > Satya Tangirala wrote: > > > > > > > From: Eric Biggers > > > > > > > > Update the device-mapper core to support exposing the inline crypto > > > > support of the underlying device(s) through the device-mapper device. > > > > > > > > This works by creating a "passthrough keyslot manager" for the dm > > > > device, which declares support for encryption settings which all > > > > underlying devices support. When a supported setting is used, the bio > > > > cloning code handles cloning the crypto context to the bios for all the > > > > underlying devices. When an unsupported setting is used, the blk-crypto > > > > fallback is used as usual. > > > > > > > > Crypto support on each underlying device is ignored unless the > > > > corresponding dm target opts into exposing it. This is needed because > > > > for inline crypto to semantically operate on the original bio, the data > > > > must not be transformed by the dm target. Thus, targets like dm-linear > > > > can expose crypto support of the underlying device, but targets like > > > > dm-crypt can't. (dm-crypt could use inline crypto itself, though.) > > > > > > > > When a key is evicted from the dm device, it is evicted from all > > > > underlying devices. > > > > > > > > Signed-off-by: Eric Biggers > > > > Co-developed-by: Satya Tangirala > > > > Signed-off-by: Satya Tangirala > > > > --- > > > > block/blk-crypto.c | 1 + > > > > block/keyslot-manager.c | 34 > > > > drivers/md/dm-core.h| 4 ++ > > > > drivers/md/dm-table.c | 52 +++ > > > > drivers/md/dm.c | 92 - > > > > include/linux/device-mapper.h | 6 +++ > > > > include/linux/keyslot-manager.h | 7 +++ > > > > 7 files changed, 195 insertions(+), 1 deletion(-) > > > > > > > > > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h > > > > index c4ef1fceead6..4542050eebfc 100644 > > > > --- a/drivers/md/dm-core.h > > > > +++ b/drivers/md/dm-core.h > > > > @@ -12,6 +12,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > #include > > > > > > > > @@ -49,6 +50,9 @@ struct mapped_device { > > > > > > > > int numa_node_id; > > > > struct request_queue *queue; > > > > +#ifdef CONFIG_BLK_INLINE_ENCRYPTION > > > > + struct blk_keyslot_manager ksm; > > > > +#endif > > > > > > > > atomic_t holders; > > > > atomic_t open_count; > > > > > > Any reason you placed the ksm member where you did? > > > > As in, any reason why it's placed right after the struct request_queue > > *queue? The ksm is going to be set up in the request_queue and is a part > > of the request_queue is some sense, so it seemed reasonable to me to > > group them togetherbut I don't think there's any reason it *has* to > > be there, if you think it should be put elsewhere (or maybe I'm > > misunderstanding your question :) ). > > Placing the full struct where you did is highly disruptive to the prior > care taken to tune alignment of struct members within mapped_device. > Ah I see - sorry about that! I ended up removing it entirely in the next version of this series while trying to address this and your other comments :). The next version is at https://lore.kernel.org/linux-block/20201015214632.41951-5-sat...@google.com/ > Switching to a pointer will be less so, but even still it might be best > to either find a hole in the struct (not looked recently, but there may > not be one) or simply put it at the end of the structure. > > The pahole utility is very useful for this kind of struct member > placement, etc. But it is increasingly unavailable in modern Linux > distros... > > Mike >
Re: [PATCH v2 2/3] selftests/run_kselftest.sh: Make each test individually selectable
On Thu, Oct 15, 2020 at 02:57:34PM +0530, Naresh Kamboju wrote: > On Tue, 29 Sep 2020 at 01:56, Kees Cook wrote: > > > > Currently with run_kselftest.sh there is no way to choose which test > > we could run. All the tests listed in kselftest-list.txt are all run > > every time. This patch enhanced the run_kselftest.sh to make the test > > collections (or tests) individually selectable. e.g.: > > > > $ ./run_kselftest.sh -c seccomp -t timers:posix_timers -t timers:nanosleep > > > > Additionally adds a way to list all known tests with "-l", usage > > with "-h", and perform a dry run without running tests with "-n". > > > While testing this patch set on LAVA the skip test functionality is not > working. > We may have to revisit test definitions kselftest skip logic > or else > may add one more option to skip a given test on run_kselftest.sh script. > > ref: > https://github.com/Linaro/test-definitions/blob/master/automated/linux/kselftest/kselftest.sh#L196 Yes, LAVA's hack to skip tests needs to be adjusted. Here's what it should probably look like: https://github.com/Linaro/test-definitions/pull/231 -- Kees Cook
Re: [PATCH v2] net: usb: rtl8150: don't incorrectly assign random MAC addresses
Hi Greg, On Mon, 12 Oct 2020 09:14:28 +1100 Stephen Rothwell wrote: > > On Sun, 11 Oct 2020 23:00:30 +0530 Anant Thazhemadam > wrote: > > > > In set_ethernet_addr(), if get_registers() succeeds, the ethernet address > > that was read must be copied over. Otherwise, a random ethernet address > > must be assigned. > > > > get_registers() returns 0 if successful, and negative error number > > otherwise. However, in set_ethernet_addr(), this return value is > > incorrectly checked. > > > > Since this return value will never be equal to sizeof(node_id), a > > random MAC address will always be generated and assigned to the > > device; even in cases when get_registers() is successful. > > > > Correctly modifying the condition that checks if get_registers() was > > successful or not fixes this problem, and copies the ethernet address > > appropriately. > > > > Fixes: f45a4248ea4c ("net: usb: rtl8150: set random MAC address when > > set_ethernet_addr() fails") > > Signed-off-by: Anant Thazhemadam > > --- > > Changes in v2: > > * Fixed the format of the Fixes tag > > * Modified the commit message to better describe the issue being > > fixed > > > > +CCing Stephen and linux-next, since the commit fixed isn't in the > > networking > > tree, but is present in linux-next. > > > > drivers/net/usb/rtl8150.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c > > index f020401adf04..bf8a60533f3e 100644 > > --- a/drivers/net/usb/rtl8150.c > > +++ b/drivers/net/usb/rtl8150.c > > @@ -261,7 +261,7 @@ static void set_ethernet_addr(rtl8150_t *dev) > > > > ret = get_registers(dev, IDR, sizeof(node_id), node_id); > > > > - if (ret == sizeof(node_id)) { > > + if (!ret) { > > ether_addr_copy(dev->netdev->dev_addr, node_id); > > } else { > > eth_hw_addr_random(dev->netdev); > > -- > > 2.25.1 > > > > I will apply the above patch to the merge of the usb tree today to fix > up a semantic conflict between the usb tree and Linus' tree. It looks like you forgot to mention this one to Linus :-( It should probably say: Fixes: b2a0f274e3f7 ("net: rtl8150: Use the new usb control message API.") -- Cheers, Stephen Rothwell pgpI7gkNKtbKq.pgp Description: OpenPGP digital signature
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
James should this get tacked on the end of your patchset? Regards, Jerry
Re: [PATCH] compiler.h: Clarify comment about the need for barrier_data()
On Thu, Oct 15, 2020 at 09:09:11PM +, David Laight wrote: > From: Arvind Sankar > > Sent: 15 October 2020 19:14 > > > > Be clear about @ptr vs the variable that @ptr points to, and add some > > more details as to why the special barrier_data() macro is required. > > > > Signed-off-by: Arvind Sankar > > --- > > include/linux/compiler.h | 33 ++--- > > 1 file changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 93035d7fee0d..d8cee7c8968d 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -86,17 +86,28 @@ void ftrace_likely_update(struct ftrace_likely_data *f, > > int val, > > > > #ifndef barrier_data > > /* > > - * This version is i.e. to prevent dead stores elimination on @ptr > > - * where gcc and llvm may behave differently when otherwise using > > - * normal barrier(): while gcc behavior gets along with a normal > > - * barrier(), llvm needs an explicit input variable to be assumed > > - * clobbered. The issue is as follows: while the inline asm might > > - * access any memory it wants, the compiler could have fit all of > > - * @ptr into memory registers instead, and since @ptr never escaped > > - * from that, it proved that the inline asm wasn't touching any of > > - * it. This version works well with both compilers, i.e. we're telling > > - * the compiler that the inline asm absolutely may see the contents > > - * of @ptr. See also: https://llvm.org/bugs/show_bug.cgi?id=15495 > > + * This version is to prevent dead stores elimination on @ptr where gcc and > > + * llvm may behave differently when otherwise using normal barrier(): > > while gcc > > + * behavior gets along with a normal barrier(), llvm needs an explicit > > input > > + * variable to be assumed clobbered. > > + * > > + * Its primary use is in implementing memzero_explicit(), which is used for > > + * clearing temporary data that may contain secrets. > > + * > > + * The issue is as follows: while the inline asm might access any memory it > > + * wants, the compiler could have fit all of the variable that @ptr points > > to > > + * into registers instead, and if @ptr never escaped from the function, it > > + * proved that the inline asm wasn't touching any of it. gcc only > > eliminates > > + * dead stores if the variable was actually allocated in registers, but > > llvm > > + * reasons that the variable _could_ have been in registers, so the inline > > asm > > + * can't reliably access it anyway, and eliminates dead stores even if the > > + * variable is actually in memory. > > I think I'd just say something like: > > Although the compiler must assume a "memory" clobber may affect all > memory, local variables (on stack) cannot actually be visible to the > asm unless their address has been passed to an external function. > So the compiler may assume such variables cannot be affected by > a normal asm volatile(::"memory") barrier(). > Passing the address of the local variables to the asm barrier > is enough to tell the compiler that the asm can 'see' the variables > (and spill anything held in registers to the stack) so that > the "memory" clobber has the expected effect. > > This is necessary to get llvm to do a memset() of on-stack data > at the end of a function to clear memory that contains secrets. > > David I think it's helpful to have the more detailed explanation about register variables -- at first glance, it's a bit mystifying as to why the compiler would think that the asm can't access the stack. Spilling registers to the stack is actually an undesirable side-effect of the workaround.
Re: [PATCH v3 0/3] Actually fix freelist pointer vs redzoning
On Thu, Oct 15, 2020 at 11:44:15AM +0200, Vlastimil Babka wrote: > On 10/15/20 10:23 AM, Christopher Lameter wrote: > > On Wed, 14 Oct 2020, Kees Cook wrote: > > > > > Note on patch 2: Christopher NAKed it, but I actually think this is a > > > reasonable thing to add -- the "too small" check is only made when built > > > with CONFIG_DEBUG_VM, so it *is* actually possible for someone to trip > > > over this directly, even if it would never make it into a released > > > kernel. I see no reason to just leave this foot-gun in place, though, so > > > we might as well just fix it too. (Which seems to be what Longman was > > > similarly supporting, IIUC.) > > > > Well then remove the duplication of checks. The NAK was there because it > > seems that you were not aware of the existing checks. > > > > > Anyway, if patch 2 stays NAKed, that's fine. It's entirely separable, > > > and the other 2 can land. :) > > > > Just deal with the old checks too and it will be fine. > > Yeah, the existing check is under CONFIG_DEBUG_VM, which means it's not > active on some configurations. Creating a cache is not exactly fast path > operation, so I would remove this guard. > As for the minimum size check, I would probably remove it (but watch out if > SLAB/SLOB can handle it). It's not effective to use slab cache for 4-byte > objects, but why make it an error. Err, why did the check exist to begin with? If the check isn't wanted, that's one thing, but I was just trying to fix what I saw in the redzone handling. What is preferred here? 1) drop patch 2 2) keep patch 2, but also: a) validate slab/slob can handle < word-sized allocations b) remove check in kmem_cache_sanity_check option 2a seems like it could be fragile if I miss something. I think I'd rather just take option 1. -- Kees Cook
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Thu, Sep 24, 2020 at 10:23:54AM -0400, Mike Snitzer wrote: > On Thu, Sep 24 2020 at 3:38am -0400, > Satya Tangirala wrote: > > > On Wed, Sep 23, 2020 at 09:21:03PM -0400, Mike Snitzer wrote: > > > On Wed, Sep 09 2020 at 7:44pm -0400, > > > Satya Tangirala wrote: > > > > > > > From: Eric Biggers > > > > > > > > Update the device-mapper core to support exposing the inline crypto > > > > support of the underlying device(s) through the device-mapper device. > > > > > > > > This works by creating a "passthrough keyslot manager" for the dm > > > > device, which declares support for encryption settings which all > > > > underlying devices support. When a supported setting is used, the bio > > > > cloning code handles cloning the crypto context to the bios for all the > > > > underlying devices. When an unsupported setting is used, the blk-crypto > > > > fallback is used as usual. > > > > > > > > Crypto support on each underlying device is ignored unless the > > > > corresponding dm target opts into exposing it. This is needed because > > > > for inline crypto to semantically operate on the original bio, the data > > > > must not be transformed by the dm target. Thus, targets like dm-linear > > > > can expose crypto support of the underlying device, but targets like > > > > dm-crypt can't. (dm-crypt could use inline crypto itself, though.) > > > > > > > > When a key is evicted from the dm device, it is evicted from all > > > > underlying devices. > > > > > > > > Signed-off-by: Eric Biggers > > > > Co-developed-by: Satya Tangirala > > > > Signed-off-by: Satya Tangirala > > > > --- > > > > block/blk-crypto.c | 1 + > > > > block/keyslot-manager.c | 34 > > > > drivers/md/dm-core.h| 4 ++ > > > > drivers/md/dm-table.c | 52 +++ > > > > drivers/md/dm.c | 92 - > > > > include/linux/device-mapper.h | 6 +++ > > > > include/linux/keyslot-manager.h | 7 +++ > > > > 7 files changed, 195 insertions(+), 1 deletion(-) > > > > > > > > > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h > > > > index c4ef1fceead6..4542050eebfc 100644 > > > > --- a/drivers/md/dm-core.h > > > > +++ b/drivers/md/dm-core.h > > > > @@ -12,6 +12,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > #include > > > > > > > > @@ -49,6 +50,9 @@ struct mapped_device { > > > > > > > > int numa_node_id; > > > > struct request_queue *queue; > > > > +#ifdef CONFIG_BLK_INLINE_ENCRYPTION > > > > + struct blk_keyslot_manager ksm; > > > > +#endif > > > > > > > > atomic_t holders; > > > > atomic_t open_count; > > > > > > Any reason you placed the ksm member where you did? > > > > > > Looking at 'struct blk_keyslot_manager' I'm really hating adding that > > > bloat to every DM device for a feature that really won't see much broad > > > use (AFAIK). > > > > > > Any chance you could allocate 'struct blk_keyslot_manager' as needed so > > > that most users of DM would only be carrying 1 extra pointer (set to > > > NULL)? > > > > I don't think there's any technical problem with doing that - the only > > other thing that would need addressing is that the patch uses > > "container_of" on that blk_keyslot_manager in dm_keyslot_evict() to get > > a pointer to the struct mapped_device. I could try adding a "private" > > field to struct blk_keyslot_manager and store a pointer to the struct > > mapped_device there). > > Yes, that'd be ideal. > > As for the lifetime of the struct blk_keyslot_manager pointer DM would > manage (in your future code revision): you meantioned in one reply that > the request_queue takes care of setting up the ksm... but the ksm > is tied to the queue at a later phase using blk_ksm_register(). > I probably wasn't clear in that reply :(. So the request_queue isn't responsible for setting up the ksm - setting up the ksm in the request queue is the responsibility of the DM device. > In any case, I think my feature reequest (to have DM allocate the ksm > struct only as needed) is a bit challenging because of how DM allocates > the request_queue upfront in alloc_dev() and then later completes the > request_queue initialization based on DM_TYPE* in dm_setup_md_queue(). > > It _could_ be that you'll need to add a new DM_TYPE_KSM_BIO_BASED or > something. But you have a catch-22 in that the dm-table.c code to > establish the intersection of supported modes assumes ksm is already > allocated. So something needs to give by reasoning through: _what_ is > the invariant that will trigger the delayed allocation of the ksm > struct? I don't yet see how you can make that informed decision that > the target(s) in the DM table _will_ use the ksm if it exists. > What I tried doing in the next version that I just sent out was to get the DM device to set up the ksm as appropriate on table swaps (and also to
Re: [GIT PULL] Kunit fixes update for Linux 5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 15:13:02 -0600: > git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest > tags/linux-kselftest-kunit-fixes-5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/578a7155c5a1894a789d4ece181abf9d25dc6b0d Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] HID for 5.10
The pull request you sent on Thu, 15 Oct 2020 20:52:36 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/bf36c6b946c8895cf590f10dbd70b589b0dc101f Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] trivial for 5.10
The pull request you sent on Thu, 15 Oct 2020 20:56:41 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial.git for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/bbf625990371782370f6eacb3155dc1fe131ddfc Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] livepatching for 5.10
The pull request you sent on Thu, 15 Oct 2020 20:31:55 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching > for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/0cd7d9795fa82226e7516d38b474bddae8b1ff26 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] configfs updates for 5.10
The pull request you sent on Thu, 15 Oct 2020 19:54:01 +0200: > git://git.infradead.org/users/hch/configfs.git tags/configfs-5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/ca5387e448e1f88440dc93e143b353592f8a8af6 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH 1/2] arm64: dts: qcom: sc7180: Add gpu cooling support
Hi, On Thu, Oct 15, 2020 at 12:07:01AM +0530, man...@codeaurora.org wrote: > On 2020-10-14 18:59, Akhil P Oommen wrote: > > On 10/9/2020 10:27 PM, Matthias Kaehlcke wrote: > > > On Fri, Oct 09, 2020 at 08:05:10AM -0700, Doug Anderson wrote: > > > > Hi, > > > > > > > > On Thu, Oct 8, 2020 at 10:10 AM Akhil P Oommen > > > > wrote: > > > > > > > > > > Add cooling-cells property and the cooling maps for the gpu tzones > > > > > to support GPU cooling. > > > > > > > > > > Signed-off-by: Akhil P Oommen > > > > > --- > > > > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 29 > > > > > ++--- > > > > > 1 file changed, 22 insertions(+), 7 deletions(-) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > index d46b383..40d6a28 100644 > > > > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > @@ -2,7 +2,7 @@ > > > > > /* > > > > >* SC7180 SoC device tree source > > > > >* > > > > > - * Copyright (c) 2019, The Linux Foundation. All rights reserved. > > > > > + * Copyright (c) 2019-20, The Linux Foundation. All rights > > > > > reserved. > > > > >*/ > > > > > > > > > > #include > > > > > @@ -1885,6 +1885,7 @@ > > > > > iommus = <_smmu 0>; > > > > > operating-points-v2 = <_opp_table>; > > > > > qcom,gmu = <>; > > > > > + #cooling-cells = <2>; > > > > > > > > Presumably we should add this to the devicetree bindings, too? > > Yes, thanks for catching this. Will update in the next patch. > > > > > > > > > > > > > > > interconnects = <_noc > > > > > MASTER_GFX3D _virt SLAVE_EBI1>; > > > > > interconnect-names = "gfx-mem"; > > > > > @@ -3825,16 +3826,16 @@ > > > > > }; > > > > > > > > > > gpuss0-thermal { > > > > > - polling-delay-passive = <0>; > > > > > + polling-delay-passive = <100>; > > > > > > > > Why did you make this change? I'm pretty sure that we _don't_ want > > > > this since we're using interrupts for the thermal sensor. See commit > > > > 22337b91022d ("arm64: dts: qcom: sc7180: Changed polling mode in > > > > Thermal-zones node"). > > > > > > I was going to ask the same, this shouldn't be needed. > As per our understanding unlike "polling-delay", this delay property is > intended to activate polling thread on post trip threshold violation and it > is irrespective of sensor is capable for trip interrupt or not. > This polling is more of governor related. Below are the few references from > Documentation/code which tells polling-delay-passive is needed for IPA for > better IPA performance. > > As per Power allocator documentations > > 1. > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/driver-api/thermal/power_allocator.rst?h=v5.4.71#n264 > > "The power allocator governor's PID controller works best if there is a > periodic tick. If you have a driver that calls > `thermal_zone_device_update()` (or anything that ends up calling the > governor's `throttle()` function) repetitively, the governor response > won't be very good. Note that this is not particular to this > governor, step-wise will also misbehave if you call its throttle() > faster than the normal thermal framework tick (due to interrupts for > example) as it will overreact" > > 2. In Power allocator code, when switch_on/control trip temp violation, it > is enabling passive counter to activate passive polling @ > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/thermal/power_allocator.c?h=v5.4.71#n634 > > 3. while calculating derivative term, it is using passive_delay @ > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/thermal/power_allocator.c?h=v5.4.71#n243 > > 4. Sensor interrupt will work if temperature is fluctuating between > trip_temp and hysteresis. But say a case where we are not enabling > polling-delay-passive. In this case if current temperature > control_temp > trip(2nd passive trip) and > temperature trend is still raising, then sensor high trip will be disabled > (OR configured for critical trip threshold). No more trip interrupt from > sensor until it reaches critical trip or falls below control_temp > hysteresis. > How the governor re-evaluate its next mitigation without passive polling > thread here ? > > I think the same is required for CPU thermal zone as well. Thanks for the explication and pointers! I ran some tests to re-confirm. For that I lowered the trip point temperatures of CPU6 to 60/70, to make it easier to trigger throttling without necessarily affecting the other CPUs. Further I enabled tracing for the events 'thermal_temperature', 'thermal_zone_trip' and 'thermal_power_allocator'. With that I ran a
Re: [GIT PULL] dma-mapping updates for 5.10
The pull request you sent on Thu, 15 Oct 2020 19:47:43 +0200: > git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/5a32c3413d3340f90c82c84b375ad4b335a59f28 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] dmaengine updates for v5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 12:36:22 +0530: > git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git > tags/dmaengine-5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/f065199d4df0b1512f935621d2de128ddb3fcc3a Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] Kselftest next update for Linux 5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 13:55:07 -0600: > git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest > tags/linux-kselftest-next-5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/0674324b16d40e14b9d8ea2d667627c010608c28 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v2] net: usb: rtl8150: don't incorrectly assign random MAC addresses
On Fri, 16 Oct 2020 08:59:22 +1100 Stephen Rothwell wrote: > > I will apply the above patch to the merge of the usb tree today to fix > > up a semantic conflict between the usb tree and Linus' tree. > > It looks like you forgot to mention this one to Linus :-( > > It should probably say: > > Fixes: b2a0f274e3f7 ("net: rtl8150: Use the new usb control message API.") Umpf. I think Greg sent his changes first, so this is on me. The networking PR is still outstanding, can we reply to the PR with the merge guidance. Or is it too late?
[PATCH 2/2] tools/include: Add rtnetlink.h, id_addr.h and neighbour.h
tools/lib/bpf/netlink.c depends on rtnetlink.h and netlink.h (via nlattr.h). Older versions of rtnetlink.h and netlink.h can cause duplicate conflicting definitions to occur, as things like header guards don't agree. To avoid these mismatches add rtnetlink.h, if_addr.h and neighbour.h to tools/include with the standard difference check. Signed-off-by: Ian Rogers --- tools/include/uapi/linux/if_addr.h | 72 +++ tools/include/uapi/linux/neighbour.h | 199 +++ tools/include/uapi/linux/rtnetlink.h | 787 +++ tools/lib/bpf/Makefile | 9 + 4 files changed, 1067 insertions(+) create mode 100644 tools/include/uapi/linux/if_addr.h create mode 100644 tools/include/uapi/linux/neighbour.h create mode 100644 tools/include/uapi/linux/rtnetlink.h diff --git a/tools/include/uapi/linux/if_addr.h b/tools/include/uapi/linux/if_addr.h new file mode 100644 index ..dfcf3ce0097f --- /dev/null +++ b/tools/include/uapi/linux/if_addr.h @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __LINUX_IF_ADDR_H +#define __LINUX_IF_ADDR_H + +#include +#include + +struct ifaddrmsg { + __u8ifa_family; + __u8ifa_prefixlen; /* The prefix length*/ + __u8ifa_flags; /* Flags*/ + __u8ifa_scope; /* Address scope*/ + __u32 ifa_index; /* Link index */ +}; + +/* + * Important comment: + * IFA_ADDRESS is prefix address, rather than local interface address. + * It makes no difference for normally configured broadcast interfaces, + * but for point-to-point IFA_ADDRESS is DESTINATION address, + * local address is supplied in IFA_LOCAL attribute. + * + * IFA_FLAGS is a u32 attribute that extends the u8 field ifa_flags. + * If present, the value from struct ifaddrmsg will be ignored. + */ +enum { + IFA_UNSPEC, + IFA_ADDRESS, + IFA_LOCAL, + IFA_LABEL, + IFA_BROADCAST, + IFA_ANYCAST, + IFA_CACHEINFO, + IFA_MULTICAST, + IFA_FLAGS, + IFA_RT_PRIORITY, /* u32, priority/metric for prefix route */ + IFA_TARGET_NETNSID, + __IFA_MAX, +}; + +#define IFA_MAX (__IFA_MAX - 1) + +/* ifa_flags */ +#define IFA_F_SECONDARY0x01 +#define IFA_F_TEMPORARYIFA_F_SECONDARY + +#defineIFA_F_NODAD 0x02 +#define IFA_F_OPTIMISTIC 0x04 +#define IFA_F_DADFAILED0x08 +#defineIFA_F_HOMEADDRESS 0x10 +#define IFA_F_DEPRECATED 0x20 +#define IFA_F_TENTATIVE0x40 +#define IFA_F_PERMANENT0x80 +#define IFA_F_MANAGETEMPADDR 0x100 +#define IFA_F_NOPREFIXROUTE0x200 +#define IFA_F_MCAUTOJOIN 0x400 +#define IFA_F_STABLE_PRIVACY 0x800 + +struct ifa_cacheinfo { + __u32 ifa_prefered; + __u32 ifa_valid; + __u32 cstamp; /* created timestamp, hundredths of seconds */ + __u32 tstamp; /* updated timestamp, hundredths of seconds */ +}; + +/* backwards compatibility for userspace */ +#ifndef __KERNEL__ +#define IFA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ifaddrmsg +#define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg)) +#endif + +#endif diff --git a/tools/include/uapi/linux/neighbour.h b/tools/include/uapi/linux/neighbour.h new file mode 100644 index ..dc8b72201f6c --- /dev/null +++ b/tools/include/uapi/linux/neighbour.h @@ -0,0 +1,199 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __LINUX_NEIGHBOUR_H +#define __LINUX_NEIGHBOUR_H + +#include +#include + +struct ndmsg { + __u8ndm_family; + __u8ndm_pad1; + __u16 ndm_pad2; + __s32 ndm_ifindex; + __u16 ndm_state; + __u8ndm_flags; + __u8ndm_type; +}; + +enum { + NDA_UNSPEC, + NDA_DST, + NDA_LLADDR, + NDA_CACHEINFO, + NDA_PROBES, + NDA_VLAN, + NDA_PORT, + NDA_VNI, + NDA_IFINDEX, + NDA_MASTER, + NDA_LINK_NETNSID, + NDA_SRC_VNI, + NDA_PROTOCOL, /* Originator of entry */ + NDA_NH_ID, + NDA_FDB_EXT_ATTRS, + __NDA_MAX +}; + +#define NDA_MAX (__NDA_MAX - 1) + +/* + * Neighbor Cache Entry Flags + */ + +#define NTF_USE0x01 +#define NTF_SELF 0x02 +#define NTF_MASTER 0x04 +#define NTF_PROXY 0x08/* == ATF_PUBL */ +#define NTF_EXT_LEARNED0x10 +#define NTF_OFFLOADED 0x20 +#define NTF_STICKY 0x40 +#define NTF_ROUTER 0x80 + +/* + * Neighbor Cache Entry States. + */ + +#define NUD_INCOMPLETE 0x01 +#define NUD_REACHABLE 0x02 +#define NUD_STALE 0x04 +#define NUD_DELAY 0x08 +#define NUD_PROBE 0x10 +#define NUD_FAILED 0x20 + +/* Dummy states */ +#define NUD_NOARP 0x40 +#define NUD_PERMANENT
Re: [PATCH v8 3/3] leds: trigger: implement a tty trigger
Hi "Uwe, I love your patch! Perhaps something to improve: [auto build test WARNING on tty/tty-testing] [also build test WARNING on pavel-linux-leds/for-next linus/master j.anaszewski-leds/for-next v5.9 next-20201015] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Uwe-Kleine-K-nig/leds-trigger-implement-a-tty-trigger/20201012-203510 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: openrisc-randconfig-s031-20201015 (attached as .config) compiler: or1k-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 # apt-get install sparse # sparse version: v0.6.3-rc1-dirty # https://github.com/0day-ci/linux/commit/4185c273a8de0340cee6655a363f98c3737665d0 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Uwe-Kleine-K-nig/leds-trigger-implement-a-tty-trigger/20201012-203510 git checkout 4185c273a8de0340cee6655a363f98c3737665d0 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot "sparse warnings: (new ones prefixed by >>)" >> drivers/leds/trigger/ledtrig-tty.c:177:20: sparse: sparse: symbol >> 'ledtrig_tty' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH 1/2] tools/include: Update if_link.h and netlink.h
These are tested to be the latest as part of the tools/lib/bpf build. Signed-off-by: Ian Rogers --- tools/include/uapi/linux/if_link.h | 269 + tools/include/uapi/linux/netlink.h | 107 2 files changed, 342 insertions(+), 34 deletions(-) diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h index 781e482dc499..c4b23f06f69e 100644 --- a/tools/include/uapi/linux/if_link.h +++ b/tools/include/uapi/linux/if_link.h @@ -7,24 +7,23 @@ /* This struct should be in sync with struct rtnl_link_stats64 */ struct rtnl_link_stats { - __u32 rx_packets; /* total packets received */ - __u32 tx_packets; /* total packets transmitted*/ - __u32 rx_bytes; /* total bytes received */ - __u32 tx_bytes; /* total bytes transmitted */ - __u32 rx_errors; /* bad packets received */ - __u32 tx_errors; /* packet transmit problems */ - __u32 rx_dropped; /* no space in linux buffers*/ - __u32 tx_dropped; /* no space available in linux */ - __u32 multicast; /* multicast packets received */ + __u32 rx_packets; + __u32 tx_packets; + __u32 rx_bytes; + __u32 tx_bytes; + __u32 rx_errors; + __u32 tx_errors; + __u32 rx_dropped; + __u32 tx_dropped; + __u32 multicast; __u32 collisions; - /* detailed rx_errors: */ __u32 rx_length_errors; - __u32 rx_over_errors; /* receiver ring buff overflow */ - __u32 rx_crc_errors; /* recved pkt with crc error*/ - __u32 rx_frame_errors;/* recv'd frame alignment error */ - __u32 rx_fifo_errors; /* recv'r fifo overrun */ - __u32 rx_missed_errors; /* receiver missed packet */ + __u32 rx_over_errors; + __u32 rx_crc_errors; + __u32 rx_frame_errors; + __u32 rx_fifo_errors; + __u32 rx_missed_errors; /* detailed tx_errors */ __u32 tx_aborted_errors; @@ -37,29 +36,200 @@ struct rtnl_link_stats { __u32 rx_compressed; __u32 tx_compressed; - __u32 rx_nohandler; /* dropped, no handler found*/ + __u32 rx_nohandler; }; -/* The main device statistics structure */ +/** + * struct rtnl_link_stats64 - The main device statistics structure. + * + * @rx_packets: Number of good packets received by the interface. + * For hardware interfaces counts all good packets received from the device + * by the host, including packets which host had to drop at various stages + * of processing (even in the driver). + * + * @tx_packets: Number of packets successfully transmitted. + * For hardware interfaces counts packets which host was able to successfully + * hand over to the device, which does not necessarily mean that packets + * had been successfully transmitted out of the device, only that device + * acknowledged it copied them out of host memory. + * + * @rx_bytes: Number of good received bytes, corresponding to @rx_packets. + * + * For IEEE 802.3 devices should count the length of Ethernet Frames + * excluding the FCS. + * + * @tx_bytes: Number of good transmitted bytes, corresponding to @tx_packets. + * + * For IEEE 802.3 devices should count the length of Ethernet Frames + * excluding the FCS. + * + * @rx_errors: Total number of bad packets received on this network device. + * This counter must include events counted by @rx_length_errors, + * @rx_crc_errors, @rx_frame_errors and other errors not otherwise + * counted. + * + * @tx_errors: Total number of transmit problems. + * This counter must include events counter by @tx_aborted_errors, + * @tx_carrier_errors, @tx_fifo_errors, @tx_heartbeat_errors, + * @tx_window_errors and other errors not otherwise counted. + * + * @rx_dropped: Number of packets received but not processed, + * e.g. due to lack of resources or unsupported protocol. + * For hardware interfaces this counter should not include packets + * dropped by the device which are counted separately in + * @rx_missed_errors (since procfs folds those two counters together). + * + * @tx_dropped: Number of packets dropped on their way to transmission, + * e.g. due to lack of resources. + * + * @multicast: Multicast packets received. + * For hardware interfaces this statistic is commonly calculated + * at the device level (unlike @rx_packets) and therefore may include + * packets which did not reach the host. + * + * For IEEE 802.3 devices this counter may be equivalent to: + * + *- 30.3.1.1.21 aMulticastFramesReceivedOK + * + * @collisions: Number of collisions during packet transmissions. + * + * @rx_length_errors: Number of packets dropped due to invalid length. + *
[RFC PATCH] leds: trigger: ledtrig_tty can be static
Signed-off-by: kernel test robot --- ledtrig-tty.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c index 806548e33cd874..09cba818fb65c7 100644 --- a/drivers/leds/trigger/ledtrig-tty.c +++ b/drivers/leds/trigger/ledtrig-tty.c @@ -174,7 +174,7 @@ static void ledtrig_tty_deactivate(struct led_classdev *led_cdev) kfree(trigger_data); } -struct led_trigger ledtrig_tty = { +static struct led_trigger ledtrig_tty = { .name = "tty", .activate = ledtrig_tty_activate, .deactivate = ledtrig_tty_deactivate,
Re: [PATCH] [PATCH] [v3] wireless: Initial driver submission for pureLiFi STA devices
On Wed, 2020-10-14 at 11:49 +0530, Srinivasan Raju wrote: > This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC > and LiFi-XL USB devices. trivia: netdev_ might be better than dev_. > diff --git a/drivers/net/wireless/purelifi/chip.c > b/drivers/net/wireless/purelifi/chip.c [] > +int purelifi_chip_switch_radio(struct purelifi_chip *chip, u32 value) > +{ > + int r; > + > + r = usb_write_req((const u8 *), sizeof(value), USB_REQ_POWER_WR); > + if (r) > + dev_err(purelifi_chip_dev(chip), "%s r=%d", __func__, r); missing '\n' termination. [] > > +int purelifi_chip_set_rate(struct purelifi_chip *chip, u8 rate) > +{ > + int r; > + static struct purelifi_chip *chip_p; > + > + if (chip) > + chip_p = chip; > + if (!chip_p) > + return -EINVAL; > + > + r = usb_write_req((const u8 *), sizeof(rate), USB_REQ_RATE_WR); > + if (r) > + dev_err(purelifi_chip_dev(chip), "set rate failed r=%d", r); same missing newline, etc It might also be better to use a consistent error message like dev_err(purelifi_chip_dev(chip), "%s: failed, r=%d\n", r); for both. > diff --git a/drivers/net/wireless/purelifi/mac.c > b/drivers/net/wireless/purelifi/mac.c [] > +static const struct ieee80211_rate purelifi_rates[] = { > + { .bitrate = 10, > + .hw_value = PURELIFI_CCK_RATE_1M, }, [] > + { .bitrate = 60, > + .hw_value = PURELIFI_OFDM_RATE_6M, > + .flags = 0 }, Seems odd to set .flags to 0 for only some of the entries. Perhaps more readable to always leave it off if unset. > + { .bitrate = 90, > + .hw_value = PURELIFI_OFDM_RATE_9M, > + .flags = 0 }, > + { .bitrate = 120, > + .hw_value = PURELIFI_OFDM_RATE_12M, > + .flags = 0 }, > + { .bitrate = 180, > + .hw_value = PURELIFI_OFDM_RATE_18M, > + .flags = 0 }, > + { .bitrate = 240, > + .hw_value = PURELIFI_OFDM_RATE_24M, > + .flags = 0 }, > + { .bitrate = 360, > + .hw_value = PURELIFI_OFDM_RATE_36M, > + .flags = 0 }, > + { .bitrate = 480, > + .hw_value = PURELIFI_OFDM_RATE_48M, > + .flags = 0 }, > + { .bitrate = 540, > + .hw_value = PURELIFI_OFDM_RATE_54M, > + .flags = 0 }, > +}; [] > +int purelifi_mac_init_hw(struct ieee80211_hw *hw) > +{ > + int r; > + struct purelifi_mac *mac = purelifi_hw_mac(hw); > + struct purelifi_chip *chip = >chip; > + > + r = purelifi_chip_init_hw(chip); > + if (r) { > + dev_warn(purelifi_mac_dev(mac), "init hw failed. (%d)\n", r); > + goto out; > + } > + > + dev_dbg(purelifi_mac_dev(mac), "irq_disabled %d", irqs_disabled()); missing newline > +static int filter_ack(struct ieee80211_hw *hw, struct ieee80211_hdr *rx_hdr, > + struct ieee80211_rx_status *stats) > +{ > + struct purelifi_mac *mac = purelifi_hw_mac(hw); > + struct sk_buff *skb; > + struct sk_buff_head *q; > + unsigned long flags; > + int found = 0; > + int i, position = 0; > + > + if (!ieee80211_is_ack(rx_hdr->frame_control)) > + return 0; > + dev_info(purelifi_mac_dev(mac), "%s::ACK Received", __func__); missing newline > +static int purelifi_op_add_interface(struct ieee80211_hw *hw, > + struct ieee80211_vif *vif) > +{ > + struct purelifi_mac *mac = purelifi_hw_mac(hw); > + static const char * const iftype80211[] = { > + [NL80211_IFTYPE_STATION]= "Station", > + [NL80211_IFTYPE_ADHOC] = "Adhoc" > + }; > + > + if (mac->type != NL80211_IFTYPE_UNSPECIFIED) > + return -EOPNOTSUPP; > + > + if (vif->type == NL80211_IFTYPE_ADHOC || > + vif->type == NL80211_IFTYPE_STATION) { > + dev_info(purelifi_mac_dev(mac), "%s %s\n", __func__, > + iftype80211[vif->type]); > + mac->type = vif->type; > + mac->vif = vif; > + } else { > + dev_info(purelifi_mac_dev(mac), "unsupported iftype"); etc...
Re: [PATCH v2] net: usb: rtl8150: don't incorrectly assign random MAC addresses
On Thu, 15 Oct 2020 15:24:51 -0700 Jakub Kicinski wrote: > On Fri, 16 Oct 2020 08:59:22 +1100 Stephen Rothwell wrote: > > > I will apply the above patch to the merge of the usb tree today to fix > > > up a semantic conflict between the usb tree and Linus' tree. > > > > It looks like you forgot to mention this one to Linus :-( > > > > It should probably say: > > > > Fixes: b2a0f274e3f7 ("net: rtl8150: Use the new usb control message API.") > > Umpf. I think Greg sent his changes first, so this is on me. > > The networking PR is still outstanding, can we reply to the PR with > the merge guidance. Or is it too late? I take that back, this came through net, not net-next so our current PR is irrelevant :)
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
On Thu, Oct 15, 2020 at 2:44 PM Jerry Snitselaar wrote: > > There is a misconfiguration in the bios of the gpio pin used for the > interrupt in the T490s. When interrupts are enabled in the tpm_tis > driver code this results in an interrupt storm. This was initially > reported when we attempted to enable the interrupt code in the tpm_tis > driver, which previously wasn't setting a flag to enable it. Due to > the reports of the interrupt storm that code was reverted and we went back > to polling instead of using interrupts. Now that we know the T490s problem > is a firmware issue, add code to check if the system is a T490s and > disable interrupts if that is the case. This will allow us to enable > interrupts for everyone else. If the user has a fixed bios they can > force the enabling of interrupts with tpm_tis.interrupts=1 on the > kernel command line. I think an implication of this is that systems haven't been well-tested with interrupts enabled. In general when we've found a firmware issue in one place it ends up happening elsewhere as well, so it wouldn't surprise me if there are other machines that will also be unhappy with interrupts enabled. Would it be possible to automatically detect this case (eg, if we get more than a certain number of interrupts in a certain timeframe immediately after enabling the interrupt) and automatically fall back to polling in that case? It would also mean that users with fixed firmware wouldn't need to pass a parameter.
[PATCH] docs: lkdtm: Modernize and improve details
The details on using LKDTM were overly obscure. Modernize the details and expand examples to better illustrate how to use the interfaces. Additionally add missing SPDX header. Signed-off-by: Kees Cook --- .../fault-injection/provoke-crashes.rst | 56 +++ 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/Documentation/fault-injection/provoke-crashes.rst b/Documentation/fault-injection/provoke-crashes.rst index 9279a3e12278..93775bd4e6c8 100644 --- a/Documentation/fault-injection/provoke-crashes.rst +++ b/Documentation/fault-injection/provoke-crashes.rst @@ -1,16 +1,19 @@ -=== -Provoke crashes -=== +.. SPDX-License-Identifier: GPL-2.0 -The lkdtm module provides an interface to crash or injure the kernel at -predefined crashpoints to evaluate the reliability of crash dumps obtained -using different dumping solutions. The module uses KPROBEs to instrument -crashing points, but can also crash the kernel directly without KRPOBE -support. + +Provoking crashes with Linux Kernel Dump Test Module (LKDTM) + +The lkdtm module provides an interface to disrupt (and usually crash) +the kernel at predefined code locations to evaluate the reliability of +the kernel's exception handling and to test crash dumps obtained using +different dumping solutions. The module uses KPROBEs to instrument the +trigger location, but can also trigger the kernel directly without KPROBE +support via debugfs. -You can provide the way either through module arguments when inserting -the module, or through a debugfs interface. +You can select the location of the trigger ("crash point name") and the +type of action ("crash point type") either through module arguments when +inserting the module, or through the debugfs interface. Usage:: @@ -18,31 +21,38 @@ Usage:: [cpoint_count={>0}] recur_count - Recursion level for the stack overflow test. Default is 10. + Recursion level for the stack overflow test. By default this is + dynamically calculated based on kernel configuration, with the + goal of being just large enough to exhaust the kernel stack. The + value can be seen at `/sys/module/lkdtm/parameters/recur_count`. cpoint_name - Crash point where the kernel is to be crashed. It can be + Where in the kernel to trigger the action. It can be one of INT_HARDWARE_ENTRY, INT_HW_IRQ_EN, INT_TASKLET_ENTRY, FS_DEVRW, MEM_SWAPOUT, TIMERADD, SCSI_DISPATCH_CMD, - IDE_CORE_CP, DIRECT + IDE_CORE_CP, or DIRECT cpoint_type Indicates the action to be taken on hitting the crash point. - It can be one of PANIC, BUG, EXCEPTION, LOOP, OVERFLOW, - CORRUPT_STACK, UNALIGNED_LOAD_STORE_WRITE, OVERWRITE_ALLOCATION, - WRITE_AFTER_FREE, + These are numerous, and best queried directly from debugfs. Some + of the common ones are PANIC, BUG, EXCEPTION, LOOP, and OVERFLOW. + See the contents of `/sys/kernel/debug/provoke-crash/DIRECT` for + a complete list. cpoint_count Indicates the number of times the crash point is to be hit - to trigger an action. The default is 10. + before triggering the action. The default is 10 (except for + DIRECT, which always fires immediately). You can also induce failures by mounting debugfs and writing the type to -/provoke-crash/. E.g.:: +/provoke-crash/. E.g.:: - mount -t debugfs debugfs /mnt - echo EXCEPTION > /mnt/provoke-crash/INT_HARDWARE_ENTRY + mount -t debugfs debugfs /sys/kernel/debug + echo EXCEPTION > /sys/kernel/debug/provoke-crash/INT_HARDWARE_ENTRY +The special file `DIRECT` will induce the action directly without KPROBE +instrumentation. This mode is the only one available when the module is +built for a kernel without KPROBEs support:: -A special file is `DIRECT` which will induce the crash directly without -KPROBE instrumentation. This mode is the only one available when the module -is built on a kernel without KPROBEs support. + # Instead of having a BUG kill your shell, have it kill "cat": + cat <(echo WRITE_RO) >/sys/kernel/debug/provoke-crash/DIRECT -- 2.25.1
Re: [PATCH] PCI: dwc: Added link up check in map_bus of dw_child_pcie_ops
On Wed, Sep 16, 2020 at 01:41:30PM +0800, Zhiqiang Hou wrote: > From: Hou Zhiqiang > > On NXP Layerscape platforms, it results in SError in the > enumeration of the PCIe controller, which is not connecting > with an Endpoint device. And it doesn't make sense to > enumerate the Endpoints when the PCIe link is down. So this > patch added the link up check to avoid to fire configuration > transactions on link down bus. Lorenzo already applied this, but a couple questions: You call out NXP Layerscape specifically, but doesn't this affect other DWC-based platforms, too? You later mentioned imx6, Kishon mentioned dra7xx, Michael mentioned ls1028a, Naresh mentioned ls2088 (probably both the same as your "NXP Layerscape"). The backtrace below contains a bunch of irrelevant info. The timestamps are pointless. The backtrace past pci_scan_single_device+0x80/0x100 or so really doesn't add anything either. It'd be nice to have a comment in the code because the code *looks* wrong and racy. Without a hint, everybody who sees it will have to dig through the history to see why we tolerate the race. > [0.807773] SError Interrupt on CPU2, code 0xbf02 -- SError > [0.807775] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > 5.9.0-rc5-next-20200914-1-gf965d3ec86fa #67 > [0.807776] Hardware name: LS1046A RDB Board (DT) > [0.80] pstate: 2085 (nzCv daIf -PAN -UAO BTYPE=--) > [0.807778] pc : pci_generic_config_read+0x3c/0xe0 > [0.807778] lr : pci_generic_config_read+0x24/0xe0 > [0.807779] sp : 80001003b7b0 > [0.807780] x29: 80001003b7b0 x28: 80001003ba74 > [0.807782] x27: 000971d96800 x26: 00096e77e0a8 > [0.807784] x25: 80001003b874 x24: 80001003b924 > [0.807786] x23: 0004 x22: > [0.807788] x21: x20: 80001003b874 > [0.807790] x19: 0004 x18: > [0.807791] x17: 00c0 x16: fe0025981840 > [0.807793] x15: b94c75b69948 x14: 62203a383634203a > [0.807795] x13: 666e6f635f726568 x12: 202c31203d207265 > [0.807797] x11: 626d756e3e2d7375 x10: 656877202c307830 > [0.807799] x9 : 203d206e66766564 x8 : 0908 > [0.807801] x7 : 0908 x6 : 80001090 > [0.807802] x5 : 00096e77e080 x4 : > [0.807804] x3 : 0003 x2 : 84fa3440ff7e7000 > [0.807806] x1 : x0 : 800010034000 > [0.807808] Kernel panic - not syncing: Asynchronous SError Interrupt > [0.807809] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > 5.9.0-rc5-next-20200914-1-gf965d3ec86fa #67 > [0.807810] Hardware name: LS1046A RDB Board (DT) > [0.807811] Call trace: > [0.807812] dump_backtrace+0x0/0x1c0 > [0.807813] show_stack+0x18/0x28 > [0.807814] dump_stack+0xd8/0x134 > [0.807814] panic+0x180/0x398 > [0.807815] add_taint+0x0/0xb0 > [0.807816] arm64_serror_panic+0x78/0x88 > [0.807817] do_serror+0x68/0x180 > [0.807818] el1_error+0x84/0x100 > [0.807818] pci_generic_config_read+0x3c/0xe0 > [0.807819] dw_pcie_rd_other_conf+0x78/0x110 > [0.807820] pci_bus_read_config_dword+0x88/0xe8 > [0.807821] pci_bus_generic_read_dev_vendor_id+0x30/0x1b0 > [0.807822] pci_bus_read_dev_vendor_id+0x4c/0x78 > [0.807823] pci_scan_single_device+0x80/0x100 > [0.807824] pci_scan_slot+0x38/0x130 > [0.807825] pci_scan_child_bus_extend+0x54/0x2a0 > [0.807826] pci_scan_child_bus+0x14/0x20 > [0.807827] pci_scan_bridge_extend+0x230/0x570 > [0.807828] pci_scan_child_bus_extend+0x134/0x2a0 > [0.807829] pci_scan_root_bus_bridge+0x64/0xf0 > [0.807829] pci_host_probe+0x18/0xc8 > [0.807830] dw_pcie_host_init+0x220/0x378 > [0.807831] ls_pcie_probe+0x104/0x140 > [0.807832] platform_drv_probe+0x54/0xa8 > [0.807833] really_probe+0x118/0x3e0 > [0.807834] driver_probe_device+0x5c/0xc0 > [0.807835] device_driver_attach+0x74/0x80 > [0.807835] __driver_attach+0x8c/0xd8 > [0.807836] bus_for_each_dev+0x7c/0xd8 > [0.807837] driver_attach+0x24/0x30 > [0.807838] bus_add_driver+0x154/0x200 > [0.807839] driver_register+0x64/0x120 > [0.807839] __platform_driver_probe+0x7c/0x148 > [0.807840] ls_pcie_driver_init+0x24/0x30 > [0.807841] do_one_initcall+0x60/0x1d8 > [0.807842] kernel_init_freeable+0x1f4/0x24c > [0.807843] kernel_init+0x14/0x118 > [0.807843] ret_from_fork+0x10/0x34 > [0.807854] SMP: stopping secondary CPUs > [0.807855] Kernel Offset: 0x394c6408 from 0x80001000 > [0.807856] PHYS_OFFSET: 0x8bfd4000 > [0.807856] CPU features: 0x0240022,21806000 > [0.807857] Memory Limit: none > > Fixes: c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") > Signed-off-by: Hou Zhiqiang > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On 14:10 Thu 15 Oct 2020, Joe Perches wrote: On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > Fix the space in the middle in below entry. > > > > memry||memory > [] > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > [] > > @@ -885,7 +885,7 @@ meetign||meeting > > memeory||memory > > memmber||member > > memoery||memory > > -memry ||memory > > +memry||memory > > No. Don't post a bad patch, assume > it's applied and then post a fix to > the bad patch as v2. > > Send a single clean patch. > Not sure what you mean...could you elaborate...don't know what is going on..> You sent a patch with a defect Who doesn't??? You sent a V2 patch that just corrects the defect in the first patch. That's how it is working here for long time ...I am not sure about your involvement. Instead send a v3 patch without any defect. No point ...I think your understanding takes a back seat...could you please rebrush it?? ..and you are telling me something which isn't practice here ..maybe some other project with some other people you worked with... certainly not here ... Long story short, you found a flaw what I sent and I appreciate your review and corrected the trivialities ...but rest what you are asking is garbage . More ...I don't know why I am explaining this stuff to you...we have been here doing this stuff for a long time ... (not sure about you) Versioning is important for the person who maintain that file..because of lots of reason ...and I am sure either you have forgotten or it skipped your mind for the moment ..that is why I suggest ...please rebrush your understanding ... Please don't unnecessarily streach thing ...we have other things to do ...do not bringing "new rules" here for the sake of it. signature.asc Description: PGP signature
Re: [PATCH v11 1/4] bitops: Introduce the for_each_set_clump macro
On Tue, Oct 6, 2020 at 4:56 PM Andy Shevchenko wrote: > > On Tue, Oct 06, 2020 at 02:52:16PM +0530, Syed Nayyar Waris wrote: > > This macro iterates for each group of bits (clump) with set bits, > > within a bitmap memory region. For each iteration, "start" is set to > > the bit offset of the found clump, while the respective clump value is > > stored to the location pointed by "clump". Additionally, the > > bitmap_get_value() and bitmap_set_value() functions are introduced to > > respectively get and set a value of n-bits in a bitmap memory region. > > The n-bits can have any size less than or equal to BITS_PER_LONG. > > Moreover, during setting value of n-bit in bitmap, if a situation arise > > that the width of next n-bit is exceeding the word boundary, then it > > will divide itself such that some portion of it is stored in that word, > > while the remaining portion is stored in the next higher word. Similar > > situation occurs while retrieving the value from bitmap. > > ... > > > @@ -75,7 +75,11 @@ > > * bitmap_from_arr32(dst, buf, nbits) Copy nbits from u32[] buf > > to dst > > * bitmap_to_arr32(buf, src, nbits)Copy nbits from buf to > > u32[] dst > > * bitmap_get_value8(map, start) Get 8bit value from map at > > start > > + * bitmap_get_value(map, start, nbits) Get bit value of size > > + * 'nbits' from map at start > > * bitmap_set_value8(map, value, start)Set 8bit value to map at > > start > > + * bitmap_set_value(map, value, start, nbits) Set bit value of size > > 'nbits' > > + * of map at start > > Formatting here is done with solely spaces, no TABs. Okay. Done > > ... > > > +/** > > + * bitmap_get_value - get a value of n-bits from the memory region > > + * @map: address to the bitmap memory region > > + * @start: bit offset of the n-bit value > > + * @nbits: size of value in bits (must be between 1 and BITS_PER_LONG > > inclusive). > > > > + * nbits less than 1 or more than BITS_PER_LONG causes undefined > > behaviour. > > Please, detach this from field description and move to a main description. Okay. Done. > > > + * > > + * Returns value of nbits located at the @start bit offset within the @map > > + * memory region. > > + */ > > ... > > > + return (map[index] >> offset) & GENMASK(nbits - 1, 0); > > Have you considered to use rather BIT{_ULL}(nbits) - 1? > It maybe better for code generation. Yes I have considered using BIT{_ULL} in earlier versions of patchset. It has a problem: This macro when used in both bitmap_get_value and bitmap_set_value functions, it will give unexpected results when nbits or clump size is BITS_PER_LONG (32 or 64 depending on arch). Actually when nbits (clump size) is 64 (BITS_PER_LONG is 64, for example), (BIT(nbits) - 1) gives a value of zero and when this zero is ANDed with any value, it makes it full zero. This is unexpected, and incorrect calculation occurs. What actually happens is in the macro expansion of BIT(64), that is 1 << 64, the '1' overflows from leftmost bit position (most significant bit) and re-enters at the rightmost bit position (least significant bit), therefore 1 << 64 becomes '0x1', and when another '1' is subtracted from this, the final result becomes 0. This is undefined behavior in the C standard (section 6.5.7 in the N1124) > > ... > > > +/** > > + * bitmap_set_value - set n-bit value within a memory region > > + * @map: address to the bitmap memory region > > + * @value: value of nbits > > + * @start: bit offset of the n-bit value > > + * @nbits: size of value in bits (must be between 1 and BITS_PER_LONG > > inclusive). > > > + * nbits less than 1 or more than BITS_PER_LONG causes undefined > > behaviour. > > Please, detach this from field description and move to a main description. Okay. Done > > > + */ > > ... > > > + value &= GENMASK(nbits - 1, 0); > > Ditto. > > > + map[index] &= ~(GENMASK(nbits + offset - 1, offset)); > > Last time I checked such GENMASK) use, it gave a lot of code when > GENMASK(nbits - 1, 0) << offset works much better, but see also above. Yes I have incorporated your suggestion to use the '<<' operator. Thank You. > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On Fri, 2020-10-16 at 04:19 +0530, Bhaskar Chowdhury wrote: > On 14:10 Thu 15 Oct 2020, Joe Perches wrote: > > On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: > > > On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > > > > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > > > > Fix the space in the middle in below entry. > > > > > > > > > > memry||memory > > > > [] > > > > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > > > > [] > > > > > @@ -885,7 +885,7 @@ meetign||meeting > > > > > memeory||memory > > > > > memmber||member > > > > > memoery||memory > > > > > -memry ||memory > > > > > +memry||memory > > > > > > > > No. Don't post a bad patch, assume > > > > it's applied and then post a fix to > > > > the bad patch as v2. > > > > > > > > Send a single clean patch. > > > > > > > > > > Not sure what you mean...could you elaborate...don't know what is going > > > on..> > > > > You sent a patch with a defect > > Who doesn't??? No one. > > You sent a V2 patch that just corrects the defect in the first patch. > > That's how it is working here for long time ...I am not sure about your > involvement. wrong. Your first patch has not been, and should not be applied, as it has a trivial known defect. > Please don't unnecessarily streach thing ...we have other things to do ...do > not bringing "new rules" here for the sake of it. It's not a new rule. Don't introduce patches with defects.
[GIT PULL] f2fs update for 5.10-rc1
Hi Linus, Could you please consider this pull request? Thanks, The following changes since commit 581cb3a26baf846ee9636214afaa5333919875b1: Merge tag 'f2fs-for-5.9-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs (2020-09-10 13:12:46 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git tags/f2fs-for-5.10-rc1 for you to fetch changes up to 788e96d1d39949fc91457a816f4bda0d374c257b: f2fs: code cleanup by removing unnecessary check (2020-10-14 13:23:41 -0700) f2fs-for-5.10-rc1 In this round, we've added new features such as zone capacity for ZNS and a new GC policy, ATGC, along with in-memory segment management. In addition, we could improve the decompression speed significantly by changing virtual mapping method. Even though we've fixed lots of small bugs in compression support, I feel that it becomes more stable so that I could give it a try in production. Enhancement: - suport zone capacity in NVMe Zoned Namespace devices - introduce in-memory current segment management - add standart casefolding support - support age threshold based garbage collection - improve decompression speed by changing virtual mapping method Bug fix: - fix condition checks in some ioctl() such as compression, move_range, etc - fix 32/64bits support in data structures - fix memory allocation in zstd decompress - add some boundary checks to avoid kernel panic on corrupted image - fix disallowing compression for non-empty file - fix slab leakage of compressed block writes In addition, it includes code refactoring for better readability and minor bug fixes for compression and zoned device support. Aravind Ramesh (1): f2fs: support zone capacity less than zone size Chao Yu (24): f2fs: compress: remove unneeded code f2fs: introduce inmem curseg f2fs: record average update time of segment f2fs: inherit mtime of original block during GC f2fs: support 64-bits key in f2fs rb-tree node entry f2fs: fix compile warning f2fs: compress: use more readable atomic_t type for {cic,dic}.ref f2fs: support age threshold based garbage collection f2fs: allocate proper size memory for zstd decompress f2fs: ignore compress mount option on image w/o compression feature f2fs: trace: fix typo f2fs: clean up kvfree f2fs: do sanity check on zoned block device path f2fs: relocate blkzoned feature check f2fs: remove unneeded parameter in find_in_block() f2fs: fix uninit-value in f2fs_lookup f2fs: fix to check segment boundary during SIT page readahead f2fs: fix to do sanity check on segment/section count f2fs: compress: introduce page array slab cache f2fs: compress: introduce cic/dic slab cache f2fs: compress: fix to disallow enabling compress on non-empty file f2fs: fix to set SBI_NEED_FSCK flag for inconsistent inode f2fs: don't issue flush in f2fs_flush_device_cache() for nobarrier case f2fs: introduce check_swap_activate_fast() Chengguang Xu (1): f2fs: code cleanup by removing unnecessary check Daeho Jeong (6): f2fs: add block address limit check to compressed file f2fs: change compr_blocks of superblock info to 64bit f2fs: change i_compr_blocks of inode to atomic value f2fs: change return value of f2fs_disable_compressed_file to bool f2fs: change virtual mapping way for compression pages f2fs: fix writecount false positive in releasing compress blocks Dan Robertson (1): f2fs: check position in move range ioctl Daniel Rosenberg (3): unicode: Add utf8_casefold_hash fs: Add standard casefolding support f2fs: Use generic casefolding support Eric Biggers (1): f2fs: reject CASEFOLD inode flag without casefold feature Jack Qiu (1): f2fs: correct statistic of APP_DIRECT_IO/APP_DIRECT_READ_IO Jaegeuk Kim (4): f2fs: point man pages for some f2fs utils f2fs: fix slab leak of rpages pointer f2fs: fix memory alignment to support 32bit f2fs: handle errors of f2fs_get_meta_page_nofail Jamie Iles (1): f2fs: wait for sysfs kobject removal before freeing f2fs_sb_info Matthew Wilcox (Oracle) (1): f2fs: Simplify SEEK_DATA implementation Randy Dunlap (1): f2fs: Documentation edits/fixes Wang Xiaojun (3): f2fs: remove unused check on version_bitmap f2fs: remove duplicated code in sanity_check_area_boundary f2fs: fix wrong total_sections check and fsmeta check Xiaojun Wang (2): f2fs: remove duplicated type casting f2fs: change return value of reserved_segments to unsigned int Zhang Qilong (1): f2fs: add trace exit in exception path Documentation/ABI/testing/sysfs-fs-f2fs | 3 +- Documentation/filesystems/f2fs.rst | 82
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On 15:53 Thu 15 Oct 2020, Joe Perches wrote: On Fri, 2020-10-16 at 04:19 +0530, Bhaskar Chowdhury wrote: On 14:10 Thu 15 Oct 2020, Joe Perches wrote: > On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: > > On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > > > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > > > Fix the space in the middle in below entry. > > > > > > > > memry||memory > > > [] > > > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > > > [] > > > > @@ -885,7 +885,7 @@ meetign||meeting > > > > memeory||memory > > > > memmber||member > > > > memoery||memory > > > > -memry ||memory > > > > +memry||memory > > > > > > No. Don't post a bad patch, assume > > > it's applied and then post a fix to > > > the bad patch as v2. > > > > > > Send a single clean patch. > > > > > > > Not sure what you mean...could you elaborate...don't know what is going on..> > > You sent a patch with a defect Who doesn't??? No one. > You sent a V2 patch that just corrects the defect in the first patch. That's how it is working here for long time ...I am not sure about your involvement. wrong. Your first patch has not been, and should not be applied, as it has a trivial known defect. Please don't unnecessarily streach thing ...we have other things to do ...do not bringing "new rules" here for the sake of it. It's not a new rule. Don't introduce patches with defects. You have all flawed understanding...please stay away ..if you don't understand something...> signature.asc Description: PGP signature
Re: [RFC PATCH 2/3] hugetlbfs: introduce hinode_rwsem for pmd sharing synchronization
On Tue, Oct 13, 2020 at 04:10:59PM -0700, Mike Kravetz wrote: > Due to pmd sharing, the huge PTE pointer returned by huge_pte_alloc > may not be valid. This can happen if a call to huge_pmd_unshare for > the same pmd is made in another thread. > > To address this issue, add a rw_semaphore (hinode_rwsem) to the hugetlbfs > inode. > - hinode_rwsem is taken in read mode before calling huge_pte_alloc, and > held until finished with the returned pte pointer. > - hinode_rwsem is held in write mode whenever huge_pmd_unshare is called. > > In the locking hierarchy, hinode_rwsem must be taken before a page lock. > > In an effort to minimize performance impacts, hinode_rwsem is not taken > if the caller knows the target can not possibly be part of a shared pmd. > lockdep_assert calls are added to huge_pmd_share and huge_pmd_unshare to > help catch callers not using the proper locking. > > Signed-off-by: Mike Kravetz Hi Mike, I didn't find a problem on main idea of introducing hinode_rwsem, so I'm fine if the known problems are fixed. I have one question. This patch seems to make sure that huge_pmd_unshare() are called under holding hinode_rwsem in write mode for some case. Some callers of try_to_unmap() seem not to hold it like shrink_page_list(), unmap_page(), which is OK because they never call try_to_unmap() for hugetlb pages. And unmap_ref_private() doesn't takes hinode_rwsem either, and that's also OK because this function never handles pmd sharing case. So what about unmap_single_vma()? It seems that this generic function could reach huge_pmd_unshare() without hinode_rwsem, so what prevents the race here? (Maybe I might miss some assumption or condition over this race...) I left a few other minor comments below ... > @@ -4424,6 +4442,11 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct > vm_area_struct *vma, > > ptep = huge_pte_offset(mm, haddr, huge_page_size(h)); > if (ptep) { > + /* > + * Since we hold no locks, ptep could be stale. That is > + * OK as we are only making decisions based on content and > + * not actually modifying content here. > + */ nice comment, thank you. > entry = huge_ptep_get(ptep); > if (unlikely(is_hugetlb_entry_migration(entry))) { > migration_entry_wait_huge(vma, mm, ptep); > @@ -4431,20 +4454,32 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct > vm_area_struct *vma, > } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry))) > return VM_FAULT_HWPOISON_LARGE | > VM_FAULT_SET_HINDEX(hstate_index(h)); > - } else { > - ptep = huge_pte_alloc(mm, haddr, huge_page_size(h)); > - if (!ptep) > - return VM_FAULT_OOM; > } > > + /* > + * Acquire hinode_sem before calling huge_pte_alloc and hold hinode_rwsem? > + * until finished with ptep. This prevents huge_pmd_unshare from > + * being called elsewhere and making the ptep no longer valid. > + * > + * ptep could have already be assigned via huge_pte_offset. That > + * is OK, as huge_pte_alloc will return the same value unless > + * something has changed. > + */ ... > @@ -278,10 +278,14 @@ static __always_inline ssize_t > __mcopy_atomic_hugetlb(struct mm_struct *dst_mm, > BUG_ON(dst_addr >= dst_start + len); > > /* > - * Serialize via hugetlb_fault_mutex > + * Serialize via hinode_rwsem hugetlb_fault_mutex. ^ "and" here? > + * hinode_rwsem ensures the dst_pte remains valid even > + * in the case of shared pmds. fault mutex prevents > + * races with other faulting threads. >*/ > idx = linear_page_index(dst_vma, dst_addr); > mapping = dst_vma->vm_file->f_mapping; > + hinode_lock_read(mapping, dst_vma, dst_addr); > hash = hugetlb_fault_mutex_hash(mapping, idx); > mutex_lock(_fault_mutex_table[hash]); Thanks, Naoya Horiguchi
Re: [GIT PULL] SafeSetID changes for v5.10
These were rebased since the merge window started, for no apparent reason. Were they in linux-next? And if so, why was I sent some different version? Linus
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On Fri, 2020-10-16 at 04:25 +0530, Bhaskar Chowdhury wrote: > You have all flawed understanding...please stay away .. > if you don't understand something... You're funny. You're wrong, but you're still funny.
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On 16:06 Thu 15 Oct 2020, Joe Perches wrote: On Fri, 2020-10-16 at 04:25 +0530, Bhaskar Chowdhury wrote: You have all flawed understanding...please stay away .. if you don't understand something... You're funny. You're wrong, but you're still funny. ROFL ..you too...what a waste of time ...shame that I am engage this kind of conversation ...heck signature.asc Description: PGP signature
Re: [GIT PULL] parisc architecture updates for kernel v5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 09:20:25 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git > parisc-5.10-1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7286d2a37eb955c5eeec2b042844f1c1b3ff0fe1 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] tracing: Updates for 5.10
The pull request you sent on Thu, 15 Oct 2020 13:53:45 -0400: > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git > trace-v5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/fefa636d815975b34afc45f50852a2810fb23ba9 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] integrity subsystem updates for v5.10
The pull request you sent on Wed, 14 Oct 2020 13:19:31 -0400: > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git > tags/integrity-v5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/840e5bb326bbcb16ce82dd2416d2769de4839aea Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] Hyper-V commits for 5.10, part 2
The pull request you sent on Thu, 15 Oct 2020 16:32:48 +: > ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git > tags/hyperv-next-signed has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2d0f6b0aab9afbd6fdf3514cb4acc249d7aebf9c Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
[PATCH] mmc: sdhci-of-esdhc: set timeout to max before tuning
On rare occations there is the following error: mmc0: Tuning timeout, falling back to fixed sampling clock There are SD cards which takes a significant longer time to reply to the first CMD19 command. The eSDHC takes the data timeout value into account during the tuning period. The SDHCI core doesn't explicitly set this timeout for the tuning procedure. Thus on the slow cards, there might be a spurious "Buffer Read Ready" interrupt, which in turn triggers a wrong sequence of events. In the end this will lead to an unsuccessful tuning procedure and to the above error. To workaround this, set the timeout to the maximum value (which is the best we can do) and the SDHCI core will take care of the proper timeout handling. Signed-off-by: Michael Walle --- drivers/mmc/host/sdhci-of-esdhc.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 0b45eff6fed4..baf7801a1804 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -1052,6 +1052,17 @@ static int esdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) esdhc_tuning_block_enable(host, true); + /* +* The eSDHC controller takes the data timeout value into account +* during tuning. If the SD card is too slow sending the response, the +* timer will expire and a "Buffer Read Ready" interrupt without data +* is triggered. This leads to tuning errors. +* +* Just set the timeout to the maximum value because the core will +* already take care of it in sdhci_send_tuning(). +*/ + sdhci_writeb(host, 0xe, SDHCI_TIMEOUT_CONTROL); + hs400_tuning = host->flags & SDHCI_HS400_TUNING; do { -- 2.20.1
Re: linux-next: manual merge of the char-misc tree with the powerpc tree
Hi all, On Tue, 6 Oct 2020 18:35:06 +1100 Stephen Rothwell wrote: > > Today's linux-next merge of the char-misc tree got a conflict in: > > drivers/misc/ocxl/Kconfig > > between commit: > > dde6f18a8779 ("ocxl: Don't return trigger page when allocating an > interrupt") > > from the powerpc tree and commit: > > 4b53a3c72116 ("ocxl: fix kconfig dependency warning for OCXL") > > from the char-misc tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > > diff --cc drivers/misc/ocxl/Kconfig > index 0d815b2a40b3,947294f6d7f4.. > --- a/drivers/misc/ocxl/Kconfig > +++ b/drivers/misc/ocxl/Kconfig > @@@ -9,9 -9,8 +9,9 @@@ config OCXL_BAS > > config OCXL > tristate "OpenCAPI coherent accelerator support" > -depends on PPC_POWERNV && PCI && EEH && HOTPLUG_PCI_POWERNV > +depends on PPC_POWERNV && PCI && EEH && PPC_XIVE_NATIVE > ++depends on HOTPLUG_PCI_POWERNV > select OCXL_BASE > - select HOTPLUG_PCI_POWERNV > default m > help > Select this option to enable the ocxl driver for Open This is now a conflict between the powerpc tree and Linus' tree. -- Cheers, Stephen Rothwell pgpxqie4XDWCo.pgp Description: OpenPGP digital signature
Re: cgroup and FALLOC_FL_PUNCH_HOLE: WARNING: CPU: 13 PID: 2438 at mm/page_counter.c:57 page_counter_uncharge+0x4b/0x5
On 10/14/20 11:31 AM, Mike Kravetz wrote: > On 10/14/20 11:18 AM, David Hildenbrand wrote: > > FWIW - I ran libhugetlbfs tests which do a bunch of hole punching > with (and without) hugetlb controller enabled and did not see this issue. > I took a closer look after running just the fallocate_stress test in libhugetlbfs. Here are the cgroup counter values: hugetlb.2MB.failcnt 0 hugetlb.2MB.limit_in_bytes 9223372036854771712 hugetlb.2MB.max_usage_in_bytes 209715200 hugetlb.2MB.rsvd.failcnt 0 hugetlb.2MB.rsvd.limit_in_bytes 9223372036854771712 hugetlb.2MB.rsvd.max_usage_in_bytes 601882624 hugetlb.2MB.rsvd.usage_in_bytes 392167424 hugetlb.2MB.usage_in_bytes 0 We did not hit the WARN_ON_ONCE(), but the 'rsvd.usage_in_bytes' value is not correct in that it should be zero. No huge page reservations remain after the test. HugePages_Total:1024 HugePages_Free: 1024 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB Hugetlb: 2097152 kB To try and better understand the reservation cgroup controller, I addded a few printks to the code. While running fallocate_stress with the printks, I can consistently hit the WARN_ON_ONCE() due to the counter going negative. Here are the cgroup counter values after such a run: hugetlb.2MB.failcnt 0 hugetlb.2MB.limit_in_bytes 9223372036854771712 hugetlb.2MB.max_usage_in_bytes 209715200 hugetlb.2MB.rsvd.failcnt 3 hugetlb.2MB.rsvd.limit_in_bytes 9223372036854771712 hugetlb.2MB.rsvd.max_usage_in_bytes 251658240 hugetlb.2MB.rsvd.usage_in_bytes 18446744073487253504 hugetlb.2MB.usage_in_bytes 0 Again, no reserved pages after the test. HugePages_Total:1024 HugePages_Free: 1024 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB Hugetlb: 2097152 kB I have some basic hugetlb hole punch functionality tests. Running these on the kernel with added printk's does not cause any issues. In order to reproduce, I need to run fallocate_stress test which will cause hole punch to race with page fault. Best guess at this time is that some of the error/race detection reservation back out code is not properly dealing with cgroup accounting. I'll take a look at this as well. -- Mike Kravetz