* [PATCH] [PATCH] Firmware security information in SYSFS
From: Daniel Gutson @ 2020-07-16 22:36 UTC (permalink / raw)
To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
linux-kernel, Richard Hughes, Alex Bazhaniuk
This patch exports security-related firmware configuration
in the sysfs filesystem. In this initial patch, I include
some configuration attributes for the system SPI chip.
This initial version exports the BIOS Write Enable (bioswe),
BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
fields of the Bios Control register. The idea is to keep adding more
flags, not only from the BC but also from other registers in following
versions.
The goal is that the attributes are avilable to fwupd when SecureBoot
is turned on.
The patch provides a new misc driver, as proposed in the previous patch,
that provides registration functions for HW Driver devices.
In this case, the intel SPI flash chip (intel-spi) registers functions
so it can export the three attributes mentioned above.
Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
---
.../ABI/stable/sysfs-firmware-security | 23 ++++
MAINTAINERS | 7 ++
drivers/misc/Kconfig | 9 ++
drivers/misc/Makefile | 1 +
drivers/misc/firmware_security_data.c | 103 ++++++++++++++++++
drivers/mtd/spi-nor/controllers/Kconfig | 1 +
.../mtd/spi-nor/controllers/intel-spi-pci.c | 45 +++++++-
drivers/mtd/spi-nor/controllers/intel-spi.c | 92 ++++++++++++----
.../platform_data/firmware_security_data.h | 17 +++
9 files changed, 277 insertions(+), 21 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-firmware-security
create mode 100644 drivers/misc/firmware_security_data.c
create mode 100644 include/linux/platform_data/firmware_security_data.h
diff --git a/Documentation/ABI/stable/sysfs-firmware-security b/Documentation/ABI/stable/sysfs-firmware-security
new file mode 100644
index 000000000000..fdfcc40c3c09
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-firmware-security
@@ -0,0 +1,23 @@
+What: /sys/kernel/firmware-security/bioswe
+Date: July 2020
+KernelVersion: 5.8.0
+Contact: Daniel Gutson <daniel.gutson@eclypsium.com>
+Description: If the system firmware set BIOS Write Enable.
+ 0: writes disabled, 1: writes enabled.
+Users: https://github.com/fwupd/fwupd
+
+What: /sys/kernel/firmware-security/ble
+Date: July 2020
+KernelVersion: 5.8.0
+Contact: Daniel Gutson <daniel.gutson@eclypsium.com>
+Description: If the system firmware set Bios Lock Enable.
+ 0: SMM lock disabled, 1: SMM lock enabled.
+Users: https://github.com/fwupd/fwupd
+
+What: /sys/kernel/firmware-security/smm_bwp
+Date: July 2020
+KernelVersion: 5.8.0
+Contact: Daniel Gutson <daniel.gutson@eclypsium.com>
+Description: If the system firmware set SMM Bios Write Protect.
+ 0: writes disabled unless in SMM, 1: writes enabled.
+Users: https://github.com/fwupd/fwupd
diff --git a/MAINTAINERS b/MAINTAINERS
index b4a43a9e7fbc..152b3b9b802c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6699,6 +6699,13 @@ F: Documentation/firmware_class/
F: drivers/base/firmware_loader/
F: include/linux/firmware.h
+FIRMWARE SECURITY DATA
+M: Daniel Gutson <daniel.gutson@eclypsium.com>
+S: Supported
+F: Documentation/ABI/sysfs-firmware-security
+F: drivers/misc/firmware_security_data.c
+F: include/linux/platform_data/firmware_security_data.h
+
FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
M: Joshua Morris <josh.h.morris@us.ibm.com>
M: Philip Kelleher <pjk1939@linux.ibm.com>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index e1b1ba5e2b92..72fafb63be58 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -456,6 +456,15 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.
+config FIRMWARE_SECURITY_DATA
+ tristate "Firmware security information in the SYSFS"
+ depends on SYSFS
+ help
+ This kernel module is a helper driver to provide information about
+ firmware security settings and configuration.
+ This module is used by other device drivers -such as the intel-spi-
+ to publish the information in /sys/kernel/firmware_security.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c7bd01ac6291..f938abc2f733 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC) += pvpanic.o
obj-$(CONFIG_HABANA_AI) += habanalabs/
obj-$(CONFIG_UACCE) += uacce/
obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
+obj-$(CONFIG_FIRMWARE_SECURITY_DATA) += firmware_security_data.o
diff --git a/drivers/misc/firmware_security_data.c b/drivers/misc/firmware_security_data.c
new file mode 100644
index 000000000000..cfb7f39a2cd9
--- /dev/null
+++ b/drivers/misc/firmware_security_data.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Firmware security data kernel module
+ *
+ * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
+ * Copyright (C) 2020 Eclypsium Inc.
+ */
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/platform_data/firmware_security_data.h>
+
+static DEFINE_MUTEX(entries_mutex);
+static LIST_HEAD(entries);
+static struct kobject *firmware_data_kobj;
+
+struct firmware_security_data {
+ struct kobj_attribute kobj_attr;
+ ssize_t (*callback)(char *buf, void *private_data);
+ void *private_data;
+ struct list_head list_node;
+};
+
+static ssize_t internal_callback(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct firmware_security_data *fwsd =
+ container_of(attr, struct firmware_security_data, kobj_attr);
+ return fwsd->callback(buf, fwsd->private_data);
+}
+
+int register_firmware_security_data_callback(
+ const char *name, ssize_t (*callback)(char *buf, void *private_data),
+ void *private_data)
+{
+ int retval;
+
+ struct firmware_security_data *new_data;
+
+ if (name == NULL || name[0] == 0)
+ return -EINVAL;
+
+ new_data = kmalloc(sizeof(struct firmware_security_data), GFP_KERNEL);
+ if (new_data == NULL)
+ return -ENOMEM;
+
+ /* initialize attributes: */
+ sysfs_attr_init(new_data->kobj_attr.attr);
+ new_data->kobj_attr.attr.name = name;
+ new_data->kobj_attr.attr.mode = 0664;
+ new_data->kobj_attr.show = internal_callback;
+ new_data->kobj_attr.store = NULL;
+
+ new_data->callback = callback;
+ new_data->private_data = private_data;
+
+ /* attempt to create the file: */
+ retval = sysfs_create_file(firmware_data_kobj,
+ &new_data->kobj_attr.attr);
+ if (retval == 0) {
+ /* append to the list of entries: */
+ INIT_LIST_HEAD(&new_data->list_node);
+ mutex_lock(&entries_mutex);
+ list_add(&new_data->list_node, &entries);
+ mutex_unlock(&entries_mutex);
+ } else {
+ kfree(new_data);
+ }
+
+ return retval;
+}
+EXPORT_SYMBOL_GPL(register_firmware_security_data_callback);
+
+static int __init firmware_security_data_init(void)
+{
+ firmware_data_kobj =
+ kobject_create_and_add("firmware_security", kernel_kobj);
+ if (!firmware_data_kobj)
+ return -ENOMEM;
+
+ return 0;
+}
+static void __exit firmware_security_data_exit(void)
+{
+ struct list_head *entry = entries.next;
+
+ while (entry != &entries) {
+ struct list_head *next = entry->next;
+
+ kfree(list_entry(entry, struct firmware_security_data,
+ list_node));
+ entry = next;
+ }
+
+ kobject_put(firmware_data_kobj);
+}
+module_init(firmware_security_data_init);
+module_exit(firmware_security_data_exit);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Daniel Gutson <daniel.gutson@eclypsium.com>");
diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
index d89a5ea9446a..01d9c49f8e49 100644
--- a/drivers/mtd/spi-nor/controllers/Kconfig
+++ b/drivers/mtd/spi-nor/controllers/Kconfig
@@ -45,6 +45,7 @@ config SPI_INTEL_SPI_PCI
tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
depends on X86 && PCI
select SPI_INTEL_SPI
+ select FIRMWARE_SECURITY_DATA
help
This enables PCI support for the Intel PCH/PCU SPI controller in
master mode. This controller is present in modern Intel hardware
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
index 81329f680bec..f919f09de1e4 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
@@ -10,11 +10,14 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/platform_data/firmware_security_data.h>
#include "intel-spi.h"
#define BCR 0xdc
#define BCR_WPD BIT(0)
+#define BCR_BLE BIT(1)
+#define BCR_SMM_BWP BIT(5)
static const struct intel_spi_boardinfo bxt_info = {
.type = INTEL_SPI_BXT,
@@ -24,6 +27,44 @@ static const struct intel_spi_boardinfo cnl_info = {
.type = INTEL_SPI_CNL,
};
+static ssize_t cnl_read_field(char *buf, struct pci_dev *pdev, u32 mask)
+{
+ u32 bcr;
+
+ if (pci_read_config_dword(pdev, BCR, &bcr) != PCIBIOS_SUCCESSFUL)
+ return -EIO;
+
+ return sprintf(buf, "%d\n", (int)!!(bcr & mask));
+}
+
+static ssize_t cnl_wpd_firmware_security_callbacks(char *buf,
+ void *private_data)
+{
+ return cnl_read_field(buf, (struct pci_dev *)private_data, BCR_WPD);
+}
+
+static ssize_t cnl_ble_firmware_security_callbacks(char *buf,
+ void *private_data)
+{
+ return cnl_read_field(buf, (struct pci_dev *)private_data, BCR_BLE);
+}
+
+static ssize_t cnl_smm_bwp_firmware_security_callbacks(char *buf,
+ void *private_data)
+{
+ return cnl_read_field(buf, (struct pci_dev *)private_data, BCR_SMM_BWP);
+}
+
+static void register_firmware_security_data_callbacks(struct pci_dev *pdev)
+{
+ register_firmware_security_data_callback(
+ "bioswe", &cnl_wpd_firmware_security_callbacks, pdev);
+ register_firmware_security_data_callback(
+ "ble", &cnl_ble_firmware_security_callbacks, pdev);
+ register_firmware_security_data_callback(
+ "smm_bwp", &cnl_smm_bwp_firmware_security_callbacks, pdev);
+}
+
static int intel_spi_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
@@ -50,6 +91,8 @@ static int intel_spi_pci_probe(struct pci_dev *pdev,
}
info->writeable = !!(bcr & BCR_WPD);
+ register_firmware_security_data_callbacks(pdev);
+
ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info);
if (IS_ERR(ispi))
return PTR_ERR(ispi);
@@ -76,7 +119,7 @@ static const struct pci_device_id intel_spi_pci_ids[] = {
{ PCI_VDEVICE(INTEL, 0xa224), (unsigned long)&bxt_info },
{ PCI_VDEVICE(INTEL, 0xa324), (unsigned long)&cnl_info },
{ PCI_VDEVICE(INTEL, 0xa3a4), (unsigned long)&bxt_info },
- { },
+ {},
};
MODULE_DEVICE_TABLE(pci, intel_spi_pci_ids);
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c
index 61d2a0ad2131..939259541e75 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.c
@@ -16,6 +16,7 @@
#include <linux/mtd/partitions.h>
#include <linux/mtd/spi-nor.h>
#include <linux/platform_data/intel-spi.h>
+#include <linux/platform_data/firmware_security_data.h>
#include "intel-spi.h"
@@ -48,17 +49,17 @@
#define FADDR 0x08
#define DLOCK 0x0c
-#define FDATA(n) (0x10 + ((n) * 4))
+#define FDATA(n) (0x10 + ((n)*4))
#define FRACC 0x50
-#define FREG(n) (0x54 + ((n) * 4))
+#define FREG(n) (0x54 + ((n)*4))
#define FREG_BASE_MASK 0x3fff
#define FREG_LIMIT_SHIFT 16
#define FREG_LIMIT_MASK (0x03fff << FREG_LIMIT_SHIFT)
/* Offset is from @ispi->pregs */
-#define PR(n) ((n) * 4)
+#define PR(n) ((n)*4)
#define PR_WPE BIT(31)
#define PR_LIMIT_SHIFT 16
#define PR_LIMIT_MASK (0x3fff << PR_LIMIT_SHIFT)
@@ -95,6 +96,8 @@
#define BYT_SSFSTS_CTL 0x90
#define BYT_BCR 0xfc
#define BYT_BCR_WPD BIT(0)
+#define BYT_BCR_BLE BIT(1)
+#define BYT_BCR_SMM_BWP BIT(5)
#define BYT_FREG_NUM 5
#define BYT_PR_NUM 5
@@ -161,7 +164,8 @@ struct intel_spi {
static bool writeable;
module_param(writeable, bool, 0);
-MODULE_PARM_DESC(writeable, "Enable write access to SPI flash chip (default=0)");
+MODULE_PARM_DESC(writeable,
+ "Enable write access to SPI flash chip (default=0)");
static void intel_spi_dump_regs(struct intel_spi *ispi)
{
@@ -179,8 +183,8 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
dev_dbg(ispi->dev, "DLOCK=0x%08x\n", readl(ispi->base + DLOCK));
for (i = 0; i < 16; i++)
- dev_dbg(ispi->dev, "FDATA(%d)=0x%08x\n",
- i, readl(ispi->base + FDATA(i)));
+ dev_dbg(ispi->dev, "FDATA(%d)=0x%08x\n", i,
+ readl(ispi->base + FDATA(i)));
dev_dbg(ispi->dev, "FRACC=0x%08x\n", readl(ispi->base + FRACC));
@@ -220,9 +224,8 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
base = value & PR_BASE_MASK;
dev_dbg(ispi->dev, " %02d base: 0x%08x limit: 0x%08x [%c%c]\n",
- i, base << 12, (limit << 12) | 0xfff,
- value & PR_WPE ? 'W' : '.',
- value & PR_RPE ? 'R' : '.');
+ i, base << 12, (limit << 12) | 0xfff,
+ value & PR_WPE ? 'W' : '.', value & PR_RPE ? 'R' : '.');
}
dev_dbg(ispi->dev, "Flash regions:\n");
@@ -237,7 +240,7 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
dev_dbg(ispi->dev, " %02d disabled\n", i);
else
dev_dbg(ispi->dev, " %02d base: 0x%08x limit: 0x%08x\n",
- i, base << 12, (limit << 12) | 0xfff);
+ i, base << 12, (limit << 12) | 0xfff);
}
dev_dbg(ispi->dev, "Using %cW sequencer for register access\n",
@@ -305,6 +308,52 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
INTEL_SPI_TIMEOUT * 1000);
}
+static ssize_t byt_read_field(char *buf, const struct intel_spi *ispi, u32 mask)
+{
+ u32 val;
+
+ val = readl(ispi->base + BYT_BCR);
+ return sprintf(buf, "%d\n", (int)!!(val & mask));
+}
+
+static ssize_t byt_wpd_firmware_security_callbacks(char *buf,
+ void *private_data)
+{
+ return byt_read_field(buf, (struct intel_spi *)private_data,
+ BYT_BCR_WPD);
+}
+
+static ssize_t byt_ble_firmware_security_callbacks(char *buf,
+ void *private_data)
+{
+ return byt_read_field(buf, (struct intel_spi *)private_data,
+ BYT_BCR_BLE);
+}
+
+static ssize_t byt_smm_bwp_firmware_security_callbacks(char *buf,
+ void *private_data)
+{
+ return byt_read_field(buf, (struct intel_spi *)private_data,
+ BYT_BCR_SMM_BWP);
+}
+
+static void register_firmware_security_data_callbacks(struct intel_spi *ispi)
+{
+ switch (ispi->info->type) {
+ case INTEL_SPI_BYT:
+ register_firmware_security_data_callback(
+ "bioswe", &byt_wpd_firmware_security_callbacks, ispi);
+ register_firmware_security_data_callback(
+ "ble", &byt_ble_firmware_security_callbacks, ispi);
+ register_firmware_security_data_callback(
+ "smm_bwp", &byt_smm_bwp_firmware_security_callbacks,
+ ispi);
+ break;
+ default:
+ break; /* TODO. not yet implemented. */
+ }
+}
+
static int intel_spi_init(struct intel_spi *ispi)
{
u32 opmenu0, opmenu1, lvscc, uvscc, val;
@@ -383,7 +432,8 @@ static int intel_spi_init(struct intel_spi *ispi)
ispi->erase_64k = false;
if (ispi->sregs == NULL && (ispi->swseq_reg || ispi->swseq_erase)) {
- dev_err(ispi->dev, "software sequencer not supported, but required\n");
+ dev_err(ispi->dev,
+ "software sequencer not supported, but required\n");
return -EINVAL;
}
@@ -422,6 +472,8 @@ static int intel_spi_init(struct intel_spi *ispi)
intel_spi_dump_regs(ispi);
+ register_firmware_security_data_callbacks(ispi);
+
return 0;
}
@@ -538,7 +590,6 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, size_t len,
default:
return -EINVAL;
}
-
}
writel(val, ispi->sregs + SSFSTS_CTL);
@@ -655,7 +706,8 @@ static ssize_t intel_spi_read(struct spi_nor *nor, loff_t from, size_t len,
/* Read cannot cross 4K boundary */
block_size = min_t(loff_t, from + block_size,
- round_up(from + 1, SZ_4K)) - from;
+ round_up(from + 1, SZ_4K)) -
+ from;
writel(from, ispi->base + FADDR);
@@ -712,7 +764,8 @@ static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
/* Write cannot cross 4K boundary */
block_size = min_t(loff_t, to + block_size,
- round_up(to + 1, SZ_4K)) - to;
+ round_up(to + 1, SZ_4K)) -
+ to;
writel(to, ispi->base + FADDR);
@@ -779,8 +832,8 @@ static int intel_spi_erase(struct spi_nor *nor, loff_t offs)
while (len > 0) {
writel(offs, ispi->base + FADDR);
- ret = intel_spi_sw_cycle(ispi, nor->erase_opcode,
- 0, OPTYPE_WRITE_WITH_ADDR);
+ ret = intel_spi_sw_cycle(ispi, nor->erase_opcode, 0,
+ OPTYPE_WRITE_WITH_ADDR);
if (ret)
return ret;
@@ -894,12 +947,11 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = {
.erase = intel_spi_erase,
};
-struct intel_spi *intel_spi_probe(struct device *dev,
- struct resource *mem, const struct intel_spi_boardinfo *info)
+struct intel_spi *intel_spi_probe(struct device *dev, struct resource *mem,
+ const struct intel_spi_boardinfo *info)
{
const struct spi_nor_hwcaps hwcaps = {
- .mask = SNOR_HWCAPS_READ |
- SNOR_HWCAPS_READ_FAST |
+ .mask = SNOR_HWCAPS_READ | SNOR_HWCAPS_READ_FAST |
SNOR_HWCAPS_PP,
};
struct mtd_partition part;
diff --git a/include/linux/platform_data/firmware_security_data.h b/include/linux/platform_data/firmware_security_data.h
new file mode 100644
index 000000000000..a73b286644d0
--- /dev/null
+++ b/include/linux/platform_data/firmware_security_data.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Firmware security data kernel module
+ *
+ * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
+ * Copyright (C) 2020 Eclypsium Inc.
+ */
+#ifndef FIRMWARE_SECURITY_DATA_H
+#define FIRMWARE_SECURITY_DATA_H
+
+#include <linux/types.h>
+
+extern int register_firmware_security_data_callback(
+ const char *name, ssize_t (*callback)(char *buf, void *private_data),
+ void *private_data);
+
+#endif /* FIRMWARE_SECURITY_DATA_H */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 2/2] repository: allow repository format upgrade with extensions
From: Jonathan Nieder @ 2020-07-16 22:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Johannes Schindelin, Derrick Stolee,
Johannes Schindelin via GitGitGadget, git, delphij,
Huan Huan Chen, brian m. carlson
In-Reply-To: <xmqqd04vigpy.fsf@gitster.c.googlers.com>
(replying from vacation; back tomorrow)
Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>> Yeah, I agree with this line of reasoning. I'd prefer to see it
>> addressed now, so that we don't have to remember to do anything later.
>
> Very true. Also the documentation may need some updating,
> e.g. "These 4 extensions are honored without adding
> repositoryFormatVersion to your repository (as special cases)" to
> avoid further confusion e.g. "why doesn't my objectFormat=SHA-3 does
> not take effect by itself?".
Yes, I agree, especially about documentation.
For 2.29, I would like to do or see the following:
- defining the list of repository format v0 supported extensions as
"these and no more", futureproofing along the lines suggested in
Peff's patch. I like the general approach taken there since it
allows parsing the relevant config in a single pass, so I think
it basically takes the right approach. (That said, it might be
possible to simplify a bit with further changes, e.g. by using the
configset API.)
When doing this for real, we'd want to document the set of
supported extensions. That is especially useful to independent
implementers wanting to support Git's formats, since it tells
them "this is the minimum set of extensions that you must
either handle or error out cleanly on to maintain compatibility
with Git's repository format v0".
- improving the behavior when an extension not supported in v0 is
encountered in a v0 repository. For extensions that are supported
in v1 and not v0, we should presumably error out so the user can
repair the repository, and we can put the "noop" extension in that
category for the sake of easy testing. We can also include a check
in "git fsck" for repositories that request the undefined behavior
of v0 repositories with non-v0 extensions, for faster diagnosis.
What about unrecognized extensions that are potentially extensions
yet to be defined? Should these be silently ignored to match the
historical behavior, or should we error out even in repository
format v0? I lean toward the latter; we'll need to be cautious,
though, e.g. by making this a separate patch so we can easily tweak
it if this ends up being disruptive in some unanticipated way.
- making "git init" use repository format v1 by default. It's been
long enough that users can count on Git implementations supporting
it. This way, users are less likely to run into v0+extensions
confusion, just because users are less likely to be using v0.
Does that sound like a good plan to others? If so, are there any
steps beyond the two first patches in jn/v0-with-extensions-fix that
we would want in order to prepare for it in 2.28?
My preference would be to move forward in 2.28 with the first two
patches in that topic branch (i.e., *not* the third yet), since they
don't produce any user facing behavior that would create danger for
users or clash with this plan. Today, the only extensions we
recognize are in that set of extensions that we'll want to continue to
recognize in v0 (except possibly the for-testing extension "noop").
The steps to take with additional extensions are more subtle so it
seems reasonable for them to bake in "next" and then "master" for a
2.29 release.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v7 01/10] media: v4l2-subdev: Introduce [get|set]_mbus_config pad ops
From: Janusz Krzysztofik @ 2020-07-16 22:15 UTC (permalink / raw)
To: mchehab, hverkuil-cisco, sakari.ailus, laurent.pinchart,
Jacopo Mondi
Cc: Jacopo Mondi, niklas.soderlund+renesas, kieran.bingham,
dave.stevenson, hyun.kwon, robert.jarzmik, linux-media,
linux-renesas-soc, Janusz Krzysztofik
In-Reply-To: <20200716142713.110655-2-jacopo+renesas@jmondi.org>
Hi Jacopo,
On Thursday, July 16, 2020 4:27:04 P.M. CEST Jacopo Mondi wrote:
> Introduce two new pad operations to allow retrieving and configuring the
> media bus parameters on a subdevice pad.
>
> The newly introduced operations aims to replace the s/g_mbus_config video
> operations, which have been on their way for deprecation since a long
> time.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> include/media/v4l2-subdev.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index f7fe78a6f65a..d8b9d5735307 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -670,6 +670,29 @@ struct v4l2_subdev_pad_config {
> *
> * @set_frame_desc: set the low level media bus frame parameters, @fd array
> * may be adjusted by the subdev driver to device capabilities.
> + *
> + * @get_mbus_config: get the media bus configuration of a remote sub-device.
> + * The media bus configuration is usually retrieved from the
> + * firmware interface at sub-device probe time, immediately
> + * applied to the hardware and eventually adjusted by the
> + * driver. Remote sub-devices (usually video receivers) shall
> + * use this operation to query the transmitting end bus
> + * configuration in order to adjust their own one accordingly.
> + * Callers should make sure they get the most up-to-date as
> + * possible configuration from the remote end, likely calling
> + * this operation as close as possible to stream on time. The
> + * operation shall fail if the pad index it has been called on
> + * is not valid.
> + *
> + * @set_mbus_config: set the media bus configuration of a remote sub-device.
> + * This operations is intended to allow, in combination with
> + * the get_mbus_config operation, the negotiation of media bus
> + * configuration parameters between media sub-devices. The
> + * operation shall not fail if the requested configuration is
> + * not supported, but the driver shall update the content of
> + * the %config argument to reflect what has been actually
> + * applied to the hardware. The operation shall fail if the
> + * pad index it has been called on is not valid.
Could this description also clarify what results are expected in case of
hardware errors? The ov6650 implementation you propose may suggest such
errors may be expected to be ignored silently as long as current configuration
can be successfully obtained from hardware and passed back to the caller.
Moreover, since validity of the pad argument is expected to be verified, I
think this should be handled by the media infrastructure layer with the
drivers/media/v4l2-core/v4l2-subdev.c:check_pad() helper called from a
.set_mbus_config() wrapper added to v4l2_subdev_call_pad_wrappers, freeing
drivers from reimplementing it.
Thanks,
Janusz
> */
> struct v4l2_subdev_pad_ops {
> int (*init_cfg)(struct v4l2_subdev *sd,
> @@ -710,6 +733,10 @@ struct v4l2_subdev_pad_ops {
> struct v4l2_mbus_frame_desc *fd);
> int (*set_frame_desc)(struct v4l2_subdev *sd, unsigned int pad,
> struct v4l2_mbus_frame_desc *fd);
> + int (*get_mbus_config)(struct v4l2_subdev *sd, unsigned int pad,
> + struct v4l2_mbus_config *config);
> + int (*set_mbus_config)(struct v4l2_subdev *sd, unsigned int pad,
> + struct v4l2_mbus_config *config);
> };
>
> /**
>
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: Add PORT0_PAD_CTRL properties
From: Florian Fainelli @ 2020-07-16 22:19 UTC (permalink / raw)
To: Matthew Hagan
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Jakub Kicinski,
linux, netdev, linux-kernel, John Crispin, Jonathan McDowell,
Rob Herring, devicetree
In-Reply-To: <ea0a35ed686e6dace77e25cb70a8f39fdd1ea8ad.1594668793.git.mnhagan88@gmail.com>
On 7/13/2020 1:50 PM, Matthew Hagan wrote:
> Add names and decriptions of additional PORT0_PAD_CTRL properties.
>
> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> ---
> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index ccbc6d89325d..3d34c4f2e891 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -13,6 +13,14 @@ Optional properties:
>
> - reset-gpios: GPIO to be used to reset the whole device
>
> +Optional MAC configuration properties:
> +
> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6.
> +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to
> + falling edge.
> +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to
> + falling edge.
Are not these two mutually exclusive, that is the presence of one
implies the absence of the other?
--
Florian
^ permalink raw reply
* + linux-sched-mmh-drop-duplicated-words-in-comments.patch added to -mm tree
From: Andrew Morton @ 2020-07-16 22:51 UTC (permalink / raw)
To: mingo, mm-commits, peterz, rdunlap, sjpark
In-Reply-To: <20200703151445.b6a0cfee402c7c5c4651f1b1@linux-foundation.org>
The patch titled
Subject: linux/sched/mm.h: drop duplicated words in comments
has been added to the -mm tree. Its filename is
linux-sched-mmh-drop-duplicated-words-in-comments.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/linux-sched-mmh-drop-duplicated-words-in-comments.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/linux-sched-mmh-drop-duplicated-words-in-comments.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Randy Dunlap <rdunlap@infradead.org>
Subject: linux/sched/mm.h: drop duplicated words in comments
Drop doubled words "to" and "that".
Link: http://lkml.kernel.org/r/927ea8d8-3f6c-9b65-4c2b-63ab4bd59ef1@infradead.org
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: SeongJae Park <sjpark@amazon.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/sched/mm.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/include/linux/sched/mm.h~linux-sched-mmh-drop-duplicated-words-in-comments
+++ a/include/linux/sched/mm.h
@@ -23,7 +23,7 @@ extern struct mm_struct *mm_alloc(void);
* will still exist later on and mmget_not_zero() has to be used before
* accessing it.
*
- * This is a preferred way to to pin @mm for a longer/unbounded amount
+ * This is a preferred way to pin @mm for a longer/unbounded amount
* of time.
*
* Use mmdrop() to release the reference acquired by mmgrab().
@@ -236,7 +236,7 @@ static inline unsigned int memalloc_noio
* @flags: Flags to restore.
*
* Ends the implicit GFP_NOIO scope started by memalloc_noio_save function.
- * Always make sure that that the given flags is the return value from the
+ * Always make sure that the given flags is the return value from the
* pairing memalloc_noio_save call.
*/
static inline void memalloc_noio_restore(unsigned int flags)
@@ -267,7 +267,7 @@ static inline unsigned int memalloc_nofs
* @flags: Flags to restore.
*
* Ends the implicit GFP_NOFS scope started by memalloc_nofs_save function.
- * Always make sure that that the given flags is the return value from the
+ * Always make sure that the given flags is the return value from the
* pairing memalloc_nofs_save call.
*/
static inline void memalloc_nofs_restore(unsigned int flags)
_
Patches currently in -mm which might be from rdunlap@infradead.org are
linux-sched-mmh-drop-duplicated-words-in-comments.patch
^ permalink raw reply
* Re: [PATCH v3 0/3] Off-load TLB invalidations to host for !GTSE
From: Stephen Rothwell @ 2020-07-16 23:09 UTC (permalink / raw)
To: Qian Cai
Cc: Bharata B Rao, linuxppc-dev, aneesh.kumar, npiggin, mpe,
linux-kernel, linux-next
In-Reply-To: <20200716172713.GA4565@lca.pw>
[-- Attachment #1: Type: text/plain, Size: 308 bytes --]
Hi all,
On Thu, 16 Jul 2020 13:27:14 -0400 Qian Cai <cai@lca.pw> wrote:
>
> Reverting the whole series fixed random memory corruptions during boot on
> POWER9 PowerNV systems below.
I will revert those commits from linux-next today as well (they revert
cleanly).
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* + autofs-fix-doubled-word.patch added to -mm tree
From: Andrew Morton @ 2020-07-16 22:52 UTC (permalink / raw)
To: mm-commits, raven, rdunlap
In-Reply-To: <20200703151445.b6a0cfee402c7c5c4651f1b1@linux-foundation.org>
The patch titled
Subject: autofs: fix doubled word
has been added to the -mm tree. Its filename is
autofs-fix-doubled-word.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/autofs-fix-doubled-word.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/autofs-fix-doubled-word.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Randy Dunlap <rdunlap@infradead.org>
Subject: autofs: fix doubled word
Change doubled word "is" to "it is".
Link: http://lkml.kernel.org/r/5a82befd-40f8-8dc0-3498-cbc0436cad9b@infradead.org
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Ian Kent <raven@themaw.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/uapi/linux/auto_dev-ioctl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/include/uapi/linux/auto_dev-ioctl.h~autofs-fix-doubled-word
+++ a/include/uapi/linux/auto_dev-ioctl.h
@@ -82,7 +82,7 @@ struct args_ismountpoint {
/*
* All the ioctls use this structure.
* When sending a path size must account for the total length
- * of the chunk of memory otherwise is is the size of the
+ * of the chunk of memory otherwise it is the size of the
* structure.
*/
_
Patches currently in -mm which might be from rdunlap@infradead.org are
linux-sched-mmh-drop-duplicated-words-in-comments.patch
mm-drop-duplicated-words-in-linux-pgtableh.patch
mm-drop-duplicated-words-in-linux-mmh.patch
autofs-fix-doubled-word.patch
^ permalink raw reply
* + mm-drop-duplicated-words-in-linux-pgtableh.patch added to -mm tree
From: Andrew Morton @ 2020-07-16 22:51 UTC (permalink / raw)
To: mm-commits, rdunlap, sjpark
In-Reply-To: <20200703151445.b6a0cfee402c7c5c4651f1b1@linux-foundation.org>
The patch titled
Subject: mm: drop duplicated words in <linux/pgtable.h>
has been added to the -mm tree. Its filename is
mm-drop-duplicated-words-in-linux-pgtableh.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/mm-drop-duplicated-words-in-linux-pgtableh.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/mm-drop-duplicated-words-in-linux-pgtableh.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Randy Dunlap <rdunlap@infradead.org>
Subject: mm: drop duplicated words in <linux/pgtable.h>
Drop the doubled words "used" and "by".
Drop the repeated acronym "TLB" and make several other fixes around it.
(capital letters, spellos)
Link: http://lkml.kernel.org/r/2bb6e13e-44df-4920-52d9-4d3539945f73@infradead.org
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: SeongJae Park <sjpark@amazon.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/pgtable.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
--- a/include/linux/pgtable.h~mm-drop-duplicated-words-in-linux-pgtableh
+++ a/include/linux/pgtable.h
@@ -838,7 +838,7 @@ static inline void ptep_modify_prot_comm
/*
* No-op macros that just return the current protection value. Defined here
- * because these macros can be used used even if CONFIG_MMU is not defined.
+ * because these macros can be used even if CONFIG_MMU is not defined.
*/
#ifndef pgprot_encrypted
#define pgprot_encrypted(prot) (prot)
@@ -1231,7 +1231,7 @@ static inline int pmd_trans_unstable(pmd
* Technically a PTE can be PROTNONE even when not doing NUMA balancing but
* the only case the kernel cares is for NUMA balancing and is only ever set
* when the VMA is accessible. For PROT_NONE VMAs, the PTEs are not marked
- * _PAGE_PROTNONE so by by default, implement the helper as "always no". It
+ * _PAGE_PROTNONE so by default, implement the helper as "always no". It
* is the responsibility of the caller to distinguish between PROT_NONE
* protections and NUMA hinting fault protections.
*/
@@ -1315,10 +1315,10 @@ static inline int pmd_free_pte_page(pmd_
/*
* ARCHes with special requirements for evicting THP backing TLB entries can
* implement this. Otherwise also, it can help optimize normal TLB flush in
- * THP regime. stock flush_tlb_range() typically has optimization to nuke the
- * entire TLB TLB if flush span is greater than a threshold, which will
- * likely be true for a single huge page. Thus a single thp flush will
- * invalidate the entire TLB which is not desitable.
+ * THP regime. Stock flush_tlb_range() typically has optimization to nuke the
+ * entire TLB if flush span is greater than a threshold, which will
+ * likely be true for a single huge page. Thus a single THP flush will
+ * invalidate the entire TLB which is not desirable.
* e.g. see arch/arc: flush_pmd_tlb_range
*/
#define flush_pmd_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end)
_
Patches currently in -mm which might be from rdunlap@infradead.org are
linux-sched-mmh-drop-duplicated-words-in-comments.patch
mm-drop-duplicated-words-in-linux-pgtableh.patch
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: Add PORT0_PAD_CTRL properties
From: Vladimir Oltean @ 2020-07-16 22:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Matthew Hagan, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S. Miller, linux, netdev, linux-kernel, John Crispin,
Jonathan McDowell, Rob Herring, devicetree
In-Reply-To: <20200716150925.0f3e01b8@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote:
> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote:
> > Add names and decriptions of additional PORT0_PAD_CTRL properties.
> >
> > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> > ---
> > Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index ccbc6d89325d..3d34c4f2e891 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -13,6 +13,14 @@ Optional properties:
> >
> > - reset-gpios: GPIO to be used to reset the whole device
> >
> > +Optional MAC configuration properties:
> > +
> > +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6.
>
> Perhaps we can say a little more here?
>
> > +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to
> > + falling edge.
> > +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to
> > + falling edge.
>
> These are not something that other vendors may implement and therefore
> something we may want to make generic? Andrew?
>
It was asked before whether this device uses source-synchronous clock
for SGMII or if it recovers the clock from the data stream. Just "pass"
was given for a response.
https://patchwork.ozlabs.org/project/netdev/patch/8ddd76e484e1bedd12c87ea0810826b60e004a65.1591380105.git.noodles@earth.li/
One can, in principle, tell easily by examining schematics. If the SGMII
is only connected via RX_P, RX_N, TX_P, TX_N (and optionally there might
be external reference clocks for the SERDES lanes, but these are not
part of the data connection itself), then the clock is recovered from
the serial data stream, and we have no idea what "SGMII delays" are.
If the schematic shows 2 extra clock signals, one in each transmit
direction, then this is, in Russell King's words, "a new world of RGMII
delay pain but for SGMII". In principle I would fully expect clock skews
to be necessary for any high-speed protocol with source-synchronous
clocking. The problem, really, is that we aren't ready to deal with this
properly. We aren't distinguishing "SGMII with clock" from "SGMII
without clock" in any way. We have no idea who else is using such a
thing. Depending on the magnitude of this new world, it may be wise to
let these bindings go in as-is, or do something more kernel-wide...
One simple question to ask Matthew is what are you connecting to these
SGMII lanes, and if you need any special configuration on the other end
of those lanes too (and what is the configuration you are using on the
qca8k: enable the "SGMII delays" in both directions?).
> > Subnodes:
> >
> > The integrated switch subnode should be specified according to the binding
>
Thanks,
-Vladimir
^ permalink raw reply
* + mm-drop-duplicated-words-in-linux-mmh.patch added to -mm tree
From: Andrew Morton @ 2020-07-16 22:52 UTC (permalink / raw)
To: mm-commits, rdunlap, sjpark
In-Reply-To: <20200703151445.b6a0cfee402c7c5c4651f1b1@linux-foundation.org>
The patch titled
Subject: mm: drop duplicated words in <linux/mm.h>
has been added to the -mm tree. Its filename is
mm-drop-duplicated-words-in-linux-mmh.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/mm-drop-duplicated-words-in-linux-mmh.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/mm-drop-duplicated-words-in-linux-mmh.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Randy Dunlap <rdunlap@infradead.org>
Subject: mm: drop duplicated words in <linux/mm.h>
Drop the doubled words "to" and "the".
Link: http://lkml.kernel.org/r/d9fae8d6-0d60-4d52-9385-3199ee98de49@infradead.org
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: SeongJae Park <sjpark@amazon.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/mm.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/include/linux/mm.h~mm-drop-duplicated-words-in-linux-mmh
+++ a/include/linux/mm.h
@@ -482,7 +482,7 @@ static inline bool fault_flag_allow_retr
{ FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }
/*
- * vm_fault is filled by the the pagefault handler and passed to the vma's
+ * vm_fault is filled by the pagefault handler and passed to the vma's
* ->fault function. The vma's ->fault is responsible for returning a bitmask
* of VM_FAULT_xxx flags that give details about how the fault was handled.
*
@@ -2601,7 +2601,7 @@ extern unsigned long stack_guard_gap;
/* Generic expand stack which grows the stack according to GROWS{UP,DOWN} */
extern int expand_stack(struct vm_area_struct *vma, unsigned long address);
-/* CONFIG_STACK_GROWSUP still needs to to grow downwards at some places */
+/* CONFIG_STACK_GROWSUP still needs to grow downwards at some places */
extern int expand_downwards(struct vm_area_struct *vma,
unsigned long address);
#if VM_GROWSUP
_
Patches currently in -mm which might be from rdunlap@infradead.org are
linux-sched-mmh-drop-duplicated-words-in-comments.patch
mm-drop-duplicated-words-in-linux-pgtableh.patch
mm-drop-duplicated-words-in-linux-mmh.patch
^ permalink raw reply
* [Bug 14442] Shell command injection vulnerability in mount.cifs
From: samba-bugs @ 2020-07-16 22:40 UTC (permalink / raw)
To: cifs-qa
In-Reply-To: <bug-14442-10630@https.bugzilla.samba.org/>
https://bugzilla.samba.org/show_bug.cgi?id=14442
--- Comment #1 from Paulo Alcantara <palcantara@suse.de> ---
Hi Vadim,
Thanks for the report!
I was able to reproduce it and ended up with the following changes:
diff --git a/mount.cifs.c b/mount.cifs.c
index 40918c18649f..bb8a7e958898 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -1695,6 +1695,43 @@ drop_child_privs(void)
return 0;
}
+#ifdef ENABLE_SYSTEMD
+static int get_passwd_by_systemd(const char *prompt, char *input, int
capacity)
+{
+ int fd[2];
+ pid_t pid;
+ int rc;
+
+ if (pipe(fd) == -1) {
+ fprintf(stderr, "Failed to create pipe: %s\n",
strerror(errno));
+ return -1;
+ }
+
+ pid = fork();
+ if (pid == -1) {
+ fprintf(stderr, "Unable to fork: %s\n", strerror(errno));
+ return -1;
+ }
+
+ if (pid == 0) {
+ close(fd[0]);
+ dup2(fd[1], STDOUT_FILENO);
+ execlp("systemd-ask-password", "systemd-ask-password", prompt,
NULL);
+ }
+
+ close(fd[1]);
+ wait(&rc);
+ if (!WIFEXITED(rc))
+ return 1;
+ if (read(fd[0], input, capacity) == -1) {
+ fprintf(stderr, "Failed to read from pipe: %s\n",
strerror(errno));
+ return 1;
+ }
+
+ return 0;
+}
+#endif
+
/*
* If systemd is running and systemd-ask-password --
* is available, then use that else fallback on getpass(..)
@@ -1714,27 +1751,11 @@ get_password(const char *prompt, char *input, int
capacity)
&& (lstat("/sys/fs/cgroup/systemd", &b) == 0)
&& (a.st_dev != b.st_dev);
- if (is_systemd_running) {
- char *cmd, *ret;
- FILE *ask_pass_fp = NULL;
-
- cmd = ret = NULL;
- if (asprintf(&cmd, "systemd-ask-password \"%s\"", prompt) >= 0)
{
- ask_pass_fp = popen (cmd, "re");
- free (cmd);
- }
-
- if (ask_pass_fp) {
- ret = fgets(input, capacity, ask_pass_fp);
- pclose(ask_pass_fp);
- }
-
- if (ret) {
- int len = strlen(input);
- if (input[len - 1] == '\n')
- input[len - 1] = '\0';
- return input;
- }
+ if (is_systemd_running && !get_passwd_by_systemd(prompt, input,
capacity)) {
+ int len = strlen(input);
+ if (input[len - 1] == '\n')
+ input[len - 1] = '\0';
+ return input;
}
#endif
---
Before the patch:
$ sudo ./mount.cifs -o username="test \$(id)" //1 /mnt
Password for test uid=0(root) gid=0(root) groups=0(root)@//1: (press TAB for
no echo)
After the patch:
$ sudo ./mount.cifs -o username="test \$(id)" //1 /mnt
Password for test $(id)@//1: (press TAB for no echo)
Let me know what you think.
--
You are receiving this mail because:
You are the QA Contact for the bug.
^ permalink raw reply related
* Re: [PATCH bpf-next 2/5] bpf: allow for tailcalls in BPF subprograms
From: Daniel Borkmann @ 2020-07-16 22:46 UTC (permalink / raw)
To: Maciej Fijalkowski, ast; +Cc: bpf, netdev, bjorn.topel, magnus.karlsson
In-Reply-To: <912b5e97-1951-5a7c-e3c1-41bc3bf4b58a@iogearbox.net>
On 7/16/20 11:29 PM, Daniel Borkmann wrote:
> On 7/16/20 1:36 AM, Maciej Fijalkowski wrote:
>> Relax verifier's restriction that was meant to forbid tailcall usage
>> when subprog count was higher than 1.
>>
>> Also, do not max out the stack depth of program that utilizes tailcalls.
>>
>> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>> ---
>> kernel/bpf/verifier.c | 5 -----
>> 1 file changed, 5 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 3c1efc9d08fd..6481342b31ba 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -4172,10 +4172,6 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
>> case BPF_FUNC_tail_call:
>> if (map->map_type != BPF_MAP_TYPE_PROG_ARRAY)
>> goto error;
>> - if (env->subprog_cnt > 1) {
>> - verbose(env, "tail_calls are not allowed in programs with bpf-to-bpf calls\n");
>> - return -EINVAL;
>> - }
>> break;
>> case BPF_FUNC_perf_event_read:
>> case BPF_FUNC_perf_event_output:
>> @@ -10252,7 +10248,6 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
>> * the program array.
>> */
>> prog->cb_access = 1;
>> - env->prog->aux->stack_depth = MAX_BPF_STACK;
>> env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
>> /* mark bpf_tail_call as different opcode to avoid
>
> Also, isn't this broken when JIT is not used (as in stack oob access)?
(Similarly for non-x86 archs after this set.)
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: sc7180: Move the fixed-perm property to SoC dtsi
From: Doug Anderson @ 2020-07-16 22:37 UTC (permalink / raw)
To: Sibi Sankar
Cc: Bjorn Andersson, Andy Gross, linux-arm-msm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Evan Green, Rakesh Pillai, Rob Herring
In-Reply-To: <20200716191746.23196-1-sibis@codeaurora.org>
Hi,
On Thu, Jul 16, 2020 at 12:18 PM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> All the platforms using SC7180 SoC are expected to have the wlan firmware
> memory statically mapped by the Trusted Firmware. Hence move back the
> qcom,msa-fixed-perm property to the SoC dtsi.
>
> Fixes: 7d484566087c0 ("arm64: dts: qcom: sc7180: Add missing properties for Wifi node")
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 1 -
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 860fc4658b8b1..26cc4913d3ddc 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -392,7 +392,6 @@ video-firmware {
>
> &wifi {
> status = "okay";
> - qcom,msa-fixed-perm;
> vdd-0.8-cx-mx-supply = <&vreg_l9a_0p6>;
> vdd-1.8-xo-supply = <&vreg_l1c_1p8>;
> vdd-1.3-rfa-supply = <&vreg_l2c_1p3>;
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 126e2fce26c1a..a91d3f074625e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -3233,6 +3233,7 @@ wifi: wifi@18800000 {
> <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH /* CE10 */>,
> <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH /* CE11 */>;
> memory-region = <&wlan_mem>;
> + qcom,msa-fixed-perm;
Seems good to me. If someone ever comes up with a firmware where it's
sane to not have this property, we can either delete the property from
those boards or migrate the property to the the existing board dts
files.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* [Buildroot] [PATCH] package/alsa-lib: bump version to 1.2.3.2
From: Sergio Prado @ 2020-07-16 23:10 UTC (permalink / raw)
To: buildroot
Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
---
package/alsa-lib/alsa-lib.hash | 2 +-
package/alsa-lib/alsa-lib.mk | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/package/alsa-lib/alsa-lib.hash b/package/alsa-lib/alsa-lib.hash
index ca36d95cfce4..8a565e580b1a 100644
--- a/package/alsa-lib/alsa-lib.hash
+++ b/package/alsa-lib/alsa-lib.hash
@@ -1,4 +1,4 @@
# Locally calculated
-sha256 1244585515fbebce7d0e53656c1fb614f99accb8413115ce3efb9a9309c4c892 alsa-lib-1.2.3.1.tar.bz2
+sha256 e81fc5b7afcaee8c9fd7f64a1e3043e88d62e9ad2c4cff55f578df6b0a9abe15 alsa-lib-1.2.3.2.tar.bz2
sha256 32434afcc8666ba060e111d715bfdb6c2d5dd8a35fa4d3ab8ad67d8f850d2f2b COPYING
sha256 bfe16cf823bcff261fc6a062c07ee96660e3c39678f42f39a788a68dbc234ced aserver/COPYING
diff --git a/package/alsa-lib/alsa-lib.mk b/package/alsa-lib/alsa-lib.mk
index 383f06855d17..30f7c8815360 100644
--- a/package/alsa-lib/alsa-lib.mk
+++ b/package/alsa-lib/alsa-lib.mk
@@ -4,7 +4,7 @@
#
################################################################################
-ALSA_LIB_VERSION = 1.2.3.1
+ALSA_LIB_VERSION = 1.2.3.2
ALSA_LIB_SOURCE = alsa-lib-$(ALSA_LIB_VERSION).tar.bz2
ALSA_LIB_SITE = https://www.alsa-project.org/files/pub/lib
ALSA_LIB_LICENSE = LGPL-2.1+ (library), GPL-2.0+ (aserver)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 0/3] Fully describe the waveform for PTP periodic output
From: Vladimir Oltean @ 2020-07-16 22:45 UTC (permalink / raw)
To: kuba, davem, netdev
Cc: richardcochran, jacob.e.keller, yangbo.lu, xiaoliang.yang_1,
po.liu, UNGLinuxDriver
While using the ancillary pin functionality of PTP hardware clocks to
synchronize multiple DSA switches on a board, a need arised to be able
to configure the duty cycle of the master of this PPS hierarchy.
Also, the PPS master is not able to emit PPS starting from arbitrary
absolute times, so a new flag is introduced to support such hardware
without making guesses.
With these patches, struct ptp_perout_request now basically describes a
general-purpose square wave.
Changes in v2:
Made sure this applies to net-next.
Vladimir Oltean (3):
ptp: add ability to configure duty cycle for periodic output
ptp: introduce a phase offset in the periodic output request
net: mscc: ocelot: add support for PTP waveform configuration
drivers/net/ethernet/mscc/ocelot_ptp.c | 74 +++++++++++++++++---------
drivers/ptp/ptp_chardev.c | 33 +++++++++---
include/uapi/linux/ptp_clock.h | 34 ++++++++++--
3 files changed, 107 insertions(+), 34 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH v2 net-next 1/3] ptp: add ability to configure duty cycle for periodic output
From: Vladimir Oltean @ 2020-07-16 22:45 UTC (permalink / raw)
To: kuba, davem, netdev
Cc: richardcochran, jacob.e.keller, yangbo.lu, xiaoliang.yang_1,
po.liu, UNGLinuxDriver
In-Reply-To: <20200716224531.1040140-1-olteanv@gmail.com>
There are external event timestampers (PHCs with support for
PTP_EXTTS_REQUEST) that timestamp both event edges.
When those edges are very close (such as in the case of a short pulse),
there is a chance that the collected timestamp might be of the rising,
or of the falling edge, we never know.
There are also PHCs capable of generating periodic output with a
configurable duty cycle. This is good news, because we can space the
rising and falling edge out enough in time, that the risks to overrun
the 1-entry timestamp FIFO of the extts PHC are lower (example: the
perout PHC can be configured for a period of 1 second, and an "on" time
of 0.5 seconds, resulting in a duty cycle of 50%).
A flag is introduced for signaling that an on time is present in the
perout request structure, for preserving compatibility. Logically
speaking, the duty cycle cannot exceed 100% and the PTP core checks for
this.
PHC drivers that don't support this flag emit a periodic output of an
unspecified duty cycle, same as before.
The duty cycle is encoded as an "on" time, similar to the "start" and
"period" times, and reuses the reserved space while preserving overall
binary layout.
Pahole reported before:
struct ptp_perout_request {
struct ptp_clock_time start; /* 0 16 */
struct ptp_clock_time period; /* 16 16 */
unsigned int index; /* 32 4 */
unsigned int flags; /* 36 4 */
unsigned int rsv[4]; /* 40 16 */
/* size: 56, cachelines: 1, members: 5 */
/* last cacheline: 56 bytes */
};
And now:
struct ptp_perout_request {
struct ptp_clock_time start; /* 0 16 */
struct ptp_clock_time period; /* 16 16 */
unsigned int index; /* 32 4 */
unsigned int flags; /* 36 4 */
union {
struct ptp_clock_time on; /* 40 16 */
unsigned int rsv[4]; /* 40 16 */
}; /* 40 16 */
/* size: 56, cachelines: 1, members: 5 */
/* last cacheline: 56 bytes */
};
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes in v2:
None.
drivers/ptp/ptp_chardev.c | 33 +++++++++++++++++++++++++++------
include/uapi/linux/ptp_clock.h | 17 ++++++++++++++---
2 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 375cd6e4aade..e0e6f85966e1 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -191,12 +191,33 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
err = -EFAULT;
break;
}
- if (((req.perout.flags & ~PTP_PEROUT_VALID_FLAGS) ||
- req.perout.rsv[0] || req.perout.rsv[1] ||
- req.perout.rsv[2] || req.perout.rsv[3]) &&
- cmd == PTP_PEROUT_REQUEST2) {
- err = -EINVAL;
- break;
+ if (cmd == PTP_PEROUT_REQUEST2) {
+ struct ptp_perout_request *perout = &req.perout;
+
+ if (perout->flags & ~PTP_PEROUT_VALID_FLAGS) {
+ err = -EINVAL;
+ break;
+ }
+ /*
+ * The "on" field has undefined meaning if
+ * PTP_PEROUT_DUTY_CYCLE isn't set, we must still treat
+ * it as reserved, which must be set to zero.
+ */
+ if (!(perout->flags & PTP_PEROUT_DUTY_CYCLE) &&
+ (perout->rsv[0] || perout->rsv[1] ||
+ perout->rsv[2] || perout->rsv[3])) {
+ err = -EINVAL;
+ break;
+ }
+ if (perout->flags & PTP_PEROUT_DUTY_CYCLE) {
+ /* The duty cycle must be subunitary. */
+ if (perout->on.sec > perout->period.sec ||
+ (perout->on.sec == perout->period.sec &&
+ perout->on.nsec > perout->period.nsec)) {
+ err = -ERANGE;
+ break;
+ }
+ }
} else if (cmd == PTP_PEROUT_REQUEST) {
req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS;
req.perout.rsv[0] = 0;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index ff070aa64278..1d2841155f7d 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -53,12 +53,14 @@
/*
* Bits of the ptp_perout_request.flags field:
*/
-#define PTP_PEROUT_ONE_SHOT (1<<0)
+#define PTP_PEROUT_ONE_SHOT (1<<0)
+#define PTP_PEROUT_DUTY_CYCLE (1<<1)
/*
* flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl.
*/
-#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT)
+#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT | \
+ PTP_PEROUT_DUTY_CYCLE)
/*
* No flags are valid for the original PTP_PEROUT_REQUEST ioctl
@@ -105,7 +107,16 @@ struct ptp_perout_request {
struct ptp_clock_time period; /* Desired period, zero means disable. */
unsigned int index; /* Which channel to configure. */
unsigned int flags;
- unsigned int rsv[4]; /* Reserved for future use. */
+ union {
+ /*
+ * The "on" time of the signal.
+ * Must be lower than the period.
+ * Valid only if (flags & PTP_PEROUT_DUTY_CYCLE) is set.
+ */
+ struct ptp_clock_time on;
+ /* Reserved for future use. */
+ unsigned int rsv[4];
+ };
};
#define PTP_MAX_SAMPLES 25 /* Maximum allowed offset measurement samples. */
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 2/3] ptp: introduce a phase offset in the periodic output request
From: Vladimir Oltean @ 2020-07-16 22:45 UTC (permalink / raw)
To: kuba, davem, netdev
Cc: richardcochran, jacob.e.keller, yangbo.lu, xiaoliang.yang_1,
po.liu, UNGLinuxDriver
In-Reply-To: <20200716224531.1040140-1-olteanv@gmail.com>
Some PHCs like the ocelot/felix switch cannot emit generic periodic
output, but just PPS (pulse per second) signals, which:
- don't start from arbitrary absolute times, but are rather
phase-aligned to the beginning of [the closest next] second.
- have an optional phase offset relative to that beginning of the
second.
For those, it was initially established that they should reject any
other absolute time for the PTP_PEROUT_REQUEST than 0.000000000 [1].
But when it actually came to writing an application [2] that makes use
of this functionality, we realized that we can't really deal generically
with PHCs that support absolute start time, and with PHCs that don't,
without an explicit interface. Namely, in an ideal world, PHC drivers
would ensure that the "perout.start" value written to hardware will
result in a functional output. This means that if the PTP time has
become in the past of this PHC's current time, it should be
automatically fast-forwarded by the driver into a close enough future
time that is known to work (note: this is necessary only if the hardware
doesn't do this fast-forward by itself). But we don't really know what
is the status for PHC drivers in use today, so in the general sense,
user space would be risking to have a non-functional periodic output if
it simply asked for a start time of 0.000000000.
So let's introduce a flag for this type of reduced-functionality
hardware, named PTP_PEROUT_PHASE. The start time is just "soon", the
only thing we know for sure about this signal is that its rising edge
events, Rn, occur at:
Rn = perout.phase + n * perout.period
The "phase" in the periodic output structure is simply an alias to the
"start" time, since both cannot logically be specified at the same time.
Therefore, the binary layout of the structure is not affected.
[1]: https://patchwork.ozlabs.org/project/netdev/patch/20200320103726.32559-7-yangbo.lu@nxp.com/
[2]: https://www.mail-archive.com/linuxptp-devel@lists.sourceforge.net/msg04142.html
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes in v2:
Typo in commit message: period -> perout
include/uapi/linux/ptp_clock.h | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 1d2841155f7d..1d108d597f66 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -55,12 +55,14 @@
*/
#define PTP_PEROUT_ONE_SHOT (1<<0)
#define PTP_PEROUT_DUTY_CYCLE (1<<1)
+#define PTP_PEROUT_PHASE (1<<2)
/*
* flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl.
*/
#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT | \
- PTP_PEROUT_DUTY_CYCLE)
+ PTP_PEROUT_DUTY_CYCLE | \
+ PTP_PEROUT_PHASE)
/*
* No flags are valid for the original PTP_PEROUT_REQUEST ioctl
@@ -103,7 +105,20 @@ struct ptp_extts_request {
};
struct ptp_perout_request {
- struct ptp_clock_time start; /* Absolute start time. */
+ union {
+ /*
+ * Absolute start time.
+ * Valid only if (flags & PTP_PEROUT_PHASE) is unset.
+ */
+ struct ptp_clock_time start;
+ /*
+ * Phase offset. The signal should start toggling at an
+ * unspecified integer multiple of the period, plus this value.
+ * The start time should be "as soon as possible".
+ * Valid only if (flags & PTP_PEROUT_PHASE) is set.
+ */
+ struct ptp_clock_time phase;
+ };
struct ptp_clock_time period; /* Desired period, zero means disable. */
unsigned int index; /* Which channel to configure. */
unsigned int flags;
--
2.25.1
^ permalink raw reply related
* Re: Compressing packed-refs
From: Konstantin Ryabitsev @ 2020-07-16 22:54 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqsgdrf64c.fsf@gitster.c.googlers.com>
On Thu, Jul 16, 2020 at 03:27:15PM -0700, Junio C Hamano wrote:
> I think the reftable is the longer term direction, but let's see if
> there is easy enough optimization opportunity that we can afford the
> development and maintenance cost for the short term.
>
> My .git/packed-refs file begins like so:
>
> # pack-refs with: peeled fully-peeled sorted
> c3808ca6982b0ad7ee9b87eca9b50b9a24ec08b0 refs/heads/maint-2.10
> 3b9e3c2cede15057af3ff8076c45ad5f33829436 refs/heads/maint-2.11
> 584f8975d2d9530a34bd0b936ae774f82fe30fed refs/heads/master
> 2cccc8116438182c988c7f26d9559a1c22e78f1c refs/heads/next
> 8300349bc1f0a0e2623d5824266bd72c1f4b5f24 refs/notes/commits
> ...
Let me offer a more special-case (but not crazy) example from
git.kernel.org. The newer version of grokmirror that I'm working on is
built to take advantage of the pack-islands feature that was added a
while back. We fetch all linux forks into a single "object storage"
repo, with each fork going into its own
refs/virtual/[uniquename]/(heads|tags) place. This means there's lots of
duplicates in packed-refs, as all the tags from torvalds/linux.git will
end up duplicated in almost every fork.
So, after running git pack-refs --all, the packed-refs file is 50-ish MB
in size, with a lot of same stuff like:
5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/virtual/00018460b026/tags/v2.6.11
^c39ae07f393806ccf406ef966e9a15afc43cc36a
...
5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/virtual/00bcef8138af/tags/v2.6.11
^c39ae07f393806ccf406ef966e9a15afc43cc36a
etc, duplicated 600 times with each fork. It compresses decently well
with gzip -9, and *amazingly* well with xz -9:
$ ls -ahl packed-refs
-rw-r--r--. 1 mirror mirror 46M Jul 16 22:37 packed-refs
$ ls -ahl packed-refs.gz
-rw-r--r--. 1 mirror mirror 19M Jul 16 22:47 packed-refs.gz
$ ls -ahl packed-refs.xz
-rw-r--r--. 1 mirror mirror 2.3M Jul 16 22:47 packed-refs.xz
Which really just indicates how much duplicated data is in that file. If
reftables will eventually replace refs entirely, then we probably
shouldn't expend too much effort super-optimizing it, especially if I'm
one of the very few people who would benefit from it. However, I'm
curious if a different sorting strategy would help remove most of the
duplication without requiring too much engineering time.
-K
^ permalink raw reply
* [PATCH v2 net-next 3/3] net: mscc: ocelot: add support for PTP waveform configuration
From: Vladimir Oltean @ 2020-07-16 22:45 UTC (permalink / raw)
To: kuba, davem, netdev
Cc: richardcochran, jacob.e.keller, yangbo.lu, xiaoliang.yang_1,
po.liu, UNGLinuxDriver
In-Reply-To: <20200716224531.1040140-1-olteanv@gmail.com>
For PPS output (perout period is 1.000000000), accept the new "phase"
parameter from the periodic output request structure.
For both PPS and freeform output, accept the new "on" argument for
specifying the duty cycle of the generated signal. Preserve the old
defaults for this "on" time: 1 us for PPS, and half the period for
freeform output.
Also preserve the old behavior that accepted the "phase" via the "start"
argument.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes in v2:
Made sure it applies to net-next.
drivers/net/ethernet/mscc/ocelot_ptp.c | 74 +++++++++++++++++---------
1 file changed, 50 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c b/drivers/net/ethernet/mscc/ocelot_ptp.c
index a3088a1676ed..1e08fe4daaef 100644
--- a/drivers/net/ethernet/mscc/ocelot_ptp.c
+++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
@@ -184,18 +184,20 @@ int ocelot_ptp_enable(struct ptp_clock_info *ptp,
struct ptp_clock_request *rq, int on)
{
struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
- struct timespec64 ts_start, ts_period;
+ struct timespec64 ts_phase, ts_period;
enum ocelot_ptp_pins ptp_pin;
unsigned long flags;
bool pps = false;
int pin = -1;
+ s64 wf_high;
+ s64 wf_low;
u32 val;
- s64 ns;
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
/* Reject requests with unsupported flags */
- if (rq->perout.flags)
+ if (rq->perout.flags & ~(PTP_PEROUT_DUTY_CYCLE |
+ PTP_PEROUT_PHASE))
return -EOPNOTSUPP;
pin = ptp_find_pin(ocelot->ptp_clock, PTP_PF_PEROUT,
@@ -211,22 +213,12 @@ int ocelot_ptp_enable(struct ptp_clock_info *ptp,
else
return -EBUSY;
- ts_start.tv_sec = rq->perout.start.sec;
- ts_start.tv_nsec = rq->perout.start.nsec;
ts_period.tv_sec = rq->perout.period.sec;
ts_period.tv_nsec = rq->perout.period.nsec;
if (ts_period.tv_sec == 1 && ts_period.tv_nsec == 0)
pps = true;
- if (ts_start.tv_sec || (ts_start.tv_nsec && !pps)) {
- dev_warn(ocelot->dev,
- "Absolute start time not supported!\n");
- dev_warn(ocelot->dev,
- "Accept nsec for PPS phase adjustment, otherwise start time should be 0 0.\n");
- return -EINVAL;
- }
-
/* Handle turning off */
if (!on) {
spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
@@ -236,16 +228,48 @@ int ocelot_ptp_enable(struct ptp_clock_info *ptp,
break;
}
+ if (rq->perout.flags & PTP_PEROUT_PHASE) {
+ ts_phase.tv_sec = rq->perout.phase.sec;
+ ts_phase.tv_nsec = rq->perout.phase.nsec;
+ } else {
+ /* Compatibility */
+ ts_phase.tv_sec = rq->perout.start.sec;
+ ts_phase.tv_nsec = rq->perout.start.nsec;
+ }
+ if (ts_phase.tv_sec || (ts_phase.tv_nsec && !pps)) {
+ dev_warn(ocelot->dev,
+ "Absolute start time not supported!\n");
+ dev_warn(ocelot->dev,
+ "Accept nsec for PPS phase adjustment, otherwise start time should be 0 0.\n");
+ return -EINVAL;
+ }
+
+ /* Calculate waveform high and low times */
+ if (rq->perout.flags & PTP_PEROUT_DUTY_CYCLE) {
+ struct timespec64 ts_on;
+
+ ts_on.tv_sec = rq->perout.on.sec;
+ ts_on.tv_nsec = rq->perout.on.nsec;
+
+ wf_high = timespec64_to_ns(&ts_on);
+ } else {
+ if (pps) {
+ wf_high = 1000;
+ } else {
+ wf_high = timespec64_to_ns(&ts_period);
+ wf_high = div_s64(wf_high, 2);
+ }
+ }
+
+ wf_low = timespec64_to_ns(&ts_period);
+ wf_low -= wf_high;
+
/* Handle PPS request */
if (pps) {
spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
- /* Pulse generated perout.start.nsec after TOD has
- * increased seconds.
- * Pulse width is set to 1us.
- */
- ocelot_write_rix(ocelot, ts_start.tv_nsec,
+ ocelot_write_rix(ocelot, ts_phase.tv_nsec,
PTP_PIN_WF_LOW_PERIOD, ptp_pin);
- ocelot_write_rix(ocelot, 1000,
+ ocelot_write_rix(ocelot, wf_high,
PTP_PIN_WF_HIGH_PERIOD, ptp_pin);
val = PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK);
val |= PTP_PIN_CFG_SYNC;
@@ -255,14 +279,16 @@ int ocelot_ptp_enable(struct ptp_clock_info *ptp,
}
/* Handle periodic clock */
- ns = timespec64_to_ns(&ts_period);
- ns = ns >> 1;
- if (ns > 0x3fffffff || ns <= 0x6)
+ if (wf_high > 0x3fffffff || wf_high <= 0x6)
+ return -EINVAL;
+ if (wf_low > 0x3fffffff || wf_low <= 0x6)
return -EINVAL;
spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
- ocelot_write_rix(ocelot, ns, PTP_PIN_WF_LOW_PERIOD, ptp_pin);
- ocelot_write_rix(ocelot, ns, PTP_PIN_WF_HIGH_PERIOD, ptp_pin);
+ ocelot_write_rix(ocelot, wf_low, PTP_PIN_WF_LOW_PERIOD,
+ ptp_pin);
+ ocelot_write_rix(ocelot, wf_high, PTP_PIN_WF_HIGH_PERIOD,
+ ptp_pin);
val = PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK);
ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ptp_pin);
spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
--
2.25.1
^ permalink raw reply related
* Re: [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task_work
From: Peter Zijlstra @ 2020-07-16 22:54 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, x86, Oleg Nesterov, Eric W. Biederman, Frederic Weisbecker,
John Stultz, Paolo Bonzini
In-Reply-To: <20200716202044.734067877@linutronix.de>
On Thu, Jul 16, 2020 at 10:19:26PM +0200, Thomas Gleixner wrote:
> +static void __run_posix_cpu_timers(struct task_struct *tsk)
> +{
> + struct posix_cputimers *pct = &tsk->posix_cputimers;
> +
> + if (!test_and_set_bit(CPUTIMERS_WORK_SCHEDULED, &pct->flags))
> + task_work_add(tsk, &pct->task_work, true);
s/true/TWA_RESUME/g
see: e91b48162332 ("task_work: teach task_work_add() to do signal_wake_up()")
> +}
^ permalink raw reply
* [FYI] commit-graph: trace expiry of commit graph links
From: Jonathan Tan @ 2020-07-16 22:57 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, stolee
In-Reply-To: <92b218ca-f2b4-db9d-9f9c-1d071ea4fa97@gmail.com>
When an obsolete link in the commit graph chain is deleted, and trace2
is enabled, trace a message that it is deleted, along with the list of
links before and after the current chain refresh.
The messages are emitted using trace2_data_string() and not
trace2_printf() because the latter is not implemented for some trace
targets.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
This patch is just for informational purposes for people who have the
same problem and/or want to help diagnose.
I've tested that this produces the expected trace2 messages using:
git commit-graph write --reachable --split=replace
> It is _possible_ that something like a case switch or a symlink
> could be causing a problem here. That's where I would look on
> the affected systems.
Indeed a symlink is present - the affected repositories are using the
git-repo [1] local mirror feature, which causes .git/objects (among
other things) to be a symlink - but I couldn't figure out how this
symlink would cause problems. In particular, looking at the code, all
relevant filenames seem to be constructed from ctx->odb->path, so no
matter which names were used to get to the object directory, all names
are built from those names, and this seems to be true in practice as
well.
So I've added some trace2 messages (in this patch), and let's see if I
can figure out what's going on. I'll write back if I find something.
[1] https://gerrit.googlesource.com/git-repo
---
commit-graph.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index 328ab06fd4..b5bd2ac6de 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2009,6 +2009,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
struct dirent *de;
size_t dirnamelen;
timestamp_t expire_time = time(NULL);
+ int commit_graph_deleted = 0;
if (ctx->split_opts && ctx->split_opts->expire_time)
expire_time = ctx->split_opts->expire_time;
@@ -2050,8 +2051,38 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
}
}
- if (!found)
+ if (!found) {
+ if (trace2_is_enabled()) {
+ struct strbuf message = STRBUF_INIT;
+
+ strbuf_addf(&message, "Deleting '%s' because it is not in [", path.buf);
+ for (i = 0; i < ctx->num_commit_graphs_after; i++) {
+ if (i != 0)
+ strbuf_addstr(&message, ", ");
+ strbuf_addf(&message, "'%s'", ctx->commit_graph_filenames_after[i]);
+ }
+ strbuf_addstr(&message, "]");
+ trace2_data_string("commit-graph", the_repository, "graph-deletion", message.buf);
+ strbuf_release(&message);
+ commit_graph_deleted = 1;
+ }
unlink(path.buf);
+ }
+ }
+
+ if (commit_graph_deleted) {
+ struct strbuf message = STRBUF_INIT;
+ uint32_t i;
+
+ strbuf_addstr(&message, "The commit graphs before were [");
+ for (i = 0; i < ctx->num_commit_graphs_before; i++) {
+ if (i != 0)
+ strbuf_addstr(&message, ", ");
+ strbuf_addf(&message, "'%s'", ctx->commit_graph_filenames_before[i]);
+ }
+ strbuf_addstr(&message, "]");
+ trace2_data_string("commit-graph", the_repository, "graph-before", message.buf);
+ strbuf_release(&message);
}
out:
--
2.28.0.rc0.105.gf9edc3c819-goog
^ permalink raw reply related
* [PATCH] iommu/arm-smmu-v3: remove the approach of MSI polling for CMD SYNC
From: Barry Song @ 2020-07-16 23:07 UTC (permalink / raw)
To: robin.murphy, will, joro
Cc: linux-kernel, linuxarm, linux-arm-kernel, iommu, Barry Song,
Prime Zeng
Before commit 587e6c10a7ce ("iommu/arm-smmu-v3: Reduce contention during
command-queue insertion"), msi polling perhaps performed better since
it could run outside the spin_lock_irqsave() while the code polling cons
reg was running in the lock.
But after the great reorganization of smmu queue, neither of these two
polling methods are running in a spinlock. And real tests show polling
cons reg via sev means smaller latency. It is probably because polling
by msi will ask hardware to write memory but sev polling depends on the
update of register only.
Using 16 threads to run netperf on hns3 100G NIC with UDP packet size
in 32768bytes and set iommu to strict, TX throughput can improve from
25227.74Mbps to 27145.59Mbps by this patch. In this case, SMMU is super
busy as hns3 sends map/unmap requests extremely frequently.
Cc: Prime Zeng <prime.zeng@hisilicon.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
drivers/iommu/arm-smmu-v3.c | 46 +------------------------------------
1 file changed, 1 insertion(+), 45 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f578677a5c41..e55282a636c8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -964,12 +964,7 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
cmd[1] |= FIELD_PREP(CMDQ_PRI_1_RESP, ent->pri.resp);
break;
case CMDQ_OP_CMD_SYNC:
- if (ent->sync.msiaddr) {
- cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_IRQ);
- cmd[1] |= ent->sync.msiaddr & CMDQ_SYNC_1_MSIADDR_MASK;
- } else {
- cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
- }
+ cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_MSH, ARM_SMMU_SH_ISH);
cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_MSIATTR, ARM_SMMU_MEMATTR_OIWB);
break;
@@ -983,21 +978,10 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
u32 prod)
{
- struct arm_smmu_queue *q = &smmu->cmdq.q;
struct arm_smmu_cmdq_ent ent = {
.opcode = CMDQ_OP_CMD_SYNC,
};
- /*
- * Beware that Hi16xx adds an extra 32 bits of goodness to its MSI
- * payload, so the write will zero the entire command on that platform.
- */
- if (smmu->features & ARM_SMMU_FEAT_MSI &&
- smmu->features & ARM_SMMU_FEAT_COHERENCY) {
- ent.sync.msiaddr = q->base_dma + Q_IDX(&q->llq, prod) *
- q->ent_dwords * 8;
- }
-
arm_smmu_cmdq_build_cmd(cmd, &ent);
}
@@ -1251,30 +1235,6 @@ static int arm_smmu_cmdq_poll_until_not_full(struct arm_smmu_device *smmu,
return ret;
}
-/*
- * Wait until the SMMU signals a CMD_SYNC completion MSI.
- * Must be called with the cmdq lock held in some capacity.
- */
-static int __arm_smmu_cmdq_poll_until_msi(struct arm_smmu_device *smmu,
- struct arm_smmu_ll_queue *llq)
-{
- int ret = 0;
- struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = &smmu->cmdq;
- u32 *cmd = (u32 *)(Q_ENT(&cmdq->q, llq->prod));
-
- queue_poll_init(smmu, &qp);
-
- /*
- * The MSI won't generate an event, since it's being written back
- * into the command queue.
- */
- qp.wfe = false;
- smp_cond_load_relaxed(cmd, !VAL || (ret = queue_poll(&qp)));
- llq->cons = ret ? llq->prod : queue_inc_prod_n(llq, 1);
- return ret;
-}
-
/*
* Wait until the SMMU cons index passes llq->prod.
* Must be called with the cmdq lock held in some capacity.
@@ -1332,10 +1292,6 @@ static int __arm_smmu_cmdq_poll_until_consumed(struct arm_smmu_device *smmu,
static int arm_smmu_cmdq_poll_until_sync(struct arm_smmu_device *smmu,
struct arm_smmu_ll_queue *llq)
{
- if (smmu->features & ARM_SMMU_FEAT_MSI &&
- smmu->features & ARM_SMMU_FEAT_COHERENCY)
- return __arm_smmu_cmdq_poll_until_msi(smmu, llq);
-
return __arm_smmu_cmdq_poll_until_consumed(smmu, llq);
}
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] powerpc/64: Fix an out of date comment about MMIO ordering
From: Benjamin Herrenschmidt @ 2020-07-16 22:38 UTC (permalink / raw)
To: Palmer Dabbelt, Will Deacon
Cc: mpe, paulus, npiggin, msuchanek, tglx, bigeasy, jniethe5,
linuxppc-dev, linux-kernel, kernel-team, Palmer Dabbelt
In-Reply-To: <20200716193820.1141936-1-palmer@dabbelt.com>
On Thu, 2020-07-16 at 12:38 -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <palmerdabbelt@google.com>
>
> This primitive has been renamed, but because it was spelled incorrectly in the
> first place it must have escaped the fixup patch. As far as I can tell this
> logic is still correct: smp_mb__after_spinlock() uses the default smp_mb()
> implementation, which is "sync" rather than "hwsync" but those are the same
> (though I'm not that familiar with PowerPC).
Typo ? That must be me ... :)
Looks fine. Yes, sync and hwsync are the same (by opposition to lwsync
which is lighter weight and doesn't order cache inhibited).
Cheers,
Ben.
> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
> ---
> arch/powerpc/kernel/entry_64.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index b3c9f15089b6..7b38b4daca93 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -357,7 +357,7 @@ _GLOBAL(_switch)
> * kernel/sched/core.c).
> *
> * Uncacheable stores in the case of involuntary preemption must
> - * be taken care of. The smp_mb__before_spin_lock() in __schedule()
> + * be taken care of. The smp_mb__after_spinlock() in __schedule()
> * is implemented as hwsync on powerpc, which orders MMIO too. So
> * long as there is an hwsync in the context switch path, it will
> * be executed on the source CPU after the task has performed
^ permalink raw reply
* Re: [PATCH] dt-bindings: uniphier-thermal: add minItems to socionext,tmod-calibration
From: Rob Herring @ 2020-07-16 23:04 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Frank Rowand, devicetree, Amit Kucheria, Daniel Lezcano,
Kunihiko Hayashi, Zhang Rui, linux-arm-kernel, linux-kernel,
linux-pm
In-Reply-To: <20200707102338.989660-1-yamada.masahiro@socionext.com>
On Tue, Jul 07, 2020 at 07:23:38PM +0900, Masahiro Yamada wrote:
> As the description says, this property contains a pair of calibration
> values. The number of items must be exactly 2.
>
> Add minItems to check a too short property.
>
> While I was here, I also added this property to the example because
> this is the case in the real DT file,
> arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
>
> Also, fix the interrupt type (edge -> level) to align with the
> real DT.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> .../bindings/thermal/socionext,uniphier-thermal.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/socionext,uniphier-thermal.yaml b/Documentation/devicetree/bindings/thermal/socionext,uniphier-thermal.yaml
> index 553c9dcdaeeb..57ffd0c4c474 100644
> --- a/Documentation/devicetree/bindings/thermal/socionext,uniphier-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/socionext,uniphier-thermal.yaml
> @@ -29,6 +29,7 @@ properties:
>
> socionext,tmod-calibration:
> $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 2
The intent was if minItems is not defined, then the default is the same
as maxItems. This is not the default for json-schema, so the tooling is
supposed to add it. But looking at processed-schema.yaml, it doesn't
seem to be happening for one case here. I'm working on a fix in the
tools.
Rob
^ permalink raw reply
* Re: [PATCH bpf-next 4/5] bpf, x64: rework pro/epilogue and tailcall handling in JIT
From: Daniel Borkmann @ 2020-07-16 23:06 UTC (permalink / raw)
To: Maciej Fijalkowski, ast; +Cc: bpf, netdev, bjorn.topel, magnus.karlsson
In-Reply-To: <20200715233634.3868-5-maciej.fijalkowski@intel.com>
On 7/16/20 1:36 AM, Maciej Fijalkowski wrote:
> This commit serves two things:
> 1) it optimizes BPF prologue/epilogue generation
> 2) it makes possible to have tailcalls within BPF subprogram
>
> Both points are related to each other since without 1), 2) could not be
> achieved.
>
> In [1], Alexei says:
> "The prologue will look like:
> nop5
> xor eax,eax // two new bytes if bpf_tail_call() is used in this
> // function
> push rbp
> mov rbp, rsp
> sub rsp, rounded_stack_depth
> push rax // zero init tail_call counter
> variable number of push rbx,r13,r14,r15
>
> Then bpf_tail_call will pop variable number rbx,..
> and final 'pop rax'
> Then 'add rsp, size_of_current_stack_frame'
> jmp to next function and skip over 'nop5; xor eax,eax; push rpb; mov
> rbp, rsp'
>
> This way new function will set its own stack size and will init tail
> call
> counter with whatever value the parent had.
>
> If next function doesn't use bpf_tail_call it won't have 'xor eax,eax'.
> Instead it would need to have 'nop2' in there."
>
> Implement that suggestion.
>
> Since the layout of stack is changed, tail call counter handling can not
> rely anymore on popping it to rbx just like it have been handled for
> constant prologue case and later overwrite of rbx with actual value of
> rbx pushed to stack. Therefore, let's use one of the register (%rcx) that
> is considered to be volatile/caller-saved and pop the value of tail call
> counter in there in the epilogue.
>
> Drop the BUILD_BUG_ON in emit_prologue and in
> emit_bpf_tail_call_indirect where instruction layout is not constant
> anymore.
>
> Introduce new poke target, 'tailcall_bypass' to poke descriptor that is
> dedicated for skipping the register pops and stack unwind that are
> generated right before the actual jump to target program. Reflect also
> the actual purpose of poke->ip and rename it to poke->tailcall_target so
> that it will not the be confused with the poke target that is being
> introduced here.
> For case when the target program is not present, BPF program will skip
> the pop instructions and nop5 dedicated for jmpq $target. An example of
> such state when only R6 of callee saved registers is used by program:
>
> ffffffffc0513aa1: e9 0e 00 00 00 jmpq 0xffffffffc0513ab4
> ffffffffc0513aa6: 5b pop %rbx
> ffffffffc0513aa7: 58 pop %rax
> ffffffffc0513aa8: 48 81 c4 00 00 00 00 add $0x0,%rsp
> ffffffffc0513aaf: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
> ffffffffc0513ab4: 48 89 df mov %rbx,%rdi
>
> When target program is inserted, the jump that was there to skip
> pops/nop5 will become the nop5, so CPU will go over pops and do the
> actual tailcall.
>
> One might ask why there simply can not be pushes after the nop5?
> In the following example snippet:
>
> ffffffffc037030c: 48 89 fb mov %rdi,%rbx
> (...)
> ffffffffc0370332: 5b pop %rbx
> ffffffffc0370333: 58 pop %rax
> ffffffffc0370334: 48 81 c4 00 00 00 00 add $0x0,%rsp
> ffffffffc037033b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
> ffffffffc0370340: 48 81 ec 00 00 00 00 sub $0x0,%rsp
> ffffffffc0370347: 50 push %rax
> ffffffffc0370348: 53 push %rbx
> ffffffffc0370349: 48 89 df mov %rbx,%rdi
> ffffffffc037034c: e8 f7 21 00 00 callq 0xffffffffc0372548
>
> There is the bpf2bpf call (at ffffffffc037034c) right after the tailcall
> and jump target is not present. ctx is in %rbx register and BPF
> subprogram that we will call into on ffffffffc037034c is relying on it,
> e.g. it will pick ctx from there. Such code layout is therefore broken
> as we would overwrite the content of %rbx with the value that was pushed
> on the prologue. That is the reason for the 'bypass' approach.
>
> Special care needs to be taken during the install/update/remove of
> tailcall target. In case when target program is not present, the CPU
> must not execute the pop instructions that precede the tailcall.
>
> To address that, the following states can be defined:
> A nop, unwind, nop
> B nop, unwind, tail
> C skip, unwind, nop
> D skip, unwind, tail
>
> A is forbidden (lead to incorrectness). The state transitions between
> tailcall install/update/remove will work as follows:
>
> First install tail call f: C->D->B(f)
> * poke the tailcall, after that get rid of the skip
> Update tail call f to f': B(f)->B(f')
> * poke the tailcall (poke->tailcall_target) and do NOT touch the
> poke->tailcall_bypass
> Remove tail call: B(f')->C(f')
> * poke->tailcall_bypass is poked back to jump, then we wait the RCU
> grace period so that other programs will finish its execution and
> after that we are safe to remove the poke->tailcall_target
> Install new tail call (f''): C(f')->D(f'')->B(f'').
> * same as first step
>
> This way CPU can never be exposed to "unwind, tail" state.
>
> For regression checks, 'tailcalls' kselftest was executed:
> $ sudo ./test_progs -t tailcalls
> #64/1 tailcall_1:OK
> #64/2 tailcall_2:OK
> #64/3 tailcall_3:OK
> #64/4 tailcall_4:OK
> #64/5 tailcall_5:OK
> #64 tailcalls:OK
> Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
>
> Tail call related cases from test_verifier kselftest are also working
> fine. Sample BPF programs that utilize tail calls (sockex3, tracex5)
> work properly as well.
>
> [1]: https://lore.kernel.org/bpf/20200517043227.2gpq22ifoq37ogst@ast-mbp.dhcp.thefacebook.com/
>
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Overall approach looks reasonable to me. The patch here could still be cleaned up a
bit further, still very rough. Just minor comments below:
> ---
> arch/x86/net/bpf_jit_comp.c | 241 +++++++++++++++++++++++++++---------
> include/linux/bpf.h | 8 +-
> kernel/bpf/arraymap.c | 61 +++++++--
> kernel/bpf/core.c | 3 +-
> 4 files changed, 239 insertions(+), 74 deletions(-)
>
[...]
> /*
> - * Emit x86-64 prologue code for BPF program and check its size.
> + * Emit x86-64 prologue code for BPF program.
> * bpf_tail_call helper will skip it while jumping into another program
> */
> -static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf)
> +static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf,
> + bool tail_call)
> {
> u8 *prog = *pprog;
> int cnt = X86_PATCH_SIZE;
> @@ -238,19 +269,16 @@ static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf)
> */
> memcpy(prog, ideal_nops[NOP_ATOMIC5], cnt);
> prog += cnt;
> + if (!ebpf_from_cbpf && tail_call)
> + EMIT2(0x31, 0xC0); /* xor eax, eax */
> + else
> + EMIT2(0x66, 0x90); /* nop2 */
nit: Why does the ebpf_from_cbpf need the extra nop?
> EMIT1(0x55); /* push rbp */
> EMIT3(0x48, 0x89, 0xE5); /* mov rbp, rsp */
> /* sub rsp, rounded_stack_depth */
> EMIT3_off32(0x48, 0x81, 0xEC, round_up(stack_depth, 8));
> - EMIT1(0x53); /* push rbx */
> - EMIT2(0x41, 0x55); /* push r13 */
> - EMIT2(0x41, 0x56); /* push r14 */
> - EMIT2(0x41, 0x57); /* push r15 */
> - if (!ebpf_from_cbpf) {
> - /* zero init tail_call_cnt */
> - EMIT2(0x6a, 0x00);
> - BUILD_BUG_ON(cnt != PROLOGUE_SIZE);
> - }
> + if (!ebpf_from_cbpf && tail_call)
> + EMIT1(0x50); /* push rax */
> *pprog = prog;
> }
>
[...]
> -static void emit_bpf_tail_call_indirect(u8 **pprog)
> +static void emit_bpf_tail_call_indirect(u8 **pprog, bool *callee_regs_used,
> + u32 stack_depth)
> {
> u8 *prog = *pprog;
> - int label1, label2, label3;
> + int pop_bytes = 0;
> + int off1 = 49;
> + int off2 = 38;
> + int off3 = 16;
> int cnt = 0;
>
> + /* count the additional bytes used for popping callee regs from stack
> + * that need to be taken into account for each of the offsets that
> + * are used for bailing out of the tail call
> + */
> + pop_bytes = get_pop_bytes(callee_regs_used);
> + off1 += pop_bytes;
> + off2 += pop_bytes;
> + off3 += pop_bytes;
> +
> /*
> * rdi - pointer to ctx
> * rsi - pointer to bpf_array
> @@ -370,72 +427,108 @@ static void emit_bpf_tail_call_indirect(u8 **pprog)
> EMIT2(0x89, 0xD2); /* mov edx, edx */
> EMIT3(0x39, 0x56, /* cmp dword ptr [rsi + 16], edx */
> offsetof(struct bpf_array, map.max_entries));
> -#define OFFSET1 (41 + RETPOLINE_RAX_BPF_JIT_SIZE) /* Number of bytes to jump */
> +#define OFFSET1 (off1 + RETPOLINE_RCX_BPF_JIT_SIZE) /* Number of bytes to jump */
The whole rename belongs into the first patch to avoid breaking bisectability
as mentioned.
> EMIT2(X86_JBE, OFFSET1); /* jbe out */
> - label1 = cnt;
>
> /*
> * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> * goto out;
> */
> - EMIT2_off32(0x8B, 0x85, -36 - MAX_BPF_STACK); /* mov eax, dword ptr [rbp - 548] */
> + EMIT2_off32(0x8B, 0x85 /* mov eax, dword ptr [rbp - (4 + sd)] */,
> + -4 - round_up(stack_depth, 8));
> EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT); /* cmp eax, MAX_TAIL_CALL_CNT */
> -#define OFFSET2 (30 + RETPOLINE_RAX_BPF_JIT_SIZE)
> +#define OFFSET2 (off2 + RETPOLINE_RCX_BPF_JIT_SIZE)
> EMIT2(X86_JA, OFFSET2); /* ja out */
> - label2 = cnt;
> EMIT3(0x83, 0xC0, 0x01); /* add eax, 1 */
> - EMIT2_off32(0x89, 0x85, -36 - MAX_BPF_STACK); /* mov dword ptr [rbp -548], eax */
> + EMIT2_off32(0x89, 0x85, /* mov dword ptr [rbp - (4 + sd)], eax */
> + -4 - round_up(stack_depth, 8));
nit: should probably sit in a var
>
> /* prog = array->ptrs[index]; */
> - EMIT4_off32(0x48, 0x8B, 0x84, 0xD6, /* mov rax, [rsi + rdx * 8 + offsetof(...)] */
> + EMIT4_off32(0x48, 0x8B, 0x8C, 0xD6, /* mov rcx, [rsi + rdx * 8 + offsetof(...)] */
> offsetof(struct bpf_array, ptrs));
>
> /*
> * if (prog == NULL)
> * goto out;
> */
> - EMIT3(0x48, 0x85, 0xC0); /* test rax,rax */
> -#define OFFSET3 (8 + RETPOLINE_RAX_BPF_JIT_SIZE)
> - EMIT2(X86_JE, OFFSET3); /* je out */
> - label3 = cnt;
> + EMIT3(0x48, 0x85, 0xC9); /* test rcx,rcx */
> +#define OFFSET3 (off3 + RETPOLINE_RCX_BPF_JIT_SIZE)
> + EMIT2(X86_JE, OFFSET3); /* je out */
>
[...]
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index c67c88ad35f8..38897b9c7d61 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -651,14 +651,15 @@ enum bpf_jit_poke_reason {
>
> /* Descriptor of pokes pointing /into/ the JITed image. */
> struct bpf_jit_poke_descriptor {
> - void *ip;
> + void *tailcall_target;
> + void *tailcall_bypass;
> union {
> struct {
> struct bpf_map *map;
> u32 key;
> } tail_call;
> };
> - bool ip_stable;
> + bool tailcall_target_stable;
Probably makes sense to split off the pure rename into a separate patch to
reduce this one slightly.
> u8 adj_off;
> u16 reason;
> };
> @@ -1775,6 +1776,9 @@ enum bpf_text_poke_type {
> BPF_MOD_JUMP,
> };
>
> +/* Number of bytes emit_patch() needs to generate instructions */
> +#define X86_PATCH_SIZE 5
nit: this is arch specific, so should not be exposed in here, neither in
arraymap.c below
> int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
> void *addr1, void *addr2);
>
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index c66e8273fccd..d15729a3f46c 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -750,6 +750,7 @@ static void prog_array_map_poke_run(struct bpf_map *map, u32 key,
> struct bpf_prog *old,
> struct bpf_prog *new)
> {
> + u8 *bypass_addr, *old_addr, *new_addr;
> struct prog_poke_elem *elem;
> struct bpf_array_aux *aux;
>
> @@ -770,12 +771,13 @@ static void prog_array_map_poke_run(struct bpf_map *map, u32 key,
> * there could be danger of use after free otherwise.
> * 2) Initially when we start tracking aux, the program
> * is not JITed yet and also does not have a kallsyms
> - * entry. We skip these as poke->ip_stable is not
> - * active yet. The JIT will do the final fixup before
> - * setting it stable. The various poke->ip_stable are
> - * successively activated, so tail call updates can
> - * arrive from here while JIT is still finishing its
> - * final fixup for non-activated poke entries.
> + * entry. We skip these as poke->tailcall_target_stable
> + * is not active yet. The JIT will do the final fixup
> + * before setting it stable. The various
> + * poke->tailcall_target_stable are successively activated,
> + * so tail call updates can arrive from here while JIT
> + * is still finishing its final fixup for non-activated
> + * poke entries.
> * 3) On program teardown, the program's kallsym entry gets
> * removed out of RCU callback, but we can only untrack
> * from sleepable context, therefore bpf_arch_text_poke()
> @@ -792,20 +794,53 @@ static void prog_array_map_poke_run(struct bpf_map *map, u32 key,
> * 5) Any other error happening below from bpf_arch_text_poke()
> * is a unexpected bug.
> */
> - if (!READ_ONCE(poke->ip_stable))
> + if (!READ_ONCE(poke->tailcall_target_stable))
> continue;
> if (poke->reason != BPF_POKE_REASON_TAIL_CALL)
> continue;
> if (poke->tail_call.map != map ||
> poke->tail_call.key != key)
> continue;
> + /* protect against un-updated poke descriptors since
> + * we could fill them from subprog and the same desc
> + * is present on main's program poke tab
> + */
> + if (!poke->tailcall_bypass || !poke->tailcall_target)
> + continue;
Can't we avoid copying these descriptors over to the subprog in the first place?
> + if (!old && !new)
> + continue;
Could we avoid this above but instead signal via bpf_arch_text_poke() that nothing
had to be patched? Reason is that bpf_arch_text_poke() will still do the sanity
check to make sure reality meets expectation wrt current insns (which is also
why I didn't add this skip). In that case we could then just avoid the expensive
synchronize_rcu().
> - ret = bpf_arch_text_poke(poke->ip, BPF_MOD_JUMP,
> - old ? (u8 *)old->bpf_func +
> - poke->adj_off : NULL,
> - new ? (u8 *)new->bpf_func +
> - poke->adj_off : NULL);
> - BUG_ON(ret < 0 && ret != -EINVAL);
> + bypass_addr = (u8 *)poke->tailcall_target + X86_PATCH_SIZE;
> + old_addr = old ? (u8 *)old->bpf_func + poke->adj_off : NULL;
> + new_addr = new ? (u8 *)new->bpf_func + poke->adj_off : NULL;
> +
> + if (new) {
> + ret = bpf_arch_text_poke(poke->tailcall_target,
> + BPF_MOD_JUMP,
> + old_addr, new_addr);
> + BUG_ON(ret < 0 && ret != -EINVAL);
> + if (!old) {
> + ret = bpf_arch_text_poke(poke->tailcall_bypass,
> + BPF_MOD_JUMP,
> + bypass_addr, NULL);
> + BUG_ON(ret < 0 && ret != -EINVAL);
> + }
> + } else {
> + ret = bpf_arch_text_poke(poke->tailcall_bypass,
> + BPF_MOD_JUMP,
> + NULL, bypass_addr);
> + BUG_ON(ret < 0 && ret != -EINVAL);
> + /* let other CPUs finish the execution of program
> + * so that it will not possible to expose them
> + * to invalid nop, stack unwind, nop state
> + */
> + synchronize_rcu();
Very heavyweight that we need to potentially call this /multiple/ times for just a
/single/ map update under poke mutex even ... but agree it's needed here to avoid
racing. :(
> + ret = bpf_arch_text_poke(poke->tailcall_target,
> + BPF_MOD_JUMP,
> + old_addr, NULL);
> + BUG_ON(ret < 0 && ret != -EINVAL);
> + }
> }
> }
> }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.