* [PATCH 03/17] fpga: dfl: fme: support 512bit data width PR
From: Wu Hao @ 2019-03-25 3:07 UTC (permalink / raw)
To: atull, mdf, linux-fpga, linux-kernel
Cc: linux-api, Wu Hao, Ananda Ravuri, Xu Yilun
In-Reply-To: <1553483264-5379-1-git-send-email-hao.wu@intel.com>
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <ananda.ravuri@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75 +++++++++++++++++++++++++++++++++++++--------
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index 086ad24..076d74f 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -21,6 +21,8 @@
#include "dfl.h"
#include "dfl-fme.h"
+#define DRV_VERSION "0.8"
+
static ssize_t ports_num_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -277,3 +279,4 @@ MODULE_DESCRIPTION("FPGA Management Engine driver");
MODULE_AUTHOR("Intel Corporation");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("platform:dfl-fme");
+MODULE_VERSION(DRV_VERSION);
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
index b3f7eee..027d457 100644
--- a/drivers/fpga/dfl-fme-mgr.c
+++ b/drivers/fpga/dfl-fme-mgr.c
@@ -22,14 +22,18 @@
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/fpga/fpga-mgr.h>
+#include "dfl.h"
#include "dfl-fme-pr.h"
+#define DRV_VERSION "0.8"
+
/* FME Partial Reconfiguration Sub Feature Register Set */
#define FME_PR_DFH 0x0
#define FME_PR_CTRL 0x8
#define FME_PR_STS 0x10
#define FME_PR_DATA 0x18
#define FME_PR_ERR 0x20
+#define FME_PR_512_DATA 0x40 /* Data Register for 512bit datawidth PR */
#define FME_PR_INTFC_ID_L 0xA8
#define FME_PR_INTFC_ID_H 0xB0
@@ -67,8 +71,31 @@
#define PR_WAIT_TIMEOUT 8000000
#define PR_HOST_STATUS_IDLE 0
+#if defined(CONFIG_X86) && defined(CONFIG_AS_AVX512)
+
+#include <asm/fpu/api.h>
+
+static inline void copy512(void *src, void __iomem *dst)
+{
+ kernel_fpu_begin();
+
+ asm volatile("vmovdqu64 (%0), %%zmm0;"
+ "vmovntdq %%zmm0, (%1);"
+ :
+ : "r"(src), "r"(dst));
+
+ kernel_fpu_end();
+}
+#else
+static inline void copy512(void *src, void __iomem *dst)
+{
+ WARN_ON_ONCE(1);
+}
+#endif
+
struct fme_mgr_priv {
void __iomem *ioaddr;
+ unsigned int pr_datawidth;
u64 pr_error;
};
@@ -169,7 +196,7 @@ static int fme_mgr_write(struct fpga_manager *mgr,
struct fme_mgr_priv *priv = mgr->priv;
void __iomem *fme_pr = priv->ioaddr;
u64 pr_ctrl, pr_status, pr_data;
- int delay = 0, pr_credit, i = 0;
+ int ret = 0, delay = 0, pr_credit;
dev_dbg(dev, "start request\n");
@@ -181,9 +208,9 @@ static int fme_mgr_write(struct fpga_manager *mgr,
/*
* driver can push data to PR hardware using PR_DATA register once HW
- * has enough pr_credit (> 1), pr_credit reduces one for every 32bit
- * pr data write to PR_DATA register. If pr_credit <= 1, driver needs
- * to wait for enough pr_credit from hardware by polling.
+ * has enough pr_credit (> 1), pr_credit reduces one for every pr data
+ * width write to PR_DATA register. If pr_credit <= 1, driver needs to
+ * wait for enough pr_credit from hardware by polling.
*/
pr_status = readq(fme_pr + FME_PR_STS);
pr_credit = FIELD_GET(FME_PR_STS_PR_CREDIT, pr_status);
@@ -192,7 +219,8 @@ static int fme_mgr_write(struct fpga_manager *mgr,
while (pr_credit <= 1) {
if (delay++ > PR_WAIT_TIMEOUT) {
dev_err(dev, "PR_CREDIT timeout\n");
- return -ETIMEDOUT;
+ ret = -ETIMEDOUT;
+ goto done;
}
udelay(1);
@@ -200,21 +228,32 @@ static int fme_mgr_write(struct fpga_manager *mgr,
pr_credit = FIELD_GET(FME_PR_STS_PR_CREDIT, pr_status);
}
- if (count < 4) {
+ if (count < priv->pr_datawidth) {
dev_err(dev, "Invalid PR bitstream size\n");
return -EINVAL;
}
- pr_data = 0;
- pr_data |= FIELD_PREP(FME_PR_DATA_PR_DATA_RAW,
- *(((u32 *)buf) + i));
- writeq(pr_data, fme_pr + FME_PR_DATA);
- count -= 4;
+ switch (priv->pr_datawidth) {
+ case 4:
+ pr_data = 0;
+ pr_data |= FIELD_PREP(FME_PR_DATA_PR_DATA_RAW,
+ *((u32 *)buf));
+ writeq(pr_data, fme_pr + FME_PR_DATA);
+ break;
+ case 64:
+ copy512((void *)buf, fme_pr + FME_PR_512_DATA);
+ break;
+ default:
+ ret = -EFAULT;
+ goto done;
+ }
+ buf += priv->pr_datawidth;
+ count -= priv->pr_datawidth;
pr_credit--;
- i++;
}
- return 0;
+done:
+ return ret;
}
static int fme_mgr_write_complete(struct fpga_manager *mgr,
@@ -302,6 +341,15 @@ static int fme_mgr_probe(struct platform_device *pdev)
return PTR_ERR(priv->ioaddr);
}
+ /*
+ * Only revision 2 supports 512bit datawidth for better performance,
+ * other revisions use default 32bit datawidth.
+ */
+ if (dfl_feature_revision(priv->ioaddr) == 2)
+ priv->pr_datawidth = 64;
+ else
+ priv->pr_datawidth = 4;
+
compat_id = devm_kzalloc(dev, sizeof(*compat_id), GFP_KERNEL);
if (!compat_id)
return -ENOMEM;
@@ -342,3 +390,4 @@ MODULE_DESCRIPTION("FPGA Manager for DFL FPGA Management Engine");
MODULE_AUTHOR("Intel Corporation");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("platform:dfl-fme-mgr");
+MODULE_VERSION(DRV_VERSION);
diff --git a/drivers/fpga/dfl-fme-pr.c b/drivers/fpga/dfl-fme-pr.c
index c1fb1fe..8a0e46a 100644
--- a/drivers/fpga/dfl-fme-pr.c
+++ b/drivers/fpga/dfl-fme-pr.c
@@ -83,7 +83,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
if (copy_from_user(&port_pr, argp, minsz))
return -EFAULT;
- if (port_pr.argsz < minsz || port_pr.flags)
+ if (port_pr.argsz < minsz || port_pr.flags || !port_pr.buffer_size)
return -EINVAL;
/* get fme header region */
@@ -101,15 +101,25 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
port_pr.buffer_size))
return -EFAULT;
+ mutex_lock(&pdata->lock);
+ fme = dfl_fpga_pdata_get_private(pdata);
+ /* fme device has been unregistered. */
+ if (!fme) {
+ ret = -EINVAL;
+ goto unlock_exit;
+ }
+
/*
* align PR buffer per PR bandwidth, as HW ignores the extra padding
* data automatically.
*/
- length = ALIGN(port_pr.buffer_size, 4);
+ length = ALIGN(port_pr.buffer_size, fme->pr_datawidth);
buf = vmalloc(length);
- if (!buf)
- return -ENOMEM;
+ if (!buf) {
+ ret = -ENOMEM;
+ goto unlock_exit;
+ }
if (copy_from_user(buf,
(void __user *)(unsigned long)port_pr.buffer_address,
@@ -127,18 +137,10 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
- mutex_lock(&pdata->lock);
- fme = dfl_fpga_pdata_get_private(pdata);
- /* fme device has been unregistered. */
- if (!fme) {
- ret = -EINVAL;
- goto unlock_exit;
- }
-
region = dfl_fme_region_find(fme, port_pr.port_id);
if (!region) {
ret = -EINVAL;
- goto unlock_exit;
+ goto free_exit;
}
fpga_image_info_free(region->info);
@@ -159,13 +161,10 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
fpga_bridges_put(®ion->bridge_list);
put_device(®ion->dev);
-unlock_exit:
- mutex_unlock(&pdata->lock);
free_exit:
vfree(buf);
- if (copy_to_user((void __user *)arg, &port_pr, minsz))
- return -EFAULT;
-
+unlock_exit:
+ mutex_unlock(&pdata->lock);
return ret;
}
@@ -391,6 +390,16 @@ static int pr_mgmt_init(struct platform_device *pdev,
mutex_lock(&pdata->lock);
priv = dfl_fpga_pdata_get_private(pdata);
+ /*
+ * Initialize PR data width.
+ * Only revision 2 supports 512bit datawidth for better performance,
+ * other revisions use default 32bit datawidth.
+ */
+ if (dfl_feature_revision(feature->ioaddr) == 2)
+ priv->pr_datawidth = 64;
+ else
+ priv->pr_datawidth = 4;
+
/* Initialize the region and bridge sub device list */
INIT_LIST_HEAD(&priv->region_list);
INIT_LIST_HEAD(&priv->bridge_list);
diff --git a/drivers/fpga/dfl-fme.h b/drivers/fpga/dfl-fme.h
index 5394a21..de20755 100644
--- a/drivers/fpga/dfl-fme.h
+++ b/drivers/fpga/dfl-fme.h
@@ -21,12 +21,14 @@
/**
* struct dfl_fme - dfl fme private data
*
+ * @pr_datawidth: data width for partial reconfiguration.
* @mgr: FME's FPGA manager platform device.
* @region_list: linked list of FME's FPGA regions.
* @bridge_list: linked list of FME's FPGA bridges.
* @pdata: fme platform device's pdata.
*/
struct dfl_fme {
+ int pr_datawidth;
struct platform_device *mgr;
struct list_head region_list;
struct list_head bridge_list;
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index a8b869e..8851c6c 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -331,6 +331,11 @@ static inline bool dfl_feature_is_port(void __iomem *base)
(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
}
+static inline u8 dfl_feature_revision(void __iomem *base)
+{
+ return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
+}
+
/**
* struct dfl_fpga_enum_info - DFL FPGA enumeration information
*
--
2.7.4
^ permalink raw reply related
* [PATCH 02/17] fpga: dfl: fme: align PR buffer size per PR datawidth
From: Wu Hao @ 2019-03-25 3:07 UTC (permalink / raw)
To: atull, mdf, linux-fpga, linux-kernel; +Cc: linux-api, Wu Hao, Xu Yilun
In-Reply-To: <1553483264-5379-1-git-send-email-hao.wu@intel.com>
Current driver checks if input bitstream file size is aligned or
not per PR data width (default 32bits). It requires one additional
step for end user when they generate the bitstream file, padding
extra zeros to bitstream file to align its size per PR data width,
but they don't have to as hardware will drop extra padding bytes
automatically.
In order to simplify the user steps, this patch aligns PR buffer
size per PR data width in driver, to allow user to pass unaligned
size bitstream files to driver.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
drivers/fpga/dfl-fme-pr.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/fpga/dfl-fme-pr.c b/drivers/fpga/dfl-fme-pr.c
index d9ca955..c1fb1fe 100644
--- a/drivers/fpga/dfl-fme-pr.c
+++ b/drivers/fpga/dfl-fme-pr.c
@@ -74,6 +74,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
struct dfl_fme *fme;
unsigned long minsz;
void *buf = NULL;
+ size_t length;
int ret = 0;
u64 v;
@@ -85,9 +86,6 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
if (port_pr.argsz < minsz || port_pr.flags)
return -EINVAL;
- if (!IS_ALIGNED(port_pr.buffer_size, 4))
- return -EINVAL;
-
/* get fme header region */
fme_hdr = dfl_get_feature_ioaddr_by_id(&pdev->dev,
FME_FEATURE_ID_HEADER);
@@ -103,7 +101,13 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
port_pr.buffer_size))
return -EFAULT;
- buf = vmalloc(port_pr.buffer_size);
+ /*
+ * align PR buffer per PR bandwidth, as HW ignores the extra padding
+ * data automatically.
+ */
+ length = ALIGN(port_pr.buffer_size, 4);
+
+ buf = vmalloc(length);
if (!buf)
return -ENOMEM;
@@ -140,7 +144,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
fpga_image_info_free(region->info);
info->buf = buf;
- info->count = port_pr.buffer_size;
+ info->count = length;
info->region_id = port_pr.port_id;
region->info = info;
--
2.7.4
^ permalink raw reply related
* [PATCH 01/17] fpga: dfl-fme-mgr: fix FME_PR_INTFC_ID register address.
From: Wu Hao @ 2019-03-25 3:07 UTC (permalink / raw)
To: atull, mdf, linux-fpga, linux-kernel; +Cc: linux-api, Wu Hao
In-Reply-To: <1553483264-5379-1-git-send-email-hao.wu@intel.com>
FME_PR_INTFC_ID is used as compat_id for fpga manager and region,
but high 64 bits and low 64 bits of the compat_id are swapped by
mistake. This patch fixes this problem by fixing register address.
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
drivers/fpga/dfl-fme-mgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
index 76f3770..b3f7eee 100644
--- a/drivers/fpga/dfl-fme-mgr.c
+++ b/drivers/fpga/dfl-fme-mgr.c
@@ -30,8 +30,8 @@
#define FME_PR_STS 0x10
#define FME_PR_DATA 0x18
#define FME_PR_ERR 0x20
-#define FME_PR_INTFC_ID_H 0xA8
-#define FME_PR_INTFC_ID_L 0xB0
+#define FME_PR_INTFC_ID_L 0xA8
+#define FME_PR_INTFC_ID_H 0xB0
/* FME PR Control Register Bitfield */
#define FME_PR_CTRL_PR_RST BIT_ULL(0) /* Reset PR engine */
--
2.7.4
^ permalink raw reply related
* [PATCH 00/17] add new features for FPGA DFL drivers
From: Wu Hao @ 2019-03-25 3:07 UTC (permalink / raw)
To: atull, mdf, linux-fpga, linux-kernel; +Cc: linux-api, Wu Hao
This patchset adds more features support for FPGA Device Feature List
(DFL) drivers, including PR enhancement, virtualization support based
on PCIe SRIOV, private features to Port, private features to FME, and
enhancement to DFL framework. Please refer to details in below list.
Patch 1: A bug fixing to current dfl-fme-mgr driver.
Patch 2-3: add 512bit data width PR support.
Patch 4-6: add virtualization support based on PCIe SRIOV.
Patch 7-8: add new AFU state and userclock related sysfs to dfl-afu.
Patch 9-10: enhancement to DFL framework in order to support id_table.
Patch 11: add error reporting private feature support to dfl-afu.
Patch 12: add STP (SignalTap) private feature support to dfl-afu.
Patch 13: add capability sysfs interfaces to dfl-fme.
Patch 14: add thermal management private feature support to dfl-fme.
Patch 15: add power management private feature support to dfl-fme.
Patch 16: add global error reporitng private feature support to dfl-fme.
Patch 17: add performance reporting support to dfl-fme.
Wu Hao (17):
fpga: dfl-fme-mgr: fix FME_PR_INTFC_ID register address.
fpga: dfl: fme: align PR buffer size per PR datawidth
fpga: dfl: fme: support 512bit data width PR
Documentation: fpga: dfl: add descriptions for virtualization and new
interfaces.
fpga: dfl: fme: add DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.
fpga: dfl: pci: enable SRIOV support.
fpga: dfl: afu: add AFU state related sysfs interfaces
fpga: dfl: afu: add userclock sysfs interfaces.
fpga: dfl: add id_table for dfl private feature driver
fpga: dfl: afu: export __port_enable/disable function.
fpga: dfl: afu: add error reporting support.
fpga: dfl: afu: add STP (SignalTap) support
fpga: dfl: fme: add capability sysfs interfaces
fpga: dfl: fme: add thermal management support
fpga: dfl: fme: add power management support
fpga: dfl: fme: add global error reporting support
fpga: dfl: fme: add performance reporting support
Documentation/ABI/testing/sysfs-platform-dfl-fme | 279 +++++++
Documentation/ABI/testing/sysfs-platform-dfl-port | 94 +++
Documentation/fpga/dfl.txt | 115 +++
drivers/fpga/Makefile | 4 +-
drivers/fpga/dfl-afu-error.c | 225 +++++
drivers/fpga/dfl-afu-main.c | 335 +++++++-
drivers/fpga/dfl-afu.h | 7 +
drivers/fpga/dfl-fme-error.c | 390 +++++++++
drivers/fpga/dfl-fme-main.c | 583 ++++++++++++-
drivers/fpga/dfl-fme-mgr.c | 79 +-
drivers/fpga/dfl-fme-perf.c | 950 ++++++++++++++++++++++
drivers/fpga/dfl-fme-pr.c | 64 +-
drivers/fpga/dfl-fme.h | 9 +-
drivers/fpga/dfl-pci.c | 40 +
drivers/fpga/dfl.c | 170 +++-
drivers/fpga/dfl.h | 56 +-
include/uapi/linux/fpga-dfl.h | 32 +
17 files changed, 3355 insertions(+), 77 deletions(-)
create mode 100644 drivers/fpga/dfl-afu-error.c
create mode 100644 drivers/fpga/dfl-fme-error.c
create mode 100644 drivers/fpga/dfl-fme-perf.c
--
2.7.4
^ permalink raw reply
* Re: [RFC PATCH v3 07/18] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
From: Richard Weinberger @ 2019-03-22 22:02 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-ext4, open list:ABI/API, linux-f2fs-devel, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, Satya Tangirala,
Paul Crowley
In-Reply-To: <20190318230830.GA40545@gmail.com>
Ericm
Am Dienstag, 19. März 2019, 00:08:31 CET schrieb Eric Biggers:
> I tried using sb->s_bdi->name, but it's still "ubifs" for all UBIFS filesystems.
hmpf.
> Perhaps there's a way you can make ->s_id for UBIFS unique? There are already
> existing places that log ->s_id, so perhaps you should do it anyway regardless
> of this patchset?
Yes, let me implement that.
ubifs does:
super_setup_bdi_name(sb, "ubifs_%d_%d", c->vi.ubi_num, c->vi.vol_id);
So, I try to set ->s_id also to ubifs_%d_%d.
> > >
> > > > Note that the keyring name isn't particularly important, since the ioctls will
> > > > work regardless. But we might as well choose something logical, since the
> > > > keyring name will still show up in /proc/keys.
> > >
> > > I'm not done with reviewing your patches, but will it be possible to use keyctl?
> > > For the a unique name is helpful. :)
> > >
> >
> > Not for adding keys, removing keys, or getting a key's status -- those are what
> > the ioctls are for.
> >
> > See e.g. the discussion in patch 7 ("fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY
> > ioctl") for why the keyrings syscalls are a poor fit for fscrypt.
> >
>
> Anyway, perhaps I should reconsider whether fscrypt should even use the keyrings
> subsystem at all, even just "internally", as its quirks still leak out a bit.
> I'd prefer a nice clean API without any quirks like having to name the keyrings
> and assign SELinux labels to the keys just to make the keyrings subsystem happy.
IMHO the keys subsytem is a good fit. For example for stuff like this one:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1945778.html
We use UBIFS on many embedded systems with crypto hardware.
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH 2/4] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux
From: Carlos O'Donell @ 2019-03-22 20:13 UTC (permalink / raw)
To: Mathieu Desnoyers, Carlos O'Donell
Cc: Florian Weimer, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <20190212194253.1951-3-mathieu.desnoyers@efficios.com>
On 2/12/19 2:42 PM, Mathieu Desnoyers wrote:
> When available, use the cpu_id field from __rseq_abi on Linux to
> implement sched_getcpu(). Fall-back on the vgetcpu vDSO if unavailable.
>
> Benchmarks:
>
> x86-64: Intel E5-2630 v3@2.40GHz, 16-core, hyperthreading
This patch looks good to me for master, but is blocked on patch 1/4
being reworked.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> glibc sched_getcpu(): 13.7 ns (baseline)
> glibc sched_getcpu() using rseq: 2.5 ns (speedup: 5.5x)
> inline load cpuid from __rseq_abi TLS: 0.8 ns (speedup: 17.1x)
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Carlos O'Donell <carlos@redhat.com>
> CC: Florian Weimer <fweimer@redhat.com>
> CC: Joseph Myers <joseph@codesourcery.com>
> CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Ben Maurer <bmaurer@fb.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> CC: Boqun Feng <boqun.feng@gmail.com>
> CC: Will Deacon <will.deacon@arm.com>
> CC: Dave Watson <davejwatson@fb.com>
> CC: Paul Turner <pjt@google.com>
> CC: libc-alpha@sourceware.org
> CC: linux-kernel@vger.kernel.org
> CC: linux-api@vger.kernel.org
> ---
> sysdeps/unix/sysv/linux/sched_getcpu.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
> index fb0d317f83..8bfb03778b 100644
> --- a/sysdeps/unix/sysv/linux/sched_getcpu.c
> +++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
> @@ -24,8 +24,8 @@
> #endif
> #include <sysdep-vdso.h>
>
> -int
> -sched_getcpu (void)
> +static int
> +vsyscall_sched_getcpu (void)
OK.
> {
> #ifdef __NR_getcpu
> unsigned int cpu;
> @@ -37,3 +37,24 @@ sched_getcpu (void)
> return -1;
> #endif
> }
> +
> +#ifdef __NR_rseq
> +#include <linux/rseq.h>
> +
> +extern __attribute__ ((tls_model ("initial-exec")))
> +__thread volatile struct rseq __rseq_abi;
OK.
> +
> +int
> +sched_getcpu (void)
> +{
> + int cpu_id = __rseq_abi.cpu_id;
> +
> + return cpu_id >= 0 ? cpu_id : vsyscall_sched_getcpu ();
OK. Impressive :-)
> +}
> +#else
> +int
> +sched_getcpu (void)
> +{
> + return vsyscall_sched_getcpu ();
OK.
> +}
> +#endif
>
--
Cheers,
Carlos.
^ permalink raw reply
* Re: [PATCH 1/4] glibc: Perform rseq(2) registration at C startup and thread creation (v7)
From: Carlos O'Donell @ 2019-03-22 20:09 UTC (permalink / raw)
To: Mathieu Desnoyers, Carlos O'Donell
Cc: Florian Weimer, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, Rich Felker,
linux-kernel, linux-api
In-Reply-To: <20190212194253.1951-2-mathieu.desnoyers@efficios.com>
On 2/12/19 2:42 PM, Mathieu Desnoyers wrote:
> Register rseq(2) TLS for each thread (including main), and unregister
> for each thread (excluding main). "rseq" stands for Restartable
> Sequences.
Thanks, the implementation is looking good, before this goes in it needs
some procedural cleanup and the following:
(a) I would like to see a final position from Florian, either accept,
object (objection recorded), or sustained objection (blocks this patch),
on the issue of the registration protocol.
I suggested a possible way forward to remove the registration protocol
down the line, leaving only the __rseq_lib_abi as the residual artifact
once we deprecate coordination with other libraries. Applications would
have to be written with this in mind, and coordination still remains
with libc for the time being. Perhaps Mathieu has another suggestion.
(b) I would like to see a final position from Rich Felker, either accept,
object (objection recorded), or sustained objection (blocks this patch),
on the patch set as a whole. I trust Rich's opinion as an alternative
libc maintainer in this matter and as a independent implementor of
similar standards. Rich will have to implement this for musl, and it
would be good to have consensus from him.
So while I reviewed this patch, and provided nit-picky feedback below,
I think we need a whole new thread, to finalize the way forward for the
registration process.
Next steps:
- Start a new thread to talk *only* about options for removing the
refcounted registration.
> See the rseq(2) man page proposed here:
> https://lkml.org/lkml/2018/9/19/647
Thank you for having a man page, that makes for a very easy to follow
description of the structures, syscall and use cases.
>
> This patch is based on glibc-2.29. The rseq(2) system call was merged
> into Linux 4.18.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Carlos O'Donell <carlos@redhat.com>
> CC: Florian Weimer <fweimer@redhat.com>
> CC: Joseph Myers <joseph@codesourcery.com>
> CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Ben Maurer <bmaurer@fb.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> CC: Boqun Feng <boqun.feng@gmail.com>
> CC: Will Deacon <will.deacon@arm.com>
> CC: Dave Watson <davejwatson@fb.com>
> CC: Paul Turner <pjt@google.com>
> CC: Rich Felker <dalias@libc.org>
> CC: libc-alpha@sourceware.org
> CC: linux-kernel@vger.kernel.org
> CC: linux-api@vger.kernel.org
> ---
> Changes since v1:
> - Move __rseq_refcount to an extra field at the end of __rseq_abi to
> eliminate one symbol.
>
> All libraries/programs which try to register rseq (glibc,
> early-adopter applications, early-adopter libraries) should use the
> rseq refcount. It becomes part of the ABI within a user-space
> process, but it's not part of the ABI shared with the kernel per se.
>
> - Restructure how this code is organized so glibc keeps building on
> non-Linux targets.
>
> - Use non-weak symbol for __rseq_abi.
>
> - Move rseq registration/unregistration implementation into its own
> nptl/rseq.c compile unit.
>
> - Move __rseq_abi symbol under GLIBC_2.29.
>
> Changes since v2:
> - Move __rseq_refcount to its own symbol, which is less ugly than
> trying to play tricks with the rseq uapi.
> - Move __rseq_abi from nptl to csu (C start up), so it can be used
> across glibc, including memory allocator and sched_getcpu(). The
> __rseq_refcount symbol is kept in nptl, because there is no reason
> to use it elsewhere in glibc.
>
> Changes since v3:
> - Set __rseq_refcount TLS to 1 on register/set to 0 on unregister
> because glibc is the first/last user.
> - Unconditionally register/unregister rseq at thread start/exit, because
> glibc is the first/last user.
> - Add missing abilist items.
> - Rebase on glibc master commit a502c5294.
> - Add NEWS entry.
>
> Changes since v4:
> - Do not use "weak" symbols for __rseq_abi and __rseq_refcount. Based on
> "System V Application Binary Interface", weak only affects the link
> editor, not the dynamic linker.
> - Install a new sys/rseq.h system header on Linux, which contains the
> RSEQ_SIG definition, __rseq_abi declaration and __rseq_refcount
> declaration. Move those definition/declarations from rseq-internal.h
> to the installed sys/rseq.h header.
> - Considering that rseq is only available on Linux, move csu/rseq.c to
> sysdeps/unix/sysv/linux/rseq-sym.c.
> - Move __rseq_refcount from nptl/rseq.c to
> sysdeps/unix/sysv/linux/rseq-sym.c, so it is only defined on Linux.
> - Move both ABI definitions for __rseq_abi and __rseq_refcount to
> sysdeps/unix/sysv/linux/Versions, so they only appear on Linux.
> - Document __rseq_abi and __rseq_refcount volatile.
> - Document the RSEQ_SIG signature define.
> - Move registration functions from rseq.c to rseq-internal.h static
> inline functions. Introduce empty stubs in misc/rseq-internal.h,
> which can be overridden by architecture code in
> sysdeps/unix/sysv/linux/rseq-internal.h.
> - Rename __rseq_register_current_thread and __rseq_unregister_current_thread
> to rseq_register_current_thread and rseq_unregister_current_thread,
> now that those are only visible as internal static inline functions.
> - Invoke rseq_register_current_thread() from libc-start.c LIBC_START_MAIN
> rather than nptl init, so applications not linked against
> libpthread.so have rseq registered for their main() thread. Note that
> it is invoked separately for SHARED and !SHARED builds.
>
> Changes since v5:
> - Replace __rseq_refcount by __rseq_lib_abi, which contains two
> uint32_t: register_state and refcount. The "register_state" field
> allows inhibiting rseq registration from signal handlers nested on top
> of glibc registration and occuring after rseq unregistration by glibc.
> - Introduce enum rseq_register_state, which contains the states allowed
> for the struct rseq_lib_abi register_state field.
>
> Changes since v6:
> - Introduce bits/rseq.h to define RSEQ_SIG for each architecture.
> The generic bits/rseq.h does not define RSEQ_SIG, meaning that each
> architecture implementing rseq needs to implement bits/rseq.h.
> - Rename enum item RSEQ_REGISTER_NESTED to RSEQ_REGISTER_ONGOING.
> - Port to glibc-2.29.
> ---
> NEWS | 11 +++
> csu/libc-start.c | 12 ++-
> misc/Makefile | 3 +-
> misc/rseq-internal.h | 34 +++++++
> nptl/pthread_create.c | 9 ++
> sysdeps/unix/sysv/linux/Makefile | 4 +-
> sysdeps/unix/sysv/linux/Versions | 4 +
> sysdeps/unix/sysv/linux/aarch64/bits/rseq.h | 24 +++++
> sysdeps/unix/sysv/linux/aarch64/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/alpha/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/arm/bits/rseq.h | 24 +++++
> sysdeps/unix/sysv/linux/arm/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/bits/rseq.h | 24 +++++
> sysdeps/unix/sysv/linux/hppa/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/i386/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/ia64/libc.abilist | 2 +
> .../sysv/linux/m68k/coldfire/libc.abilist | 2 +
> .../unix/sysv/linux/m68k/m680x0/libc.abilist | 2 +
> .../unix/sysv/linux/microblaze/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/mips/bits/rseq.h | 24 +++++
> .../sysv/linux/mips/mips32/fpu/libc.abilist | 2 +
> .../sysv/linux/mips/mips32/nofpu/libc.abilist | 2 +
> .../sysv/linux/mips/mips64/n32/libc.abilist | 2 +
> .../sysv/linux/mips/mips64/n64/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/nios2/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/powerpc/bits/rseq.h | 24 +++++
> .../linux/powerpc/powerpc32/fpu/libc.abilist | 2 +
> .../powerpc/powerpc32/nofpu/libc.abilist | 2 +
> .../linux/powerpc/powerpc64/be/libc.abilist | 2 +
> .../linux/powerpc/powerpc64/le/libc.abilist | 2 +
> .../unix/sysv/linux/riscv/rv64/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/rseq-internal.h | 91 +++++++++++++++++++
> sysdeps/unix/sysv/linux/rseq-sym.c | 54 +++++++++++
> sysdeps/unix/sysv/linux/s390/bits/rseq.h | 24 +++++
> .../unix/sysv/linux/s390/s390-32/libc.abilist | 2 +
> .../unix/sysv/linux/s390/s390-64/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/sh/libc.abilist | 2 +
> .../sysv/linux/sparc/sparc32/libc.abilist | 2 +
> .../sysv/linux/sparc/sparc64/libc.abilist | 2 +
> sysdeps/unix/sysv/linux/sys/rseq.h | 65 +++++++++++++
> sysdeps/unix/sysv/linux/x86/bits/rseq.h | 24 +++++
> .../unix/sysv/linux/x86_64/64/libc.abilist | 2 +
> .../unix/sysv/linux/x86_64/x32/libc.abilist | 2 +
> 43 files changed, 501 insertions(+), 6 deletions(-)
> create mode 100644 misc/rseq-internal.h
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/arm/bits/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/bits/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/mips/bits/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/rseq-internal.h
> create mode 100644 sysdeps/unix/sysv/linux/rseq-sym.c
> create mode 100644 sysdeps/unix/sysv/linux/s390/bits/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/sys/rseq.h
> create mode 100644 sysdeps/unix/sysv/linux/x86/bits/rseq.h
>
> diff --git a/NEWS b/NEWS
> index 912a9bdc0f..0608c60f7d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,17 @@ See the end for copying conditions.
> Please send GNU C library bug reports via <https://sourceware.org/bugzilla/>
> using `glibc' in the "product" field.
> \f
> +Version 2.30
> +
> +Major new features:
> +
> +* Support for automatically registering threads with the Linux rseq(2)
> + system call has been added. This system call is implemented starting
> + from Linux 4.18. In order to be activated, it requires that glibc is built
> + against kernel headers that include this system call, and that glibc
> + detects availability of that system call at runtime.
What benefit does the feature have for users? Can you talk about that please?
Why would a user want to use it. Please feel free to link to an external reference.
> +
> +\f
> Version 2.29
>
> Major new features:
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 5d9c3675fa..8680afc0ef 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -22,6 +22,7 @@
> #include <ldsodefs.h>
> #include <exit-thread.h>
> #include <libc-internal.h>
> +#include <rseq-internal.h>
OK.
>
> #include <elf/dl-tunables.h>
>
> @@ -140,7 +141,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>
> __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up;
>
> -#ifndef SHARED
> +#ifdef SHARED
> + /* Register rseq ABI to the kernel. */
> + (void) rseq_register_current_thread ();
OK, early registration of main thread in shared case.
> +#else
> _dl_relocate_static_pie ();
>
> char **ev = &argv[argc + 1];
> @@ -218,6 +222,9 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> }
> # endif
>
> + /* Register rseq ABI to the kernel. */
> + (void) rseq_register_current_thread ();
OK, early registration of main thread in static case.
> +
> /* Initialize libpthread if linked in. */
> if (__pthread_initialize_minimal != NULL)
> __pthread_initialize_minimal ();
> @@ -230,8 +237,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> # else
> __pointer_chk_guard_local = pointer_chk_guard;
> # endif
> -
> -#endif /* !SHARED */
> +#endif
OK.
>
> /* Register the destructor of the dynamic linker if there is any. */
> if (__glibc_likely (rtld_fini != NULL))
> diff --git a/misc/Makefile b/misc/Makefile
> index cf0daa1161..0ae1dbaf80 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -36,7 +36,8 @@ headers := sys/uio.h bits/uio-ext.h bits/uio_lim.h \
> syslog.h sys/syslog.h \
> bits/syslog.h bits/syslog-ldbl.h bits/syslog-path.h bits/error.h \
> bits/select2.h bits/hwcap.h sys/auxv.h \
> - sys/sysmacros.h bits/sysmacros.h bits/types/struct_iovec.h
> + sys/sysmacros.h bits/sysmacros.h bits/types/struct_iovec.h \
> + rseq-internal.h
OK.
>
> routines := brk sbrk sstk ioctl \
> readv writev preadv preadv64 pwritev pwritev64 \
> diff --git a/misc/rseq-internal.h b/misc/rseq-internal.h
> new file mode 100644
> index 0000000000..915122e4bf
> --- /dev/null
> +++ b/misc/rseq-internal.h
> @@ -0,0 +1,34 @@
> +/* Copyright (C) 2018 Free Software Foundation, Inc.
Add a one line description before copyright which describes the file.
Is it a stub file. Is it the Linux implementation. etc.
Please add a one sentence first line short description for all new files
you create.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
We don't use "contributed by" markers anymore, we let the git commit author
carry that information, along with git blame information.
Please remove this contributed by line and all subsequent such lines.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef RSEQ_INTERNAL_H
> +#define RSEQ_INTERNAL_H
> +
> +static inline int
> +rseq_register_current_thread (void)
> +{
> + return -1;
> +}
> +
> +static inline int
> +rseq_unregister_current_thread (void)
> +{
> + return -1;
> +}
> +
OK.
> +#endif /* rseq-internal.h */
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 2bd2b10727..90b3419390 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -33,6 +33,7 @@
> #include <default-sched.h>
> #include <futex-internal.h>
> #include <tls-setup.h>
> +#include <rseq-internal.h>
Ok.
> #include "libioP.h"
>
> #include <shlib-compat.h>
> @@ -378,6 +379,7 @@ __free_tcb (struct pthread *pd)
> START_THREAD_DEFN
> {
> struct pthread *pd = START_THREAD_SELF;
> + bool has_rseq = false;
OK.
>
> #if HP_TIMING_AVAIL
> /* Remember the time when the thread was started. */
> @@ -396,6 +398,9 @@ START_THREAD_DEFN
> if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0) == -2))
> futex_wake (&pd->setxid_futex, 1, FUTEX_PRIVATE);
>
> + /* Register rseq TLS to the kernel. */
> + has_rseq = !rseq_register_current_thread ();
OK, initialize rseq after enabling setxid but before setting up roubust list,
no particularly reason it should be right here that I can see, that's OK though.
> +
> #ifdef __NR_set_robust_list
> # ifndef __ASSUME_SET_ROBUST_LIST
> if (__set_robust_list_avail >= 0)
> @@ -573,6 +578,10 @@ START_THREAD_DEFN
> }
> #endif
>
> + /* Unregister rseq TLS from kernel. */
> + if (has_rseq && rseq_unregister_current_thread ())
> + abort();
OK. Calling abort() seems harsh because it kills the whole process, but I'm OK
with failing catastrophically if that's what we want here. It seems unrecoverable.
> +
> advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
> pd->guardsize);
>
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 5f8c2c7c7d..5b541469ec 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -1,5 +1,5 @@
> ifeq ($(subdir),csu)
> -sysdep_routines += errno-loc
> +sysdep_routines += errno-loc rseq-sym
OK.
> endif
>
> ifeq ($(subdir),assert)
> @@ -48,7 +48,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
> bits/termios-c_iflag.h bits/termios-c_oflag.h \
> bits/termios-baud.h bits/termios-c_cflag.h \
> bits/termios-c_lflag.h bits/termios-tcflow.h \
> - bits/termios-misc.h
> + bits/termios-misc.h sys/rseq.h bits/rseq.h
OK.
>
> tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
> tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> index f1e12d9c69..ad88c2b7ff 100644
> --- a/sysdeps/unix/sysv/linux/Versions
> +++ b/sysdeps/unix/sysv/linux/Versions
> @@ -174,6 +174,10 @@ libc {
> GLIBC_2.29 {
> getcpu;
> }
> + GLIBC_2.30 {
> + __rseq_abi;
> + __rseq_lib_abi;
> + }
OK, good using GLIBC_2.30 for master.
> GLIBC_PRIVATE {
> # functions used in other libraries
> __syscall_rt_sigqueueinfo;
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> new file mode 100644
> index 0000000000..543bc5388a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> @@ -0,0 +1,24 @@
Needs a one sentence description.
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Signature required before each abort handler code. */
> +#define RSEQ_SIG 0xd428bc00 /* BRK #0x45E0. */
OK.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
> index 9c330f325e..bc937f585d 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
> @@ -2141,3 +2141,5 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
> index f630fa4c6f..89cc8b1cfb 100644
> --- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
> @@ -2036,6 +2036,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> new file mode 100644
> index 0000000000..19d3755837
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> @@ -0,0 +1,24 @@
Needs a one sentence description.
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Signature required before each abort handler code. */
> +#define RSEQ_SIG 0x53053053
Why isn't this an arm specific op code? Does the user have to mark this
up as part of a constant pool when placing it in front of the abort handler
to avoid disassembling the constant as code? This was at one point required
to get gdb to work properly.
> diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist
> index b96f45590f..e5055f2d4e 100644
> --- a/sysdeps/unix/sysv/linux/arm/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/arm/libc.abilist
> @@ -126,6 +126,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.4 _Exit F
> GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
> GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
> diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h b/sysdeps/unix/sysv/linux/bits/rseq.h
> new file mode 100644
> index 0000000000..d60f02cfeb
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/rseq.h
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
Needs a one sentence description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Each architecture supporting rseq should define RSEQ_SIG as a 32-bit
> + signature inserted before each rseq abort label in the code section. */
Needs a huge explanation about RSEQ_SIG and the reasons why it's per-arch.
Basically cut-and-paste what you wrote to libc-alpha about this.
> diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
> index 088a8ee369..546d073cdb 100644
> --- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
> @@ -1883,6 +1883,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
> index f7ff2c57b9..ac1de6e4b3 100644
> --- a/sysdeps/unix/sysv/linux/i386/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
> @@ -2048,6 +2048,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
> index becd8b1033..cc3445b958 100644
> --- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
> @@ -1917,6 +1917,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
> index 74e42a5209..f7e28bd5a0 100644
> --- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
> @@ -127,6 +127,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.4 _Exit F
> GLIBC_2.4 _IO_2_1_stderr_ D 0x98
> GLIBC_2.4 _IO_2_1_stdin_ D 0x98
> diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
> index 4af5a74e8a..b8f00f6111 100644
> --- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
> @@ -1992,6 +1992,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/microblaze/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/libc.abilist
> index ccef673fd2..19f191434f 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/microblaze/libc.abilist
> @@ -2133,3 +2133,5 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/rseq.h b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
> new file mode 100644
> index 0000000000..19d3755837
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
Needs a one sentence description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Signature required before each abort handler code. */
> +#define RSEQ_SIG 0x53053053
Why isn't this a mips-specific op code?
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
> index 1054bb599e..fe43507f55 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
> @@ -1970,6 +1970,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
> index 4f5b5ffebf..b247c6ea9b 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
> @@ -1968,6 +1968,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
> index 943aee58d4..5339ca52b6 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
> @@ -1976,6 +1976,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
> index 17a5d17ef9..11f24eb440 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
> @@ -1971,6 +1971,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
> index 4d62a540fd..fd223bfc44 100644
> --- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
> @@ -2174,3 +2174,5 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
> new file mode 100644
> index 0000000000..19d3755837
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
Needs a one sentence description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Signature required before each abort handler code. */
> +#define RSEQ_SIG 0x53053053
Why isn't this an opcode specific to power?
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
> index ecc2d6fa13..cc53178e81 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
> @@ -1996,6 +1996,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
> index f5830f9c33..2de3134bc7 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
> @@ -2000,6 +2000,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
> index 633d8f4792..aae3def700 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
> @@ -126,6 +126,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 _Exit F
> GLIBC_2.3 _IO_2_1_stderr_ D 0xe0
> GLIBC_2.3 _IO_2_1_stdin_ D 0xe0
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
> index 2c712636ef..8d582a3a9b 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
> @@ -2231,3 +2231,5 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> index 195bc8b2cf..155953f6cf 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> @@ -2103,3 +2103,5 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> new file mode 100644
> index 0000000000..d676da3701
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
> @@ -0,0 +1,91 @@
> +/* Copyright (C) 2018 Free Software Foundation, Inc.
Needs a one sentence description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef RSEQ_INTERNAL_H
> +#define RSEQ_INTERNAL_H
> +
> +#include <sysdep.h>
> +
> +#ifdef __NR_rseq
> +
> +#include <errno.h>
> +#include <sys/rseq.h>
> +
This needs a giant comment describing the reference count algorithm
and how it is intended to work.
> +static inline int
> +rseq_register_current_thread (void)
> +{
> + int rc, ret = 0;
> + INTERNAL_SYSCALL_DECL (err);
> +
> + if (__rseq_abi.cpu_id == RSEQ_CPU_ID_REGISTRATION_FAILED)
> + return -1;
> + /* Temporarily prevent nested signal handlers from registering rseq. */
> + __rseq_lib_abi.register_state = RSEQ_REGISTER_ONGOING;
OK.
> + if (__rseq_lib_abi.refcount == UINT_MAX)
> + {
> + ret = -1;
> + goto end;
> + }
> + if (__rseq_lib_abi.refcount++)
> + goto end;
OK.
> + rc = INTERNAL_SYSCALL_CALL (rseq, err, &__rseq_abi, sizeof (struct rseq),
> + 0, RSEQ_SIG);
> + if (!rc)
> + goto end;
> + if (INTERNAL_SYSCALL_ERRNO (rc, err) != EBUSY)
> + __rseq_abi.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
> + ret = -1;
> +end:
> + __rseq_lib_abi.register_state = RSEQ_REGISTER_ALLOWED;
OK.
> + return ret;
> +}
> +
> +static inline int
> +rseq_unregister_current_thread (void)
> +{
> + int rc, ret = 0;
> + INTERNAL_SYSCALL_DECL (err);
> +
> + /* Setting __rseq_register_state = RSEQ_REGISTER_EXITING for the rest of the
> + thread lifetime. Ensures signal handlers nesting just before thread exit
> + don't try to register rseq. */
> + __rseq_lib_abi.register_state = RSEQ_REGISTER_EXITING;
> + __rseq_lib_abi.refcount = 0;
> + rc = INTERNAL_SYSCALL_CALL (rseq, err, &__rseq_abi, sizeof (struct rseq),
> + RSEQ_FLAG_UNREGISTER, RSEQ_SIG);
OK.
> + if (!rc)
> + goto end;
> + ret = -1;
> +end:
> + return ret;
> +}
> +#else
> +static inline int
> +rseq_register_current_thread (void)
> +{
> + return -1;
> +}
> +
> +static inline int
> +rseq_unregister_current_thread (void)
> +{
> + return -1;
> +}
OK.
> +#endif
> +
> +#endif /* rseq-internal.h */
> diff --git a/sysdeps/unix/sysv/linux/rseq-sym.c b/sysdeps/unix/sysv/linux/rseq-sym.c
> new file mode 100644
> index 0000000000..99b277e9d6
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/rseq-sym.c
> @@ -0,0 +1,54 @@
> +/* Copyright (C) 2018 Free Software Foundation, Inc.
Needs a one sentence description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <sys/syscall.h>
> +#include <stdint.h>
> +
> +#ifdef __NR_rseq
> +#include <sys/rseq.h>
> +#else
> +
> +enum rseq_cpu_id_state {
> + RSEQ_CPU_ID_UNINITIALIZED = -1,
> + RSEQ_CPU_ID_REGISTRATION_FAILED = -2,
OK.
> +};
> +
> +/* linux/rseq.h defines struct rseq as aligned on 32 bytes. The kernel ABI
> + size is 20 bytes. */
> +struct rseq {
> + uint32_t cpu_id_start;
> + uint32_t cpu_id;
> + uint64_t rseq_cs;
> + uint32_t flags;
> +} __attribute__ ((aligned(4 * sizeof(uint64_t))));
OK, this is the kernel abi.
> +
> +struct rseq_lib_abi
> +{
> + uint32_t register_state;
> + uint32_t refcount;
OK. This is the refcount coordination structure for registration.
> +};
> +
> +#endif
> +
> +/* volatile because fields can be read/updated by the kernel. */
> +__thread volatile struct rseq __rseq_abi = {
> + .cpu_id = RSEQ_CPU_ID_UNINITIALIZED,
OK.
> +};
> +
> +/* volatile because fields can be read/updated by signal handlers. */
> +__thread volatile struct rseq_lib_abi __rseq_lib_abi;
OK.
> diff --git a/sysdeps/unix/sysv/linux/s390/bits/rseq.h b/sysdeps/unix/sysv/linux/s390/bits/rseq.h
> new file mode 100644
> index 0000000000..19d3755837
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/s390/bits/rseq.h
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
Needs a one sentence description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Signature required before each abort handler code. */
> +#define RSEQ_SIG 0x53053053
Why not a s390 specific value here?
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
> index 334def033c..42316d8666 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
> @@ -2005,6 +2005,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
> index 536f4c4ced..c6c4e55a77 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
> @@ -1911,6 +1911,8 @@ GLIBC_2.29 __fentry__ F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/sh/libc.abilist b/sysdeps/unix/sysv/linux/sh/libc.abilist
> index 30ae3b6ebb..8652dfea59 100644
> --- a/sysdeps/unix/sysv/linux/sh/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sh/libc.abilist
> @@ -1887,6 +1887,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
> index 68b107d080..95b58dfa67 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
> @@ -1999,6 +1999,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
> index e5b6a4da50..bfd24f9d1c 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
> @@ -1940,6 +1940,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h b/sysdeps/unix/sysv/linux/sys/rseq.h
> new file mode 100644
> index 0000000000..83c8976f50
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
> @@ -0,0 +1,65 @@
Needs a one sentence description.
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +#define _SYS_RSEQ_H 1
> +
> +/* We use the structures declarations from the kernel headers. */
> +#include <linux/rseq.h>
> +/* Architecture-specific rseq signature. */
> +#include <bits/rseq.h>
> +#include <stdint.h>
OK.
> +
> +enum rseq_register_state
> +{
> + /* Value RSEQ_REGISTER_ALLOWED means it is allowed to update
> + the refcount field and to register/unregister rseq. */
> + RSEQ_REGISTER_ALLOWED = 0,
> + /* Value RSEQ_REGISTER_ONGOING means a rseq registration is in progress,
> + so it is temporarily forbidden to update the refcount field or to
> + register/unregister rseq for this thread or signal handlers nested
> + on this thread. */
> + RSEQ_REGISTER_ONGOING = 1,
> + /* Value RSEQ_REGISTER_EXITING means it is forbidden to update the
> + refcount field or to register/unregister rseq for the rest of the
> + thread's lifetime. */
> + RSEQ_REGISTER_EXITING = 2,
OK.
> +};
> +
> +struct rseq_lib_abi
> +{
> + uint32_t register_state; /* enum rseq_register_state. */
> + /* The refcount field keeps track of rseq users, so early adopters
> + of rseq can cooperate amongst each other and with glibc to
> + share rseq thread registration. The refcount field can only be
> + updated when allowed by the value of field register_state.
> + Registering rseq should be performed when incrementing refcount
> + from 0 to 1, and unregistering rseq should be performed when
> + decrementing refcount from 1 to 0. */
OK.
> + uint32_t refcount;
> +};
> +
> +/* volatile because fields can be read/updated by the kernel. */
> +extern __thread volatile struct rseq __rseq_abi
> +__attribute__ ((tls_model ("initial-exec")));
OK.
> +
> +/* volatile because fields can be read/updated by signal handlers. */
> +extern __thread volatile struct rseq_lib_abi __rseq_lib_abi
> +__attribute__ ((tls_model ("initial-exec")));
OK.
> +
> +#endif /* sys/rseq.h */
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/rseq.h b/sysdeps/unix/sysv/linux/x86/bits/rseq.h
> new file mode 100644
> index 0000000000..19d3755837
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/bits/rseq.h
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
Add one line short description.
> + This file is part of the GNU C Library.
> + Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* Signature required before each abort handler code. */
> +#define RSEQ_SIG 0x53053053
Why not an x86-specific op code?
> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
> index 86dfb0c94d..e9f8411fb2 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
> @@ -1898,6 +1898,8 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
> GLIBC_2.3 __ctype_b_loc F
> GLIBC_2.3 __ctype_tolower_loc F
> GLIBC_2.3 __ctype_toupper_loc F
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
> index dd688263aa..f9432d07f1 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
> @@ -2149,3 +2149,5 @@ GLIBC_2.28 thrd_yield F
> GLIBC_2.29 getcpu F
> GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
> GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
> +GLIBC_2.30 __rseq_abi T 0x20
> +GLIBC_2.30 __rseq_lib_abi T 0x8
OK.
>
--
Cheers,
Carlos.
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Christoph Paasch @ 2019-03-22 19:25 UTC (permalink / raw)
To: Paolo Abeni
Cc: Alexander Duyck, netdev, LKML, Samudrala, Sridhar, Eric Dumazet,
David Miller, Linux API
In-Reply-To: <0eb092b7ca67942f52e36c672d20f130f1d54e1e.camel@redhat.com>
Hi Paolo,
On Fri, Mar 22, 2019 at 6:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
> On Thu, 2019-03-21 at 23:05 -0400, Christoph Paasch wrote:
> > On Thu, Mar 21, 2019 at 12:43 PM Alexander Duyck
> > <alexander.duyck@gmail.com> wrote:
> > > On Thu, Mar 21, 2019 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > > The following - completely untested - should avoid the unbounded loop,
> > > > but it's not a complete fix, I *think* we should also change
> > > > sk_busy_loop_end() in a similar way, but that is a little more complex
> > > > due to the additional indirections.
> > >
> > > As far as sk_busy_loop_end we could look at just forking sk_busy_loop
> > > and writing a separate implementation for datagram sockets that uses a
> > > different loop_end function. It shouldn't take much to change since
> > > all we would need to do is pass a structure containing the sk and last
> > > pointers instead of just passing the sk directly as the loop_end
> > > argument.
> > >
> > > > Could you please test it?
> > > >
> > > > Any feedback welcome!
> > >
> > > The change below looks good to me.
> >
> > I just tried it out. Worked for me!
> >
> > You can add my Tested-by if you do a formal patch-submission:
> >
> > Tested-by: Christoph Paasch <cpaasch@apple.com>
>
> Thanks for testing!
>
> I'm trying to reproduce the issue locally, but I'm unable. I think that
> the current UDP implementation is not affected, as we always ensure
> sk_receive_queue is empty before busy polling. Unix sockets should not
> be affected, too, as busy polling should not have any effect there
> (sk_napi_id should be never >= MIN_NAPI_ID). Can you reproduce the
> issue on an unpatched, recent, upstream kernel?
yes, I can repro it with the C-reproducer reliably on the latest
net-branch, v4.14.105 and then also bisected it down to the commit.
> Can you please provide the syzkaller repro?
Sure, here is the full report:
Syzkaller hit 'INFO: rcu detected stall in unix_seqpacket_recvmsg' bug.
INFO: rcu_sched self-detected stall on CPU
1-...: (19373 ticks this GP) idle=ac6/140000000000001/0 softirq=4477/4477 fqs=4
(t=21000 jiffies g=1853 c=1852 q=16)
rcu_sched kthread starved for 20984 jiffies! g1853 c1852 f0x0
RCU_GP_WAIT_FQS(3) ->state=0x0 ->cpu=0
rcu_sched R running task 21040 8 2 0x80000000
Call Trace:
schedule+0xee/0x3a0 kernel/sched/core.c:3427
schedule_timeout+0x158/0x260 kernel/time/timer.c:1744
rcu_gp_kthread+0x12fc/0x3580 kernel/rcu/tree.c:2247
kthread+0x355/0x430 kernel/kthread.c:232
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:402
NMI backtrace for cpu 1
CPU: 1 PID: 1956 Comm: syz-executor808 Not tainted 4.14.105 #6
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.5.1 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x10a/0x1d1 lib/dump_stack.c:53
nmi_cpu_backtrace+0xf2/0x110 lib/nmi_backtrace.c:101
nmi_trigger_cpumask_backtrace+0x116/0x170 lib/nmi_backtrace.c:62
trigger_single_cpu_backtrace include/linux/nmi.h:158 [inline]
rcu_dump_cpu_stacks+0x180/0x1dd kernel/rcu/tree.c:1396
print_cpu_stall kernel/rcu/tree.c:1542 [inline]
check_cpu_stall.isra.69+0x9f1/0x1080 kernel/rcu/tree.c:1610
__rcu_pending kernel/rcu/tree.c:3382 [inline]
rcu_pending kernel/rcu/tree.c:3444 [inline]
rcu_check_callbacks+0x380/0xc90 kernel/rcu/tree.c:2784
update_process_times+0x28/0x60 kernel/time/timer.c:1588
tick_sched_handle+0x7d/0x150 kernel/time/tick-sched.c:161
tick_sched_timer+0x3d/0x110 kernel/time/tick-sched.c:1219
__run_hrtimer kernel/time/hrtimer.c:1220 [inline]
__hrtimer_run_queues+0x3d8/0xb80 kernel/time/hrtimer.c:1284
hrtimer_interrupt+0x1b0/0x590 kernel/time/hrtimer.c:1318
local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1037 [inline]
smp_apic_timer_interrupt+0x1d5/0x5b0 arch/x86/kernel/apic/apic.c:1062
apic_timer_interrupt+0x87/0x90 arch/x86/entry/entry_64.S:787
</IRQ>
RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:778 [inline]
RIP: 0010:__raw_spin_unlock_irqrestore
include/linux/spinlock_api_smp.h:160 [inline]
RIP: 0010:_raw_spin_unlock_irqrestore+0x32/0x60 kernel/locking/spinlock.c:192
RSP: 0018:ffff888065b273b0 EFLAGS: 00000297 ORIG_RAX: ffffffffffffff10
RAX: 0000000000000007 RBX: 0000000000000297 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000297
RBP: ffff88806a2447a0 R08: 1ffff1100cb64e57 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff888065b27450
R13: ffffed100cb64e92 R14: 0000000000000000 R15: ffff88806a244788
spin_unlock_irqrestore include/linux/spinlock.h:372 [inline]
__skb_try_recv_datagram+0x299/0x4b0 net/core/datagram.c:274
unix_dgram_recvmsg+0x294/0x1840 net/unix/af_unix.c:2107
unix_seqpacket_recvmsg+0x82/0xb0 net/unix/af_unix.c:2073
sock_recvmsg_nosec net/socket.c:818 [inline]
sock_recvmsg+0xc4/0x110 net/socket.c:825
___sys_recvmsg+0x2a7/0x620 net/socket.c:2220
__sys_recvmsg+0xc6/0x200 net/socket.c:2265
SYSC_recvmsg net/socket.c:2277 [inline]
SyS_recvmsg+0x27/0x40 net/socket.c:2272
do_syscall_64+0x23f/0x6d0 arch/x86/entry/common.c:289
entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x7f4a53385469
RSP: 002b:00007f4a53a76f28 EFLAGS: 00000246 ORIG_RAX: 000000000000002f
RAX: ffffffffffffffda RBX: 0000000000603168 RCX: 00007f4a53385469
RDX: 0000000040000002 RSI: 0000000020001680 RDI: 0000000000000003
RBP: 0000000000603160 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000060316c
R13: 0800000000008923 R14: 00007f4a53a57000 R15: 0000000000000003
Syzkaller reproducer:
# {Threaded:true Collide:false Repeat:true RepeatTimes:0 Procs:8
Sandbox:none Fault:false FaultCall:-1 FaultNth:0 EnableTun:false
UseTmpDir:false EnableCgroups:false EnableNetdev:false ResetNet:false
HandleSegv:false Repro:false Trace:false}
socketpair$unix(0x1, 0x1000000000005, 0x0,
&(0x7f0000000340)={<r0=>0xffffffffffffffff, <r1=>0xffffffffffffffff})
setsockopt$sock_int(r0, 0x1, 0x2a, &(0x7f00000000c0)=0x9, 0x4)
recvmsg(r0, &(0x7f0000001680)={0x0, 0x0, 0x0}, 0x40000002)
setsockopt$sock_int(r0, 0x1, 0x2e, &(0x7f00000003c0)=0x9, 0x4)
sendmsg(r1, &(0x7f0000000680)={0x0, 0x0, 0x0}, 0x0)
connect$inet6(0xffffffffffffffff, 0x0, 0x0)
socketpair(0xa, 0x800, 0x4, 0x0)
fcntl$getflags(0xffffffffffffffff, 0x40b)
sendto$inet6(0xffffffffffffffff, 0x0, 0x0, 0x24000855, 0x0, 0x0)
getsockopt$inet_IP_XFRM_POLICY(0xffffffffffffff9c, 0x0, 0x11, 0x0, 0x0)
socket$nl_xfrm(0x10, 0x3, 0x6)
setsockopt$netlink_NETLINK_BROADCAST_ERROR(0xffffffffffffffff, 0x10e,
0x4, 0x0, 0x0)
socket$unix(0x1, 0x3, 0x0)
getsockopt$sock_int(0xffffffffffffffff, 0x1, 0x1e, 0x0, 0x0)
ioctl$sock_SIOCGSKNS(0xffffffffffffffff, 0x894c, 0x0)
ioctl$sock_inet_udp_SIOCOUTQ(0xffffffffffffffff, 0x5411, 0x0)
setsockopt$IP_VS_SO_SET_EDIT(0xffffffffffffffff, 0x0, 0x483, 0x0, 0x0)
setsockopt$inet6_IPV6_XFRM_POLICY(0xffffffffffffffff, 0x29, 0x23, 0x0, 0x0)
ioctl$sock_ifreq(0xffffffffffffffff, 0x800000000008923, 0x0)
bind(0xffffffffffffffff, 0x0, 0x0)
socket(0xa, 0x3, 0x3)
C reproducer:
// autogenerated by syzkaller (https://github.com/google/syzkaller)
#define _GNU_SOURCE
#include <dirent.h>
#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mount.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>
#include <linux/futex.h>
unsigned long long procid;
static void sleep_ms(uint64_t ms)
{
usleep(ms * 1000);
}
static uint64_t current_time_ms(void)
{
struct timespec ts;
if (clock_gettime(CLOCK_MONOTONIC, &ts))
exit(1);
return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000;
}
static void thread_start(void* (*fn)(void*), void* arg)
{
pthread_t th;
pthread_attr_t attr;
pthread_attr_init(&attr);
pthread_attr_setstacksize(&attr, 128 << 10);
int i;
for (i = 0; i < 100; i++) {
if (pthread_create(&th, &attr, fn, arg) == 0) {
pthread_attr_destroy(&attr);
return;
}
if (errno == EAGAIN) {
usleep(50);
continue;
}
break;
}
exit(1);
}
typedef struct {
int state;
} event_t;
static void event_init(event_t* ev)
{
ev->state = 0;
}
static void event_reset(event_t* ev)
{
ev->state = 0;
}
static void event_set(event_t* ev)
{
if (ev->state)
exit(1);
__atomic_store_n(&ev->state, 1, __ATOMIC_RELEASE);
syscall(SYS_futex, &ev->state, FUTEX_WAKE | FUTEX_PRIVATE_FLAG);
}
static void event_wait(event_t* ev)
{
while (!__atomic_load_n(&ev->state, __ATOMIC_ACQUIRE))
syscall(SYS_futex, &ev->state, FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0, 0);
}
static int event_isset(event_t* ev)
{
return __atomic_load_n(&ev->state, __ATOMIC_ACQUIRE);
}
static int event_timedwait(event_t* ev, uint64_t timeout)
{
uint64_t start = current_time_ms();
uint64_t now = start;
for (;;) {
uint64_t remain = timeout - (now - start);
struct timespec ts;
ts.tv_sec = remain / 1000;
ts.tv_nsec = (remain % 1000) * 1000 * 1000;
syscall(SYS_futex, &ev->state, FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0, &ts);
if (__atomic_load_n(&ev->state, __ATOMIC_RELAXED))
return 1;
now = current_time_ms();
if (now - start > timeout)
return 0;
}
}
static bool write_file(const char* file, const char* what, ...)
{
char buf[1024];
va_list args;
va_start(args, what);
vsnprintf(buf, sizeof(buf), what, args);
va_end(args);
buf[sizeof(buf) - 1] = 0;
int len = strlen(buf);
int fd = open(file, O_WRONLY | O_CLOEXEC);
if (fd == -1)
return false;
if (write(fd, buf, len) != len) {
int err = errno;
close(fd);
errno = err;
return false;
}
close(fd);
return true;
}
static void setup_common()
{
if (mount(0, "/sys/fs/fuse/connections", "fusectl", 0, 0)) {
}
}
static void loop();
static void sandbox_common()
{
prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
setpgrp();
setsid();
struct rlimit rlim;
rlim.rlim_cur = rlim.rlim_max = (200 << 20);
setrlimit(RLIMIT_AS, &rlim);
rlim.rlim_cur = rlim.rlim_max = 32 << 20;
setrlimit(RLIMIT_MEMLOCK, &rlim);
rlim.rlim_cur = rlim.rlim_max = 136 << 20;
setrlimit(RLIMIT_FSIZE, &rlim);
rlim.rlim_cur = rlim.rlim_max = 1 << 20;
setrlimit(RLIMIT_STACK, &rlim);
rlim.rlim_cur = rlim.rlim_max = 0;
setrlimit(RLIMIT_CORE, &rlim);
rlim.rlim_cur = rlim.rlim_max = 256;
setrlimit(RLIMIT_NOFILE, &rlim);
if (unshare(CLONE_NEWNS)) {
}
if (unshare(CLONE_NEWIPC)) {
}
if (unshare(0x02000000)) {
}
if (unshare(CLONE_NEWUTS)) {
}
if (unshare(CLONE_SYSVSEM)) {
}
typedef struct {
const char* name;
const char* value;
} sysctl_t;
static const sysctl_t sysctls[] = {
{"/proc/sys/kernel/shmmax", "16777216"},
{"/proc/sys/kernel/shmall", "536870912"},
{"/proc/sys/kernel/shmmni", "1024"},
{"/proc/sys/kernel/msgmax", "8192"},
{"/proc/sys/kernel/msgmni", "1024"},
{"/proc/sys/kernel/msgmnb", "1024"},
{"/proc/sys/kernel/sem", "1024 1048576 500 1024"},
};
unsigned i;
for (i = 0; i < sizeof(sysctls) / sizeof(sysctls[0]); i++)
write_file(sysctls[i].name, sysctls[i].value);
}
int wait_for_loop(int pid)
{
if (pid < 0)
exit(1);
int status = 0;
while (waitpid(-1, &status, __WALL) != pid) {
}
return WEXITSTATUS(status);
}
static int do_sandbox_none(void)
{
if (unshare(CLONE_NEWPID)) {
}
int pid = fork();
if (pid != 0)
return wait_for_loop(pid);
setup_common();
sandbox_common();
if (unshare(CLONE_NEWNET)) {
}
loop();
exit(1);
}
static void kill_and_wait(int pid, int* status)
{
kill(-pid, SIGKILL);
kill(pid, SIGKILL);
int i;
for (i = 0; i < 100; i++) {
if (waitpid(-1, status, WNOHANG | __WALL) == pid)
return;
usleep(1000);
}
DIR* dir = opendir("/sys/fs/fuse/connections");
if (dir) {
for (;;) {
struct dirent* ent = readdir(dir);
if (!ent)
break;
if (strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0)
continue;
char abort[300];
snprintf(abort, sizeof(abort), "/sys/fs/fuse/connections/%s/abort",
ent->d_name);
int fd = open(abort, O_WRONLY);
if (fd == -1) {
continue;
}
if (write(fd, abort, 1) < 0) {
}
close(fd);
}
closedir(dir);
} else {
}
while (waitpid(-1, status, __WALL) != pid) {
}
}
#define SYZ_HAVE_SETUP_TEST 1
static void setup_test()
{
prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
setpgrp();
}
#define SYZ_HAVE_RESET_TEST 1
static void reset_test()
{
int fd;
for (fd = 3; fd < 30; fd++)
close(fd);
}
struct thread_t {
int created, call;
event_t ready, done;
};
static struct thread_t threads[16];
static void execute_call(int call);
static int running;
static void* thr(void* arg)
{
struct thread_t* th = (struct thread_t*)arg;
for (;;) {
event_wait(&th->ready);
event_reset(&th->ready);
execute_call(th->call);
__atomic_fetch_sub(&running, 1, __ATOMIC_RELAXED);
event_set(&th->done);
}
return 0;
}
static void execute_one(void)
{
int i, call, thread;
for (call = 0; call < 21; call++) {
for (thread = 0; thread < (int)(sizeof(threads) / sizeof(threads[0]));
thread++) {
struct thread_t* th = &threads[thread];
if (!th->created) {
th->created = 1;
event_init(&th->ready);
event_init(&th->done);
event_set(&th->done);
thread_start(thr, th);
}
if (!event_isset(&th->done))
continue;
event_reset(&th->done);
th->call = call;
__atomic_fetch_add(&running, 1, __ATOMIC_RELAXED);
event_set(&th->ready);
event_timedwait(&th->done, 45);
break;
}
}
for (i = 0; i < 100 && __atomic_load_n(&running, __ATOMIC_RELAXED); i++)
sleep_ms(1);
}
static void execute_one(void);
#define WAIT_FLAGS __WALL
static void loop(void)
{
int iter;
for (iter = 0;; iter++) {
int pid = fork();
if (pid < 0)
exit(1);
if (pid == 0) {
setup_test();
execute_one();
reset_test();
exit(0);
}
int status = 0;
uint64_t start = current_time_ms();
for (;;) {
if (waitpid(-1, &status, WNOHANG | WAIT_FLAGS) == pid)
break;
sleep_ms(1);
if (current_time_ms() - start < 5 * 1000)
continue;
kill_and_wait(pid, &status);
break;
}
}
}
uint64_t r[2] = {0xffffffffffffffff, 0xffffffffffffffff};
void execute_call(int call)
{
long res; switch (call) {
case 0:
res = syscall(__NR_socketpair, 1, 0x1000000000005, 0, 0x20000340);
if (res != -1) {
r[0] = *(uint32_t*)0x20000340;
r[1] = *(uint32_t*)0x20000344;
}
break;
case 1:
*(uint32_t*)0x200000c0 = 9;
syscall(__NR_setsockopt, r[0], 1, 0x2a, 0x200000c0, 4);
break;
case 2:
*(uint64_t*)0x20001680 = 0;
*(uint32_t*)0x20001688 = 0;
*(uint64_t*)0x20001690 = 0;
*(uint64_t*)0x20001698 = 0;
*(uint64_t*)0x200016a0 = 0;
*(uint64_t*)0x200016a8 = 0;
*(uint32_t*)0x200016b0 = 0;
syscall(__NR_recvmsg, r[0], 0x20001680, 0x40000002);
break;
case 3:
*(uint32_t*)0x200003c0 = 9;
syscall(__NR_setsockopt, r[0], 1, 0x2e, 0x200003c0, 4);
break;
case 4:
*(uint64_t*)0x20000680 = 0;
*(uint32_t*)0x20000688 = 0;
*(uint64_t*)0x20000690 = 0;
*(uint64_t*)0x20000698 = 0;
*(uint64_t*)0x200006a0 = 0;
*(uint64_t*)0x200006a8 = 0;
*(uint32_t*)0x200006b0 = 0;
syscall(__NR_sendmsg, r[1], 0x20000680, 0);
break;
case 5:
syscall(__NR_connect, -1, 0, 0);
break;
case 6:
syscall(__NR_socketpair, 0xa, 0x800, 4, 0);
break;
case 7:
syscall(__NR_fcntl, -1, 0x40b, 0);
break;
case 8:
syscall(__NR_sendto, -1, 0, 0, 0x24000855, 0, 0);
break;
case 9:
syscall(__NR_getsockopt, 0xffffff9c, 0, 0x11, 0, 0);
break;
case 10:
syscall(__NR_socket, 0x10, 3, 6);
break;
case 11:
syscall(__NR_setsockopt, -1, 0x10e, 4, 0, 0);
break;
case 12:
syscall(__NR_socket, 1, 3, 0);
break;
case 13:
syscall(__NR_getsockopt, -1, 1, 0x1e, 0, 0);
break;
case 14:
syscall(__NR_ioctl, -1, 0x894c, 0);
break;
case 15:
syscall(__NR_ioctl, -1, 0x5411, 0);
break;
case 16:
syscall(__NR_setsockopt, -1, 0, 0x483, 0, 0);
break;
case 17:
syscall(__NR_setsockopt, -1, 0x29, 0x23, 0, 0);
break;
case 18:
syscall(__NR_ioctl, -1, 0x800000000008923, 0);
break;
case 19:
syscall(__NR_bind, -1, 0, 0);
break;
case 20:
syscall(__NR_socket, 0xa, 3, 3);
break;
}
}
int main(void)
{
syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
for (procid = 0; procid < 8; procid++) {
if (fork() == 0) {
do_sandbox_none();
}
}
sleep(1000000);
return 0;
}
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Paolo Abeni @ 2019-03-22 13:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christoph Paasch, Alexander Duyck, netdev, LKML,
Samudrala, Sridhar, David Miller, Linux API
In-Reply-To: <CANn89iJhzT4hkn38oK77J0L464eNgiXZmongFnRuRnSPcVnGbQ@mail.gmail.com>
On Fri, 2019-03-22 at 05:59 -0700, Eric Dumazet wrote:
> On Fri, Mar 22, 2019 at 3:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > Hi,
> >
> > On Thu, 2019-03-21 at 23:05 -0400, Christoph Paasch wrote:
> > > On Thu, Mar 21, 2019 at 12:43 PM Alexander Duyck
> > > <alexander.duyck@gmail.com> wrote:
> > > > On Thu, Mar 21, 2019 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > > > The following - completely untested - should avoid the unbounded loop,
> > > > > but it's not a complete fix, I *think* we should also change
> > > > > sk_busy_loop_end() in a similar way, but that is a little more complex
> > > > > due to the additional indirections.
> > > >
> > > > As far as sk_busy_loop_end we could look at just forking sk_busy_loop
> > > > and writing a separate implementation for datagram sockets that uses a
> > > > different loop_end function. It shouldn't take much to change since
> > > > all we would need to do is pass a structure containing the sk and last
> > > > pointers instead of just passing the sk directly as the loop_end
> > > > argument.
> > > >
> > > > > Could you please test it?
> > > > >
> > > > > Any feedback welcome!
> > > >
> > > > The change below looks good to me.
> > >
> > > I just tried it out. Worked for me!
> > >
> > > You can add my Tested-by if you do a formal patch-submission:
> > >
> > > Tested-by: Christoph Paasch <cpaasch@apple.com>
> >
> > Thanks for testing!
> >
> > I'm trying to reproduce the issue locally, but I'm unable. I think that
> > the current UDP implementation is not affected, as we always ensure
> > sk_receive_queue is empty before busy polling.
>
> But right after check is done we release the queue lock, so a packet might
> come right after the test has been done.
Yep I was unclear and uncorrect. My point is: with the current UDP
implementation, if we have a non empty sk_receive_queue after the busy
loop, it always means there are more packets to be processed and we
should loop again, as we currently do.
AFAICS, that is different from the reported issue, where the system
stall looping around sk_receive_queue while no new packets are appended
there.
Cheers,
Paol
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Eric Dumazet @ 2019-03-22 12:59 UTC (permalink / raw)
To: Paolo Abeni
Cc: Christoph Paasch, Alexander Duyck, netdev, LKML,
Samudrala, Sridhar, David Miller, Linux API
In-Reply-To: <0eb092b7ca67942f52e36c672d20f130f1d54e1e.camel@redhat.com>
On Fri, Mar 22, 2019 at 3:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On Thu, 2019-03-21 at 23:05 -0400, Christoph Paasch wrote:
> > On Thu, Mar 21, 2019 at 12:43 PM Alexander Duyck
> > <alexander.duyck@gmail.com> wrote:
> > > On Thu, Mar 21, 2019 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > > The following - completely untested - should avoid the unbounded loop,
> > > > but it's not a complete fix, I *think* we should also change
> > > > sk_busy_loop_end() in a similar way, but that is a little more complex
> > > > due to the additional indirections.
> > >
> > > As far as sk_busy_loop_end we could look at just forking sk_busy_loop
> > > and writing a separate implementation for datagram sockets that uses a
> > > different loop_end function. It shouldn't take much to change since
> > > all we would need to do is pass a structure containing the sk and last
> > > pointers instead of just passing the sk directly as the loop_end
> > > argument.
> > >
> > > > Could you please test it?
> > > >
> > > > Any feedback welcome!
> > >
> > > The change below looks good to me.
> >
> > I just tried it out. Worked for me!
> >
> > You can add my Tested-by if you do a formal patch-submission:
> >
> > Tested-by: Christoph Paasch <cpaasch@apple.com>
>
> Thanks for testing!
>
> I'm trying to reproduce the issue locally, but I'm unable. I think that
> the current UDP implementation is not affected, as we always ensure
> sk_receive_queue is empty before busy polling.
But right after check is done we release the queue lock, so a packet might
come right after the test has been done.
> Unix sockets should not
> be affected, too, as busy polling should not have any effect there
> (sk_napi_id should be never >= MIN_NAPI_ID). Can you reproduce the
> issue on an unpatched, recent, upstream kernel?
>
> Can you please provide the syzkaller repro?
>
> Thanks,
>
> Paolo
>
>
>
>
>
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Paolo Abeni @ 2019-03-22 10:33 UTC (permalink / raw)
To: Christoph Paasch, Alexander Duyck
Cc: netdev, LKML, Samudrala, Sridhar, Eric Dumazet, David Miller,
Linux API
In-Reply-To: <CALMXkpZ_eB+aRx3+aKM5Bs4RwDfzPNX+XneESo2-yuMPjHV5-Q@mail.gmail.com>
Hi,
On Thu, 2019-03-21 at 23:05 -0400, Christoph Paasch wrote:
> On Thu, Mar 21, 2019 at 12:43 PM Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > On Thu, Mar 21, 2019 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > The following - completely untested - should avoid the unbounded loop,
> > > but it's not a complete fix, I *think* we should also change
> > > sk_busy_loop_end() in a similar way, but that is a little more complex
> > > due to the additional indirections.
> >
> > As far as sk_busy_loop_end we could look at just forking sk_busy_loop
> > and writing a separate implementation for datagram sockets that uses a
> > different loop_end function. It shouldn't take much to change since
> > all we would need to do is pass a structure containing the sk and last
> > pointers instead of just passing the sk directly as the loop_end
> > argument.
> >
> > > Could you please test it?
> > >
> > > Any feedback welcome!
> >
> > The change below looks good to me.
>
> I just tried it out. Worked for me!
>
> You can add my Tested-by if you do a formal patch-submission:
>
> Tested-by: Christoph Paasch <cpaasch@apple.com>
Thanks for testing!
I'm trying to reproduce the issue locally, but I'm unable. I think that
the current UDP implementation is not affected, as we always ensure
sk_receive_queue is empty before busy polling. Unix sockets should not
be affected, too, as busy polling should not have any effect there
(sk_napi_id should be never >= MIN_NAPI_ID). Can you reproduce the
issue on an unpatched, recent, upstream kernel?
Can you please provide the syzkaller repro?
Thanks,
Paolo
^ permalink raw reply
* Re: [RESEND PATCH v1] moduleparam: Save information about built-in modules in separate file
From: Masahiro Yamada @ 2019-03-22 5:34 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Michal Marek, Linux Kernel Mailing List,
Linux Kbuild mailing list, linux-api, Kirill A . Shutemov,
Gleb Fotengauer-Malinovskiy, Dmitry V. Levin, Dmitry Torokhov,
Rusty Russell, Jessica Yu
In-Reply-To: <20190315101013.GN8455@Legion-PC.fortress>
Hi.
(added some people to CC)
On Fri, Mar 15, 2019 at 7:10 PM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
>
> Problem:
>
> When a kernel module is compiled as a separate module, some important
> information about the kernel module is available via .modinfo section of
> the module. In contrast, when the kernel module is compiled into the
> kernel, that information is not available.
I might be missing something, but
vmlinux provides info of builtin modules
in /sys/module/.
(Looks like currently only module_param and MODULE_VERSION)
This patch is not exactly the same, but I see a kind of overwrap.
I'd like to be sure if we want this new scheme.
> Information about built-in modules is necessary in the following cases:
>
> 1. When it is necessary to find out what additional parameters can be
> passed to the kernel at boot time.
Actually, /sys/module/<module>/parameters/
exposes this information.
Doesn't it work for your purpose?
> 2. When you need to know which module names and their aliases are in
> the kernel. This is very useful for creating an initrd image.
>
> Proposal:
>
> The proposed patch does not remove .modinfo section with module
> information from the vmlinux at the build time and saves it into a
> separate file after kernel linking. So, the kernel does not increase in
> size and no additional information remains in it. Information is stored
> in the same format as in the separate modules (null-terminated string
> array). Because the .modinfo section is already exported with a separate
> modules, we are not creating a new API.
>
> It can be easily read in the userspace:
>
> $ tr '\0' '\n' < kernel.builtin.modinfo
> ext4.softdep=pre: crc32c
> ext4.license=GPL
> ext4.description=Fourth Extended Filesystem
> ext4.author=Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others
> ext4.alias=fs-ext4
> ext4.alias=ext3
> ext4.alias=fs-ext3
> ext4.alias=ext2
> ext4.alias=fs-ext2
> md_mod.alias=block-major-9-*
> md_mod.alias=md
> md_mod.description=MD RAID framework
> md_mod.license=GPL
> md_mod.parmtype=create_on_open:bool
> md_mod.parmtype=start_dirty_degraded:int
> ...
>
> Co-Developed-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
> Signed-off-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
> Makefile | 1 +
> include/linux/moduleparam.h | 12 +++++-------
> scripts/link-vmlinux.sh | 8 ++++++++
> 3 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d5713e7b1e50..971102194c92 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1288,6 +1288,7 @@ _modinst_:
> fi
> @cp -f $(objtree)/modules.order $(MODLIB)/
> @cp -f $(objtree)/modules.builtin $(MODLIB)/
> + @cp -f $(objtree)/kernel.builtin.modinfo $(MODLIB)/
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
>
> # This depmod is only for convenience to give the initial
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index ba36506db4fb..5ba250d9172a 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -10,23 +10,21 @@
> module name. */
> #ifdef MODULE
> #define MODULE_PARAM_PREFIX /* empty */
> +#define __MODULE_INFO_PREFIX /* empty */
> #else
> #define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
> +/* We cannot use MODULE_PARAM_PREFIX because some modules override it. */
> +#define __MODULE_INFO_PREFIX KBUILD_MODNAME "."
> #endif
>
> /* Chosen so that structs with an unsigned long line up. */
> #define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
>
> -#ifdef MODULE
> #define __MODULE_INFO(tag, name, info) \
> static const char __UNIQUE_ID(name)[] \
> __used __attribute__((section(".modinfo"), unused, aligned(1))) \
> - = __stringify(tag) "=" info
> -#else /* !MODULE */
> -/* This struct is here for syntactic coherency, it is not used */
> -#define __MODULE_INFO(tag, name, info) \
> - struct __UNIQUE_ID(name) {}
> -#endif
> + = __MODULE_INFO_PREFIX __stringify(tag) "=" info
> +
> #define __MODULE_PARM_TYPE(name, _type) \
> __MODULE_INFO(parmtype, name##type, #name ":" _type)
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index c8cf45362bd6..399d7e4d11ec 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -258,10 +258,12 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
>
> # step 1
> vmlinux_link "" .tmp_vmlinux1
> + "${OBJCOPY}" -R .modinfo .tmp_vmlinux1
> kallsyms .tmp_vmlinux1 .tmp_kallsyms1.o
>
> # step 2
> vmlinux_link .tmp_kallsyms1.o .tmp_vmlinux2
> + "${OBJCOPY}" -R .modinfo .tmp_vmlinux2
> kallsyms .tmp_vmlinux2 .tmp_kallsyms2.o
>
> # step 3
> @@ -273,6 +275,7 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
> kallsyms_vmlinux=.tmp_vmlinux3
>
> vmlinux_link .tmp_kallsyms2.o .tmp_vmlinux3
> + "${OBJCOPY}" -R .modinfo .tmp_vmlinux3
>
> kallsyms .tmp_vmlinux3 .tmp_kallsyms3.o
> fi
> @@ -281,6 +284,11 @@ fi
> info LD vmlinux
> vmlinux_link "${kallsymso}" vmlinux
>
> +info MODINFO kernel.builtin.modinfo
> +"${OBJCOPY}" -j .modinfo -O binary vmlinux kernel.builtin.modinfo
> +chmod 444 kernel.builtin.modinfo
> +"${OBJCOPY}" -R .modinfo vmlinux
> +
> if [ -n "${CONFIG_BUILDTIME_EXTABLE_SORT}" ]; then
> info SORTEX vmlinux
> sortextable vmlinux
> --
> 2.19.2
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH linux-next v8 6/7] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2019-03-22 4:16 UTC (permalink / raw)
To: Oleg Nesterov, Kees Cook, Andy Lutomirski
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, Eugene Syromyatnikov,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20190322041409.GA27266-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
From: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
ptrace(2) man page:
long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
Retrieve information about the syscall that caused the stop.
The information is placed into the buffer pointed by "data"
argument, which should be a pointer to a buffer of type
"struct ptrace_syscall_info".
The "addr" argument contains the size of the buffer pointed to
by "data" argument (i.e., sizeof(struct ptrace_syscall_info)).
The return value contains the number of bytes available
to be written by the kernel.
If the size of data to be written by the kernel exceeds the size
specified by "addr" argument, the output is truncated.
Co-authored-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Reviewed-by: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Eugene Syromyatnikov <esyr-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
---
Notes:
v8:
* Rebased to linux-next.
* Moved ptrace_get_syscall_info code under #ifdef CONFIG_HAVE_ARCH_TRACEHOOK,
narrowing down the set of architectures supported by this implementation
back to those 19 that enable CONFIG_HAVE_ARCH_TRACEHOOK because
I failed to get all syscall_get_*(), instruction_pointer(),
and user_stack_pointer() functions implemented on some niche
architectures. This leaves the following architectures out:
alpha, h8300, m68k, microblaze, and unicore32.
v7: unchanged
v6:
* Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
into account, use the end of the last field of the structure being written.
* Change struct ptrace_syscall_info:
* remove .frame_pointer field, is is not needed and not portable;
* make .arch field explicitly aligned, remove no longer needed
padding before .arch field;
* remove trailing pads, they are no longer needed.
v5:
* Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
* Change struct ptrace_syscall_info: generalize instruction_pointer,
stack_pointer, and frame_pointer fields by moving them from
ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
and initializing them for all stops.
* Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
instruction_pointer when the tracee is in a signal stop.
* Make available for all architectures: do not conditionalize on
CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
are implemented on all architectures.
v4:
* Do not introduce task_struct.ptrace_event,
use child->last_siginfo->si_code instead.
* Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
ptrace_syscall_info.{entry,exit}.
v3:
* Change struct ptrace_syscall_info.
* Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
* Add proper defines for ptrace_syscall_info.op values.
* Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
* and move them to uapi.
v2:
* Do not use task->ptrace.
* Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
* Use addr argument of sys_ptrace to get expected size of the struct;
return full size of the struct.
include/linux/tracehook.h | 9 ++--
include/uapi/linux/ptrace.h | 35 ++++++++++++
kernel/ptrace.c | 103 +++++++++++++++++++++++++++++++++++-
3 files changed, 143 insertions(+), 4 deletions(-)
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index df20f8bdbfa3..6bc7a3d58e2f 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
/*
* ptrace report for syscall entry and exit looks identical.
*/
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+ unsigned long message)
{
int ptrace = current->ptrace;
if (!(ptrace & PT_PTRACED))
return 0;
+ current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
+ current->ptrace_message = 0;
return fatal_signal_pending(current);
}
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
{
- return ptrace_report_syscall(regs);
+ return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
}
/**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
- ptrace_report_syscall(regs);
+ ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
}
/**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..a71b6e3b03eb 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,41 @@ struct seccomp_metadata {
__u64 flags; /* Output: filter's flags */
};
+#define PTRACE_GET_SYSCALL_INFO 0x420e
+#define PTRACE_SYSCALL_INFO_NONE 0
+#define PTRACE_SYSCALL_INFO_ENTRY 1
+#define PTRACE_SYSCALL_INFO_EXIT 2
+#define PTRACE_SYSCALL_INFO_SECCOMP 3
+
+struct ptrace_syscall_info {
+ __u8 op; /* PTRACE_SYSCALL_INFO_* */
+ __u32 arch __attribute__((__aligned__(sizeof(__u32))));
+ __u64 instruction_pointer;
+ __u64 stack_pointer;
+ union {
+ struct {
+ __u64 nr;
+ __u64 args[6];
+ } entry;
+ struct {
+ __s64 rval;
+ __u8 is_error;
+ } exit;
+ struct {
+ __u64 nr;
+ __u64 args[6];
+ __u32 ret_data;
+ } seccomp;
+ };
+};
+
+/*
+ * These values are stored in task->ptrace_message
+ * by tracehook_report_syscall_* to describe the current syscall-stop.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1
+#define PTRACE_EVENTMSG_SYSCALL_EXIT 2
+
/* Read signals from a shared (process wide) queue */
#define PTRACE_PEEKSIGINFO_SHARED (1 << 0)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 6f357f4fc859..c4a1317410f4 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -31,6 +31,10 @@
#include <linux/compat.h>
#include <linux/sched/signal.h>
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+#include <asm/syscall.h> /* For syscall_get_* */
+#endif
+
/*
* Access another process' address space via ptrace.
* Source/target buffer must be kernel space,
@@ -879,7 +883,100 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
* to ensure no machine forgets it.
*/
EXPORT_SYMBOL_GPL(task_user_regset_view);
-#endif
+
+static unsigned long
+ptrace_get_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ unsigned long args[ARRAY_SIZE(info->entry.args)];
+ int i;
+
+ info->op = PTRACE_SYSCALL_INFO_ENTRY;
+ info->entry.nr = syscall_get_nr(child, regs);
+ syscall_get_arguments(child, regs, 0, ARRAY_SIZE(args), args);
+ for (i = 0; i < ARRAY_SIZE(args); i++)
+ info->entry.args[i] = args[i];
+
+ /* args is the last field in struct ptrace_syscall_info.entry */
+ return offsetofend(struct ptrace_syscall_info, entry.args);
+}
+
+static unsigned long
+ptrace_get_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ /*
+ * As struct ptrace_syscall_info.entry is currently a subset
+ * of struct ptrace_syscall_info.seccomp, it makes sense to
+ * initialize that subset using ptrace_get_syscall_info_entry().
+ * This can be reconsidered in the future if these structures
+ * diverge significantly enough.
+ */
+ ptrace_get_syscall_info_entry(child, regs, info);
+ info->op = PTRACE_SYSCALL_INFO_SECCOMP;
+ info->seccomp.ret_data = child->ptrace_message;
+
+ /* ret_data is the last field in struct ptrace_syscall_info.seccomp */
+ return offsetofend(struct ptrace_syscall_info, seccomp.ret_data);
+}
+
+static unsigned long
+ptrace_get_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ info->op = PTRACE_SYSCALL_INFO_EXIT;
+ info->exit.rval = syscall_get_error(child, regs);
+ info->exit.is_error = !!info->exit.rval;
+ if (!info->exit.is_error)
+ info->exit.rval = syscall_get_return_value(child, regs);
+
+ /* is_error is the last field in struct ptrace_syscall_info.exit */
+ return offsetofend(struct ptrace_syscall_info, exit.is_error);
+}
+
+static int
+ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
+ void __user *datavp)
+{
+ struct pt_regs *regs = task_pt_regs(child);
+ struct ptrace_syscall_info info = {
+ .op = PTRACE_SYSCALL_INFO_NONE,
+ .arch = syscall_get_arch(child),
+ .instruction_pointer = instruction_pointer(regs),
+ .stack_pointer = user_stack_pointer(regs),
+ };
+ unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
+ unsigned long write_size;
+
+ /*
+ * This does not need lock_task_sighand() to access
+ * child->last_siginfo because ptrace_freeze_traced()
+ * called earlier by ptrace_check_attach() ensures that
+ * the tracee cannot go away and clear its last_siginfo.
+ */
+ switch (child->last_siginfo ? child->last_siginfo->si_code : 0) {
+ case SIGTRAP | 0x80:
+ switch (child->ptrace_message) {
+ case PTRACE_EVENTMSG_SYSCALL_ENTRY:
+ actual_size = ptrace_get_syscall_info_entry(child, regs,
+ &info);
+ break;
+ case PTRACE_EVENTMSG_SYSCALL_EXIT:
+ actual_size = ptrace_get_syscall_info_exit(child, regs,
+ &info);
+ break;
+ }
+ break;
+ case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
+ actual_size = ptrace_get_syscall_info_seccomp(child, regs,
+ &info);
+ break;
+ }
+
+ write_size = min(actual_size, user_size);
+ return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
+}
+#endif /* CONFIG_HAVE_ARCH_TRACEHOOK */
int ptrace_request(struct task_struct *child, long request,
unsigned long addr, unsigned long data)
@@ -1096,6 +1193,10 @@ int ptrace_request(struct task_struct *child, long request,
ret = __put_user(kiov.iov_len, &uiov->iov_len);
break;
}
+
+ case PTRACE_GET_SYSCALL_INFO:
+ ret = ptrace_get_syscall_info(child, addr, datavp);
+ break;
#endif
case PTRACE_SECCOMP_GET_FILTER:
--
ldv
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply related
* [PATCH linux-next v8 0/7] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2019-03-22 4:14 UTC (permalink / raw)
To: Oleg Nesterov, Andy Lutomirski
Cc: Elvira Khabirova, Eugene Syromyatnikov, Benjamin Herrenschmidt,
Greentime Hu, Helge Deller, James E.J. Bottomley, James Hogan,
Kees Cook, Michael Ellerman, Paul Burton, Paul Mackerras,
Ralf Baechle, Richard Kuo, Shuah Khan, Vincent Chen, linux-api,
linux-hexagon, linux-kselftest, linux-mips, linux-parisc,
linuxppc-dev, linux-kernel
PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
PTRACE_GET_SYSCALL_INFO returns the following structure:
struct ptrace_syscall_info {
__u8 op; /* PTRACE_SYSCALL_INFO_* */
__u32 arch __attribute__((__aligned__(sizeof(__u32))));
__u64 instruction_pointer;
__u64 stack_pointer;
union {
struct {
__u64 nr;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
} exit;
struct {
__u64 nr;
__u64 args[6];
__u32 ret_data;
} seccomp;
};
};
The structure was chosen according to [2], except for the following
changes:
* seccomp substructure was added as a superset of entry substructure;
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer field was added along with instruction_pointer field
since it is readily available and can save the tracer from extra
PTRACE_GETREGS/PTRACE_GETREGSET calls;
* arch is always initialized to aid with tracing system calls
* such as execve();
* instruction_pointer and stack_pointer are always initialized
so they could be easily obtained for non-syscall stops;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.
strace has been ported to PTRACE_GET_SYSCALL_INFO.
Starting with release 4.26, strace uses PTRACE_GET_SYSCALL_INFO API
as the preferred mechanism of obtaining syscall information.
[1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg@mail.gmail.com/
[2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA@mail.gmail.com/
---
Notes:
v8:
* Moved syscall_get_arch() specific patches to a separate patchset
which is now merged into audit/next tree.
* Rebased to linux-next.
* Moved ptrace_get_syscall_info code under #ifdef CONFIG_HAVE_ARCH_TRACEHOOK,
narrowing down the set of architectures supported by this implementation
back to those 19 that enable CONFIG_HAVE_ARCH_TRACEHOOK because
I failed to get all syscall_get_*(), instruction_pointer(),
and user_stack_pointer() functions implemented on some niche
architectures. This leaves the following architectures out:
alpha, h8300, m68k, microblaze, and unicore32.
v7:
* Rebased to v5.0-rc1.
* 5 arch-specific preparatory patches out of 25 have been merged
into v5.0-rc1 via arch trees.
v6:
* Add syscall_get_arguments and syscall_set_arguments wrappers
to asm-generic/syscall.h, requested by Geert.
* Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
into account, use the end of the last field of the structure being written.
* Change struct ptrace_syscall_info:
* remove .frame_pointer field, is is not needed and not portable;
* make .arch field explicitly aligned, remove no longer needed
padding before .arch field;
* remove trailing pads, they are no longer needed.
v5:
* Merge separate series and patches into the single series.
* Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
* Change struct ptrace_syscall_info: generalize instruction_pointer,
stack_pointer, and frame_pointer fields by moving them from
ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
and initializing them for all stops.
* Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
instruction_pointer when the tracee is in a signal stop.
* Patch all remaining architectures to provide all necessary
syscall_get_* functions.
* Make available for all architectures: do not conditionalize on
CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
are implemented on all architectures.
* Add a test for PTRACE_GET_SYSCALL_INFO to selftests/ptrace.
v4:
* Do not introduce task_struct.ptrace_event,
use child->last_siginfo->si_code instead.
* Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
ptrace_syscall_info.{entry,exit}.
v3:
* Change struct ptrace_syscall_info.
* Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
* Add proper defines for ptrace_syscall_info.op values.
* Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
* and move them to uapi.
v2:
* Do not use task->ptrace.
* Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
* Use addr argument of sys_ptrace to get expected size of the struct;
return full size of the struct.
Dmitry V. Levin (6):
nds32: fix asm/syscall.h
hexagon: define syscall_get_error() and syscall_get_return_value()
mips: define syscall_get_error()
parisc: define syscall_get_error()
powerpc: define syscall_get_error()
selftests/ptrace: add a test case for PTRACE_GET_SYSCALL_INFO
Elvira Khabirova (1):
ptrace: add PTRACE_GET_SYSCALL_INFO request
arch/hexagon/include/asm/syscall.h | 14 +
arch/mips/include/asm/syscall.h | 6 +
arch/nds32/include/asm/syscall.h | 29 +-
arch/parisc/include/asm/syscall.h | 7 +
arch/powerpc/include/asm/syscall.h | 10 +
include/linux/tracehook.h | 9 +-
include/uapi/linux/ptrace.h | 35 +++
kernel/ptrace.c | 103 ++++++-
tools/testing/selftests/ptrace/.gitignore | 1 +
tools/testing/selftests/ptrace/Makefile | 2 +-
.../selftests/ptrace/get_syscall_info.c | 271 ++++++++++++++++++
11 files changed, 471 insertions(+), 16 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/get_syscall_info.c
--
ldv
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Christoph Paasch @ 2019-03-22 3:05 UTC (permalink / raw)
To: Alexander Duyck, Paolo Abeni
Cc: netdev, LKML, Samudrala, Sridhar, Eric Dumazet, David Miller,
Linux API
In-Reply-To: <CAKgT0Uc68jsuPHKbxxFpeMCQAo2M69DhzA_XGw7Kvou4H3eHPg@mail.gmail.com>
Hi,
On Thu, Mar 21, 2019 at 12:43 PM Alexander Duyck
<alexander.duyck@gmail.com> wrote:
>
> On Thu, Mar 21, 2019 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > Hi,
> >
> > On Wed, 2019-03-20 at 11:35 -0700, Christoph Paasch wrote:
> > > Hello,
> > >
> > > On Fri, Mar 24, 2017 at 3:23 PM Alexander Duyck
> > > <alexander.duyck@gmail.com> wrote:
> > > > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > > >
> > > > > From what I can tell there is only a couple spots where we are actually
> > > > checking the return value of sk_busy_loop. As there are only a few
> > > > consumers of that data, and the data being checked for can be replaced
> > > > with a check for !skb_queue_empty() we might as well just pull the code
> > > > out of sk_busy_loop and place it in the spots that actually need it.
> > > >
> > > > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > > Acked-by: Eric Dumazet <edumazet@google.com>
> > > > ---
> > > > include/net/busy_poll.h | 5 ++---
> > > > net/core/datagram.c | 8 ++++++--
> > > > net/core/dev.c | 25 +++++++++++--------------
> > > > net/sctp/socket.c | 9 ++++++---
> > > > 4 files changed, 25 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> > > > index b82d6ba70a14..c55760f4820f 100644
> > > > --- a/include/net/busy_poll.h
> > > > +++ b/include/net/busy_poll.h
> > > > @@ -74,7 +74,7 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > > > return time_after(now, end_time);
> > > > }
> > > >
> > > > -bool sk_busy_loop(struct sock *sk, int nonblock);
> > > > +void sk_busy_loop(struct sock *sk, int nonblock);
> > > >
> > > > #else /* CONFIG_NET_RX_BUSY_POLL */
> > > > static inline unsigned long net_busy_loop_on(void)
> > > > @@ -97,9 +97,8 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > > > return true;
> > > > }
> > > >
> > > > -static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> > > > +static inline void sk_busy_loop(struct sock *sk, int nonblock)
> > > > {
> > > > - return false;
> > > > }
> > > >
> > > > #endif /* CONFIG_NET_RX_BUSY_POLL */
> > > > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > > > index ea633342ab0d..4608aa245410 100644
> > > > --- a/net/core/datagram.c
> > > > +++ b/net/core/datagram.c
> > > > @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct
> > > > sock *sk, unsigned int flags,
> > > > }
> > > >
> > > > spin_unlock_irqrestore(&queue->lock, cpu_flags);
> > > > - } while (sk_can_busy_loop(sk) &&
> > > > - sk_busy_loop(sk, flags & MSG_DONTWAIT));
> > > > +
> > > > + if (!sk_can_busy_loop(sk))
> > > > + break;
> > > > +
> > > > + sk_busy_loop(sk, flags & MSG_DONTWAIT);
> > > > + } while (!skb_queue_empty(&sk->sk_receive_queue));
> > >
> > > since this change I am hitting stalls where it's looping in this
> > > while-loop with syzkaller.
> > >
> > > It worked prior to this change because sk->sk_napi_id was not set thus
> > > sk_busy_loop would make us get out of the loop.
> > >
> > > Now, it keeps on looping because there is an skb in the queue with
> > > skb->len == 0 and we are peeking with an offset, thus
> > > __skb_try_recv_from_queue will return NULL and thus we have no way of
> > > getting out of the loop.
> > >
> > > I'm not sure what would be the best way to fix it. I don't see why we
> > > end up with an skb in the list with skb->len == 0. So, shooting a
> > > quick e-mail, maybe somebody has an idea :-)
> > >
> > > I have the syzkaller-reproducer if needed.
> >
> > IIRC we can have 0 len UDP packet sitting on sk_receive_queue since:
> >
> > commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
> > Author: samanthakumar <samanthakumar@google.com>
> > Date: Tue Apr 5 12:41:15 2016 -0400
> >
> > udp: remove headers from UDP packets before queueing
> >
> > Both __skb_try_recv_datagram() and napi_busy_loop() assume that we
> > received some packets if the queue is not empty. When peeking such
> > assumption is not true, we should check if the last packet is changed,
> > as __skb_recv_datagram() already does. So I *think* the root cause of
> > this issue is older than Alex's patch.
>
> I agree.
>
> > The following - completely untested - should avoid the unbounded loop,
> > but it's not a complete fix, I *think* we should also change
> > sk_busy_loop_end() in a similar way, but that is a little more complex
> > due to the additional indirections.
>
> As far as sk_busy_loop_end we could look at just forking sk_busy_loop
> and writing a separate implementation for datagram sockets that uses a
> different loop_end function. It shouldn't take much to change since
> all we would need to do is pass a structure containing the sk and last
> pointers instead of just passing the sk directly as the loop_end
> argument.
>
> > Could you please test it?
> >
> > Any feedback welcome!
>
> The change below looks good to me.
I just tried it out. Worked for me!
You can add my Tested-by if you do a formal patch-submission:
Tested-by: Christoph Paasch <cpaasch@apple.com>
Christoph
>
> > Could you please test it?
> >
> > Paolo
> > ---
> > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > index b2651bb6d2a3..e657289db4ac 100644
> > --- a/net/core/datagram.c
> > +++ b/net/core/datagram.c
> > @@ -279,7 +279,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock
> > *sk, unsigned int flags,
> > break;
> >
> > sk_busy_loop(sk, flags & MSG_DONTWAIT);
> > - } while (!skb_queue_empty(&sk->sk_receive_queue));
> > + } while (sk->sk_receive_queue.prev != *last);
> >
> > error = -EAGAIN;
> >
> >
^ permalink raw reply
* Re: [PATCH v2 1/1] userfaultfd/sysctl: add vm.unprivileged_userfaultfd
From: Andrea Arcangeli @ 2019-03-21 21:06 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Dr. David Alan Gilbert, Andrew Morton, Peter Xu, linux-kernel,
Paolo Bonzini, Hugh Dickins, Maxime Coquelin, Maya Gokhale,
Jerome Glisse, Pavel Emelyanov, Johannes Weiner, Martin Cracauer,
Denis Plotnikov, linux-mm, Marty McFadden, Mike Kravetz,
Mike Rapoport, Kees Cook, Mel Gorman, Kirill A . Shutemov,
linux-ap
In-Reply-To: <20190321134335.GB1146@42.do-not-panic.com>
Hello,
On Thu, Mar 21, 2019 at 01:43:35PM +0000, Luis Chamberlain wrote:
> On Wed, Mar 20, 2019 at 03:01:12PM -0400, Andrea Arcangeli wrote:
> > but
> > that would be better be achieved through SECCOMP and not globally.'.
>
> That begs the question why not use seccomp for this? What if everyone
> decided to add a knob for all syscalls to do the same? For the commit
> log, why is it OK then to justify a knob for this syscall?
That's a good point and it's obviously more secure because you can
block a lot more than just bpf and userfaultfd: however not all
syscalls have CONFIG_USERFAULTFD=n or CONFIG_BPF_SYSCALL=n that you
can set to =n at build time, then they'll return -ENOSYS (implemented
as sys_ni_syscall in the =n case).
The point of the bpf (already included upstream) and userfaultfd
(proposed) sysctl is to avoid users having to rebuild the kernel if
they want to harden their setup without being forced to run all
containers under seccomp, just like they could by setting those two
config options "=n" at build time.
So you can see it like allowing a runtime selection of
CONFIG_USERFAULTFD and CONFIG_BPF_SYSCALL without the kernel build
time config forcing the decision on behalf of the end user.
Thanks,
Andrea
^ permalink raw reply
* Re: [PATCH RESEND v5 0/5] namei: vfs flags to restrict path resolution
From: Andy Lutomirski @ 2019-03-21 17:06 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells, Eric Biederman, Andy Lutomirski, Jann Horn,
Christian Brauner, David Drysdale, Tycho Andersen, Kees Cook,
Linux Containers, Linux FS Devel, Linux API, Andrew Morton,
Alexei Starovoitov, Chanho Min, Oleg Nesterov, Aleksa Sarai
In-Reply-To: <20190320143717.2523-1-cyphar@cyphar.com>
On Wed, Mar 20, 2019 at 7:38 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> Now that the holiday break is over, it's time to re-send this patch
> series (with a few additions, due to new information we got from
> CVE-2019-5736 -- which this patchset mostly protected against but had
> some holes with regards to #!-style scripts).
I generally like this, but, as Linus pointed out, it will be
unfortunate if application authors see this as just another
non-portable weird Linux API and don't use it. Would it be worthwhile
to put some thought into making it an API that other OSes might be
willing to implement? As it stands, the openat(2) flags are getting
rather crazy in this patch set.
Aleksa had a resolveat(2) proposal that really didn't seem too bad.
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Alexander Duyck @ 2019-03-21 16:43 UTC (permalink / raw)
To: Paolo Abeni
Cc: Christoph Paasch, netdev, LKML, Samudrala, Sridhar, Eric Dumazet,
David Miller, Linux API
In-Reply-To: <e532fdb97e142eca2484e5abbeca4c30d11fbad6.camel@redhat.com>
On Thu, Mar 21, 2019 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On Wed, 2019-03-20 at 11:35 -0700, Christoph Paasch wrote:
> > Hello,
> >
> > On Fri, Mar 24, 2017 at 3:23 PM Alexander Duyck
> > <alexander.duyck@gmail.com> wrote:
> > > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > >
> > > > From what I can tell there is only a couple spots where we are actually
> > > checking the return value of sk_busy_loop. As there are only a few
> > > consumers of that data, and the data being checked for can be replaced
> > > with a check for !skb_queue_empty() we might as well just pull the code
> > > out of sk_busy_loop and place it in the spots that actually need it.
> > >
> > > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > Acked-by: Eric Dumazet <edumazet@google.com>
> > > ---
> > > include/net/busy_poll.h | 5 ++---
> > > net/core/datagram.c | 8 ++++++--
> > > net/core/dev.c | 25 +++++++++++--------------
> > > net/sctp/socket.c | 9 ++++++---
> > > 4 files changed, 25 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> > > index b82d6ba70a14..c55760f4820f 100644
> > > --- a/include/net/busy_poll.h
> > > +++ b/include/net/busy_poll.h
> > > @@ -74,7 +74,7 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > > return time_after(now, end_time);
> > > }
> > >
> > > -bool sk_busy_loop(struct sock *sk, int nonblock);
> > > +void sk_busy_loop(struct sock *sk, int nonblock);
> > >
> > > #else /* CONFIG_NET_RX_BUSY_POLL */
> > > static inline unsigned long net_busy_loop_on(void)
> > > @@ -97,9 +97,8 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > > return true;
> > > }
> > >
> > > -static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> > > +static inline void sk_busy_loop(struct sock *sk, int nonblock)
> > > {
> > > - return false;
> > > }
> > >
> > > #endif /* CONFIG_NET_RX_BUSY_POLL */
> > > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > > index ea633342ab0d..4608aa245410 100644
> > > --- a/net/core/datagram.c
> > > +++ b/net/core/datagram.c
> > > @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct
> > > sock *sk, unsigned int flags,
> > > }
> > >
> > > spin_unlock_irqrestore(&queue->lock, cpu_flags);
> > > - } while (sk_can_busy_loop(sk) &&
> > > - sk_busy_loop(sk, flags & MSG_DONTWAIT));
> > > +
> > > + if (!sk_can_busy_loop(sk))
> > > + break;
> > > +
> > > + sk_busy_loop(sk, flags & MSG_DONTWAIT);
> > > + } while (!skb_queue_empty(&sk->sk_receive_queue));
> >
> > since this change I am hitting stalls where it's looping in this
> > while-loop with syzkaller.
> >
> > It worked prior to this change because sk->sk_napi_id was not set thus
> > sk_busy_loop would make us get out of the loop.
> >
> > Now, it keeps on looping because there is an skb in the queue with
> > skb->len == 0 and we are peeking with an offset, thus
> > __skb_try_recv_from_queue will return NULL and thus we have no way of
> > getting out of the loop.
> >
> > I'm not sure what would be the best way to fix it. I don't see why we
> > end up with an skb in the list with skb->len == 0. So, shooting a
> > quick e-mail, maybe somebody has an idea :-)
> >
> > I have the syzkaller-reproducer if needed.
>
> IIRC we can have 0 len UDP packet sitting on sk_receive_queue since:
>
> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
> Author: samanthakumar <samanthakumar@google.com>
> Date: Tue Apr 5 12:41:15 2016 -0400
>
> udp: remove headers from UDP packets before queueing
>
> Both __skb_try_recv_datagram() and napi_busy_loop() assume that we
> received some packets if the queue is not empty. When peeking such
> assumption is not true, we should check if the last packet is changed,
> as __skb_recv_datagram() already does. So I *think* the root cause of
> this issue is older than Alex's patch.
I agree.
> The following - completely untested - should avoid the unbounded loop,
> but it's not a complete fix, I *think* we should also change
> sk_busy_loop_end() in a similar way, but that is a little more complex
> due to the additional indirections.
As far as sk_busy_loop_end we could look at just forking sk_busy_loop
and writing a separate implementation for datagram sockets that uses a
different loop_end function. It shouldn't take much to change since
all we would need to do is pass a structure containing the sk and last
pointers instead of just passing the sk directly as the loop_end
argument.
> Could you please test it?
>
> Any feedback welcome!
The change below looks good to me.
> Could you please test it?
>
> Paolo
> ---
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index b2651bb6d2a3..e657289db4ac 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -279,7 +279,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock
> *sk, unsigned int flags,
> break;
>
> sk_busy_loop(sk, flags & MSG_DONTWAIT);
> - } while (!skb_queue_empty(&sk->sk_receive_queue));
> + } while (sk->sk_receive_queue.prev != *last);
>
> error = -EAGAIN;
>
>
^ permalink raw reply
* [PATCH] fanotify: Make wait for permission events interruptible
From: Jan Kara @ 2019-03-21 15:11 UTC (permalink / raw)
To: linux-fsdevel
Cc: Orion Poplawski, Vivek Trivedi, Amir Goldstein, linux-api, LKML,
Eric W. Biederman, Jan Kara
Switch waiting for response to fanotify permission events interruptible.
This allows e.g. the system to be suspended while there are some
fanotify permission events pending (which is reportedly pretty common
when for example AV solution is in use). However just making the wait
interruptible can result in e.g. open(2) returning -EINTR where
previously such error code never happened in practice. To avoid
confusion of userspace due to this error code, return -ERESTARTNOINTR
instead.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/notify/fanotify/fanotify.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
Orion, can you give this patch some testing with your usecase? Also if anybody
sees any issue with returning -ERESTARTNOINTR I have missed, please speak up.
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 6b9c27548997..eb790853844b 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -92,10 +92,17 @@ static int fanotify_get_response(struct fsnotify_group *group,
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- ret = wait_event_killable(group->fanotify_data.access_waitq,
- event->state == FAN_EVENT_ANSWERED);
+ ret = wait_event_interruptible(group->fanotify_data.access_waitq,
+ event->state == FAN_EVENT_ANSWERED);
/* Signal pending? */
if (ret < 0) {
+ /*
+ * Force restarting a syscall so that this is mostly invisible
+ * for userspace which is not prepared for handling EINTR e.g.
+ * from open(2).
+ */
+ if (ret == -ERESTARTSYS)
+ ret = -ERESTARTNOINTR;
spin_lock(&group->notification_lock);
/* Event reported to userspace and no answer yet? */
if (event->state == FAN_EVENT_REPORTED) {
--
2.16.4
^ permalink raw reply related
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Willem de Bruijn @ 2019-03-21 14:28 UTC (permalink / raw)
To: Paolo Abeni
Cc: Christoph Paasch, Alexander Duyck, netdev, LKML,
Samudrala, Sridhar, Eric Dumazet, David Miller, Linux API,
Alexei Starovoitov
In-Reply-To: <e532fdb97e142eca2484e5abbeca4c30d11fbad6.camel@redhat.com>
On Thu, Mar 21, 2019 at 5:46 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On Wed, 2019-03-20 at 11:35 -0700, Christoph Paasch wrote:
> > Hello,
> >
> > On Fri, Mar 24, 2017 at 3:23 PM Alexander Duyck
> > <alexander.duyck@gmail.com> wrote:
> > > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > >
> > > > From what I can tell there is only a couple spots where we are actually
> > > checking the return value of sk_busy_loop. As there are only a few
> > > consumers of that data, and the data being checked for can be replaced
> > > with a check for !skb_queue_empty() we might as well just pull the code
> > > out of sk_busy_loop and place it in the spots that actually need it.
> > >
> > > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > Acked-by: Eric Dumazet <edumazet@google.com>
> > > ---
> > > include/net/busy_poll.h | 5 ++---
> > > net/core/datagram.c | 8 ++++++--
> > > net/core/dev.c | 25 +++++++++++--------------
> > > net/sctp/socket.c | 9 ++++++---
> > > 4 files changed, 25 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> > > index b82d6ba70a14..c55760f4820f 100644
> > > --- a/include/net/busy_poll.h
> > > +++ b/include/net/busy_poll.h
> > > @@ -74,7 +74,7 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > > return time_after(now, end_time);
> > > }
> > >
> > > -bool sk_busy_loop(struct sock *sk, int nonblock);
> > > +void sk_busy_loop(struct sock *sk, int nonblock);
> > >
> > > #else /* CONFIG_NET_RX_BUSY_POLL */
> > > static inline unsigned long net_busy_loop_on(void)
> > > @@ -97,9 +97,8 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > > return true;
> > > }
> > >
> > > -static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> > > +static inline void sk_busy_loop(struct sock *sk, int nonblock)
> > > {
> > > - return false;
> > > }
> > >
> > > #endif /* CONFIG_NET_RX_BUSY_POLL */
> > > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > > index ea633342ab0d..4608aa245410 100644
> > > --- a/net/core/datagram.c
> > > +++ b/net/core/datagram.c
> > > @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct
> > > sock *sk, unsigned int flags,
> > > }
> > >
> > > spin_unlock_irqrestore(&queue->lock, cpu_flags);
> > > - } while (sk_can_busy_loop(sk) &&
> > > - sk_busy_loop(sk, flags & MSG_DONTWAIT));
> > > +
> > > + if (!sk_can_busy_loop(sk))
> > > + break;
> > > +
> > > + sk_busy_loop(sk, flags & MSG_DONTWAIT);
> > > + } while (!skb_queue_empty(&sk->sk_receive_queue));
> >
> > since this change I am hitting stalls where it's looping in this
> > while-loop with syzkaller.
> >
> > It worked prior to this change because sk->sk_napi_id was not set thus
> > sk_busy_loop would make us get out of the loop.
> >
> > Now, it keeps on looping because there is an skb in the queue with
> > skb->len == 0 and we are peeking with an offset, thus
> > __skb_try_recv_from_queue will return NULL and thus we have no way of
> > getting out of the loop.
> >
> > I'm not sure what would be the best way to fix it. I don't see why we
> > end up with an skb in the list with skb->len == 0. So, shooting a
> > quick e-mail, maybe somebody has an idea :-)
> > I have the syzkaller-reproducer if needed.
>
> IIRC we can have 0 len UDP packet sitting on sk_receive_queue since:
Yes, as of header before enqueue pulling zero byte datagrams may be
queued. And these need to be delivered, among other reason for their
cmsg metadata.
> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
> Author: samanthakumar <samanthakumar@google.com>
> Date: Tue Apr 5 12:41:15 2016 -0400
>
> udp: remove headers from UDP packets before queueing
>
> Both __skb_try_recv_datagram() and napi_busy_loop() assume that we
> received some packets if the queue is not empty. When peeking such
> assumption is not true, we should check if the last packet is changed,
> as __skb_recv_datagram() already does.
Good catch. The condition in sk_busy_loop_end is not easy to address.
Since busy poll is an optimization and poll at offset rare, one way
out may be to amend the __sk_can_busy_loop test in __skb_recv_udp to
disallow busy polling together with peek at offset.
The difference in behavior betwee __skb_try_recv_datagram and
__skb_recv_datagram also reminds of Alexei's earlier report (without
busy polling, seemingly with a list corruption introduced elsewhere)
in
[net-next,1/3] net/sock: factor out dequeue/peek with offset code
https://patchwork.ozlabs.org/patch/762327/
> So I *think* the root cause of
> this issue is older than Alex's patch.
>
> The following - completely untested - should avoid the unbounded loop,
> but it's not a complete fix, I *think* we should also change
> sk_busy_loop_end() in a similar way, but that is a little more complex
> due to the additional indirections.
>
> Could you please test it?
>
> Any feedback welcome!
>
>
> Could you please test it?
>
> Paolo
> ---
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index b2651bb6d2a3..e657289db4ac 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -279,7 +279,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock
> *sk, unsigned int flags,
> break;
>
> sk_busy_loop(sk, flags & MSG_DONTWAIT);
> - } while (!skb_queue_empty(&sk->sk_receive_queue));
> + } while (sk->sk_receive_queue.prev != *last);
>
> error = -EAGAIN;
>
>
^ permalink raw reply
* Re: [PATCH v2 1/1] userfaultfd/sysctl: add vm.unprivileged_userfaultfd
From: Luis Chamberlain @ 2019-03-21 13:43 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Dr. David Alan Gilbert, Andrew Morton, Peter Xu, linux-kernel,
Paolo Bonzini, Hugh Dickins, Maxime Coquelin, Maya Gokhale,
Jerome Glisse, Pavel Emelyanov, Johannes Weiner, Martin Cracauer,
Denis Plotnikov, linux-mm, Marty McFadden, Mike Kravetz,
Mike Rapoport, Kees Cook, Mel Gorman, Kirill A . Shutemov,
linux-ap
In-Reply-To: <20190320190112.GD23793@redhat.com>
On Wed, Mar 20, 2019 at 03:01:12PM -0400, Andrea Arcangeli wrote:
> but
> that would be better be achieved through SECCOMP and not globally.'.
That begs the question why not use seccomp for this? What if everyone
decided to add a knob for all syscalls to do the same? For the commit
log, why is it OK then to justify a knob for this syscall?
Luis
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Paolo Abeni @ 2019-03-21 9:45 UTC (permalink / raw)
To: Christoph Paasch, Alexander Duyck
Cc: netdev, linux-kernel, sridhar.samudrala, Eric Dumazet,
David Miller, linux-api
In-Reply-To: <CALMXkpb3MfLbNoBgLJc0rgMDbPg9yOevKOiq9N8pzVw-aeNb3g@mail.gmail.com>
Hi,
On Wed, 2019-03-20 at 11:35 -0700, Christoph Paasch wrote:
> Hello,
>
> On Fri, Mar 24, 2017 at 3:23 PM Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > > From what I can tell there is only a couple spots where we are actually
> > checking the return value of sk_busy_loop. As there are only a few
> > consumers of that data, and the data being checked for can be replaced
> > with a check for !skb_queue_empty() we might as well just pull the code
> > out of sk_busy_loop and place it in the spots that actually need it.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > Acked-by: Eric Dumazet <edumazet@google.com>
> > ---
> > include/net/busy_poll.h | 5 ++---
> > net/core/datagram.c | 8 ++++++--
> > net/core/dev.c | 25 +++++++++++--------------
> > net/sctp/socket.c | 9 ++++++---
> > 4 files changed, 25 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> > index b82d6ba70a14..c55760f4820f 100644
> > --- a/include/net/busy_poll.h
> > +++ b/include/net/busy_poll.h
> > @@ -74,7 +74,7 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > return time_after(now, end_time);
> > }
> >
> > -bool sk_busy_loop(struct sock *sk, int nonblock);
> > +void sk_busy_loop(struct sock *sk, int nonblock);
> >
> > #else /* CONFIG_NET_RX_BUSY_POLL */
> > static inline unsigned long net_busy_loop_on(void)
> > @@ -97,9 +97,8 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> > return true;
> > }
> >
> > -static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> > +static inline void sk_busy_loop(struct sock *sk, int nonblock)
> > {
> > - return false;
> > }
> >
> > #endif /* CONFIG_NET_RX_BUSY_POLL */
> > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > index ea633342ab0d..4608aa245410 100644
> > --- a/net/core/datagram.c
> > +++ b/net/core/datagram.c
> > @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct
> > sock *sk, unsigned int flags,
> > }
> >
> > spin_unlock_irqrestore(&queue->lock, cpu_flags);
> > - } while (sk_can_busy_loop(sk) &&
> > - sk_busy_loop(sk, flags & MSG_DONTWAIT));
> > +
> > + if (!sk_can_busy_loop(sk))
> > + break;
> > +
> > + sk_busy_loop(sk, flags & MSG_DONTWAIT);
> > + } while (!skb_queue_empty(&sk->sk_receive_queue));
>
> since this change I am hitting stalls where it's looping in this
> while-loop with syzkaller.
>
> It worked prior to this change because sk->sk_napi_id was not set thus
> sk_busy_loop would make us get out of the loop.
>
> Now, it keeps on looping because there is an skb in the queue with
> skb->len == 0 and we are peeking with an offset, thus
> __skb_try_recv_from_queue will return NULL and thus we have no way of
> getting out of the loop.
>
> I'm not sure what would be the best way to fix it. I don't see why we
> end up with an skb in the list with skb->len == 0. So, shooting a
> quick e-mail, maybe somebody has an idea :-)
>
> I have the syzkaller-reproducer if needed.
IIRC we can have 0 len UDP packet sitting on sk_receive_queue since:
commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
Author: samanthakumar <samanthakumar@google.com>
Date: Tue Apr 5 12:41:15 2016 -0400
udp: remove headers from UDP packets before queueing
Both __skb_try_recv_datagram() and napi_busy_loop() assume that we
received some packets if the queue is not empty. When peeking such
assumption is not true, we should check if the last packet is changed,
as __skb_recv_datagram() already does. So I *think* the root cause of
this issue is older than Alex's patch.
The following - completely untested - should avoid the unbounded loop,
but it's not a complete fix, I *think* we should also change
sk_busy_loop_end() in a similar way, but that is a little more complex
due to the additional indirections.
Could you please test it?
Any feedback welcome!
Could you please test it?
Paolo
---
diff --git a/net/core/datagram.c b/net/core/datagram.c
index b2651bb6d2a3..e657289db4ac 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -279,7 +279,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock
*sk, unsigned int flags,
break;
sk_busy_loop(sk, flags & MSG_DONTWAIT);
- } while (!skb_queue_empty(&sk->sk_receive_queue));
+ } while (sk->sk_receive_queue.prev != *last);
error = -EAGAIN;
^ permalink raw reply related
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: David Miller @ 2019-03-20 19:40 UTC (permalink / raw)
To: christoph.paasch
Cc: alexander.duyck, netdev, linux-kernel, sridhar.samudrala,
edumazet, linux-api
In-Reply-To: <CALMXkpb3MfLbNoBgLJc0rgMDbPg9yOevKOiq9N8pzVw-aeNb3g@mail.gmail.com>
From: Christoph Paasch <christoph.paasch@gmail.com>
Date: Wed, 20 Mar 2019 11:35:31 -0700
> On Fri, Mar 24, 2017 at 3:23 PM Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> diff --git a/net/core/datagram.c b/net/core/datagram.c
>> index ea633342ab0d..4608aa245410 100644
>> --- a/net/core/datagram.c
>> +++ b/net/core/datagram.c
>> @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
>> }
>>
>> spin_unlock_irqrestore(&queue->lock, cpu_flags);
>> - } while (sk_can_busy_loop(sk) &&
>> - sk_busy_loop(sk, flags & MSG_DONTWAIT));
>> +
>> + if (!sk_can_busy_loop(sk))
>> + break;
>> +
>> + sk_busy_loop(sk, flags & MSG_DONTWAIT);
>> + } while (!skb_queue_empty(&sk->sk_receive_queue));
>
> since this change I am hitting stalls where it's looping in this
> while-loop with syzkaller.
>
> It worked prior to this change because sk->sk_napi_id was not set thus
> sk_busy_loop would make us get out of the loop.
>
> Now, it keeps on looping because there is an skb in the queue with
> skb->len == 0 and we are peeking with an offset, thus
> __skb_try_recv_from_queue will return NULL and thus we have no way of
> getting out of the loop.
>
> I'm not sure what would be the best way to fix it. I don't see why we
> end up with an skb in the list with skb->len == 0. So, shooting a
> quick e-mail, maybe somebody has an idea :-)
>
> I have the syzkaller-reproducer if needed.
Just for the record, __skb_try_recv_datagram() and it's friend
__skb_try_recv_from_queue() are my least favorite functions in the
entire tree for the past year or so.
Their current design, and how they assume things about the
implementation of SKB queues, together result in all the weird
problems we keep fixing in them.
There has to be a much better way to do this.
^ permalink raw reply
* Re: [PATCH v2 1/1] userfaultfd/sysctl: add vm.unprivileged_userfaultfd
From: Andrea Arcangeli @ 2019-03-20 19:01 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Andrew Morton, Peter Xu, linux-kernel, Paolo Bonzini,
Hugh Dickins, Luis Chamberlain, Maxime Coquelin, Maya Gokhale,
Jerome Glisse, Pavel Emelyanov, Johannes Weiner, Martin Cracauer,
Denis Plotnikov, linux-mm, Marty McFadden, Mike Kravetz,
Mike Rapoport, Kees Cook, Mel Gorman, Kirill A . Shutemov,
linux-api
In-Reply-To: <20190319182822.GK2727@work-vm>
Hello,
On Tue, Mar 19, 2019 at 06:28:23PM +0000, Dr. David Alan Gilbert wrote:
> ---
> Userfaultfd can be misued to make it easier to exploit existing use-after-free
> (and similar) bugs that might otherwise only make a short window
> or race condition available. By using userfaultfd to stall a kernel
> thread, a malicious program can keep some state, that it wrote, stable
> for an extended period, which it can then access using an existing
> exploit. While it doesn't cause the exploit itself, and while it's not
> the only thing that can stall a kernel thread when accessing a memory location,
> it's one of the few that never needs priviledge.
>
> Add a flag, allowing userfaultfd to be restricted, so that in general
> it won't be useable by arbitrary user programs, but in environments that
> require userfaultfd it can be turned back on.
The default in the patch leaves userfaultfd enabled to all users, so
it may be clearer to reverse the last sentence to "in hardened
environments it allows to restrict userfaultfd to privileged processes.".
We can also make example that 'While this is not a kernel issue, in
practice unless you also "chmod u-s /usr/bin/fusermount" there's no
tangible benefit in removing privileges for userfaultfd, other than
probabilistic ones by decreasig the attack surface of the kernel, but
that would be better be achieved through SECCOMP and not globally.'.
^ permalink raw reply
* Re: [net-next PATCH v3 4/8] net: Change return type of sk_busy_loop from bool to void
From: Christoph Paasch @ 2019-03-20 18:35 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, linux-kernel, sridhar.samudrala, Eric Dumazet,
David Miller, linux-api
In-Reply-To: <20170324170812.15226.97497.stgit@localhost.localdomain>
Hello,
On Fri, Mar 24, 2017 at 3:23 PM Alexander Duyck
<alexander.duyck@gmail.com> wrote:
>
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> >From what I can tell there is only a couple spots where we are actually
> checking the return value of sk_busy_loop. As there are only a few
> consumers of that data, and the data being checked for can be replaced
> with a check for !skb_queue_empty() we might as well just pull the code
> out of sk_busy_loop and place it in the spots that actually need it.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
> ---
> include/net/busy_poll.h | 5 ++---
> net/core/datagram.c | 8 ++++++--
> net/core/dev.c | 25 +++++++++++--------------
> net/sctp/socket.c | 9 ++++++---
> 4 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> index b82d6ba70a14..c55760f4820f 100644
> --- a/include/net/busy_poll.h
> +++ b/include/net/busy_poll.h
> @@ -74,7 +74,7 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> return time_after(now, end_time);
> }
>
> -bool sk_busy_loop(struct sock *sk, int nonblock);
> +void sk_busy_loop(struct sock *sk, int nonblock);
>
> #else /* CONFIG_NET_RX_BUSY_POLL */
> static inline unsigned long net_busy_loop_on(void)
> @@ -97,9 +97,8 @@ static inline bool busy_loop_timeout(unsigned long end_time)
> return true;
> }
>
> -static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> +static inline void sk_busy_loop(struct sock *sk, int nonblock)
> {
> - return false;
> }
>
> #endif /* CONFIG_NET_RX_BUSY_POLL */
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index ea633342ab0d..4608aa245410 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
> }
>
> spin_unlock_irqrestore(&queue->lock, cpu_flags);
> - } while (sk_can_busy_loop(sk) &&
> - sk_busy_loop(sk, flags & MSG_DONTWAIT));
> +
> + if (!sk_can_busy_loop(sk))
> + break;
> +
> + sk_busy_loop(sk, flags & MSG_DONTWAIT);
> + } while (!skb_queue_empty(&sk->sk_receive_queue));
since this change I am hitting stalls where it's looping in this
while-loop with syzkaller.
It worked prior to this change because sk->sk_napi_id was not set thus
sk_busy_loop would make us get out of the loop.
Now, it keeps on looping because there is an skb in the queue with
skb->len == 0 and we are peeking with an offset, thus
__skb_try_recv_from_queue will return NULL and thus we have no way of
getting out of the loop.
I'm not sure what would be the best way to fix it. I don't see why we
end up with an skb in the list with skb->len == 0. So, shooting a
quick e-mail, maybe somebody has an idea :-)
I have the syzkaller-reproducer if needed.
Thanks,
Christoph
>
> error = -EAGAIN;
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ab337bf5bbf4..af70eb6ba682 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5060,21 +5060,19 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock)
> do_softirq();
> }
>
> -bool sk_busy_loop(struct sock *sk, int nonblock)
> +void sk_busy_loop(struct sock *sk, int nonblock)
> {
> unsigned long end_time = !nonblock ? sk_busy_loop_end_time(sk) : 0;
> int (*napi_poll)(struct napi_struct *napi, int budget);
> void *have_poll_lock = NULL;
> struct napi_struct *napi;
> unsigned int napi_id;
> - int rc;
>
> restart:
> napi_id = READ_ONCE(sk->sk_napi_id);
> if (napi_id < MIN_NAPI_ID)
> - return 0;
> + return;
>
> - rc = false;
> napi_poll = NULL;
>
> rcu_read_lock();
> @@ -5085,7 +5083,8 @@ bool sk_busy_loop(struct sock *sk, int nonblock)
>
> preempt_disable();
> for (;;) {
> - rc = 0;
> + int work = 0;
> +
> local_bh_disable();
> if (!napi_poll) {
> unsigned long val = READ_ONCE(napi->state);
> @@ -5103,12 +5102,12 @@ bool sk_busy_loop(struct sock *sk, int nonblock)
> have_poll_lock = netpoll_poll_lock(napi);
> napi_poll = napi->poll;
> }
> - rc = napi_poll(napi, BUSY_POLL_BUDGET);
> - trace_napi_poll(napi, rc, BUSY_POLL_BUDGET);
> + work = napi_poll(napi, BUSY_POLL_BUDGET);
> + trace_napi_poll(napi, work, BUSY_POLL_BUDGET);
> count:
> - if (rc > 0)
> + if (work > 0)
> __NET_ADD_STATS(sock_net(sk),
> - LINUX_MIB_BUSYPOLLRXPACKETS, rc);
> + LINUX_MIB_BUSYPOLLRXPACKETS, work);
> local_bh_enable();
>
> if (nonblock || !skb_queue_empty(&sk->sk_receive_queue) ||
> @@ -5121,9 +5120,9 @@ bool sk_busy_loop(struct sock *sk, int nonblock)
> preempt_enable();
> rcu_read_unlock();
> cond_resched();
> - rc = !skb_queue_empty(&sk->sk_receive_queue);
> - if (rc || busy_loop_timeout(end_time))
> - return rc;
> + if (!skb_queue_empty(&sk->sk_receive_queue) ||
> + busy_loop_timeout(end_time))
> + return;
> goto restart;
> }
> cpu_relax();
> @@ -5131,10 +5130,8 @@ bool sk_busy_loop(struct sock *sk, int nonblock)
> if (napi_poll)
> busy_poll_stop(napi, have_poll_lock);
> preempt_enable();
> - rc = !skb_queue_empty(&sk->sk_receive_queue);
> out:
> rcu_read_unlock();
> - return rc;
> }
> EXPORT_SYMBOL(sk_busy_loop);
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 72cc3ecf6516..ccc08fc39722 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -7518,9 +7518,12 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags,
> if (sk->sk_shutdown & RCV_SHUTDOWN)
> break;
>
> - if (sk_can_busy_loop(sk) &&
> - sk_busy_loop(sk, noblock))
> - continue;
> + if (sk_can_busy_loop(sk)) {
> + sk_busy_loop(sk, noblock);
> +
> + if (!skb_queue_empty(&sk->sk_receive_queue))
> + continue;
> + }
>
> /* User doesn't want to wait. */
> error = -EAGAIN;
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox