Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 04/10] net: dsa: sja1105: Actually implement the P/Q/R/S FDB bits
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
  To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>

In commit 1da73821343c ("net: dsa: sja1105: Add FDB operations for
P/Q/R/S series"), these bits were set in the static config, but
apparently they did not do anything.  The reason is that the packing
accessors for them were part of a patch I forgot to send.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_static_config.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_static_config.c b/drivers/net/dsa/sja1105/sja1105_static_config.c
index 242f001c59fe..a1e9656c881c 100644
--- a/drivers/net/dsa/sja1105/sja1105_static_config.c
+++ b/drivers/net/dsa/sja1105/sja1105_static_config.c
@@ -232,9 +232,14 @@ sja1105pqrs_l2_lookup_params_entry_packing(void *buf, void *entry_ptr,
 	struct sja1105_l2_lookup_params_entry *entry = entry_ptr;
 
 	sja1105_packing(buf, &entry->maxage,         57,  43, size, op);
+	sja1105_packing(buf, &entry->start_dynspc,   42,  33, size, op);
+	sja1105_packing(buf, &entry->drpnolearn,     32,  28, size, op);
 	sja1105_packing(buf, &entry->shared_learn,   27,  27, size, op);
 	sja1105_packing(buf, &entry->no_enf_hostprt, 26,  26, size, op);
 	sja1105_packing(buf, &entry->no_mgmt_learn,  25,  25, size, op);
+	sja1105_packing(buf, &entry->use_static,     24,  24, size, op);
+	sja1105_packing(buf, &entry->owr_dyn,        23,  23, size, op);
+	sja1105_packing(buf, &entry->learn_once,     22,  22, size, op);
 	return size;
 }
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 03/10] net: dsa: sja1105: Make vid 1 the default pvid
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
  To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>

In SJA1105 there is no concept of 'default values' per se, everything
needs to be driver-supplied through the static configuration tables.

The issue is that the hardware manual says that 'at least the default
untagging VLAN' is mandatory to be provided through the static config.
But VLAN 0 isn't a very good initial pvid - its use is reserved for
priority-tagged frames, and the layers of the stack that care about
those already make sure that this VLAN is installed, as can be seen in
the message below:

  8021q: adding VLAN 0 to HW filter on device swp2

So change the pvid provided through the static configuration to 1, which
matches the bridge core's defaults.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 9395e8f5f790..bc9f37cd3876 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -80,7 +80,7 @@ static int sja1105_init_mac_settings(struct sja1105_private *priv)
 		.maxage = 0xFF,
 		/* Internal VLAN (pvid) to apply to untagged ingress */
 		.vlanprio = 0,
-		.vlanid = 0,
+		.vlanid = 1,
 		.ing_mirr = false,
 		.egr_mirr = false,
 		/* Don't drop traffic with other EtherType than ETH_P_IP */
@@ -264,20 +264,15 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 		.vmemb_port = 0,
 		.vlan_bc = 0,
 		.tag_port = 0,
-		.vlanid = 0,
+		.vlanid = 1,
 	};
 	int i;
 
 	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
 
-	/* The static VLAN table will only contain the initial pvid of 0.
+	/* The static VLAN table will only contain the initial pvid of 1.
 	 * All other VLANs are to be configured through dynamic entries,
 	 * and kept in the static configuration table as backing memory.
-	 * The pvid of 0 is sufficient to pass traffic while the ports are
-	 * standalone and when vlan_filtering is disabled. When filtering
-	 * gets enabled, the switchdev core sets up the VLAN ID 1 and sets
-	 * it as the new pvid. Actually 'pvid 1' still comes up in 'bridge
-	 * vlan' even when vlan_filtering is off, but it has no effect.
 	 */
 	if (table->entry_count) {
 		kfree(table->entries);
@@ -291,7 +286,7 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 
 	table->entry_count = 1;
 
-	/* VLAN ID 0: all DT-defined ports are members; no restrictions on
+	/* VLAN 1: all DT-defined ports are members; no restrictions on
 	 * forwarding; always transmit priority-tagged frames as untagged.
 	 */
 	for (i = 0; i < SJA1105_NUM_PORTS; i++) {
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 02/10] net: dsa: sja1105: Cancel PTP delayed work on unregister
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
  To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>

Currently when the driver unloads and PTP is enabled, the delayed work
that prevents the timecounter from expiring becomes a ticking time bomb.
The kernel will schedule the work thread within 60 seconds of driver
removal, but the work handler is no longer there, leading to this
strange and inconclusive stack trace:

[   64.473112] Unable to handle kernel paging request at virtual address 79746970
[   64.480340] pgd = 008c4af9
[   64.483042] [79746970] *pgd=00000000
[   64.486620] Internal error: Oops: 80000005 [#1] SMP ARM
[   64.491820] Modules linked in:
[   64.494871] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0-rc5-01634-ge3a2773ba9e5 #1246
[   64.503007] Hardware name: Freescale LS1021A
[   64.507259] PC is at 0x79746970
[   64.510393] LR is at call_timer_fn+0x3c/0x18c
[   64.514729] pc : [<79746970>]    lr : [<c03bd734>]    psr: 60010113
[   64.520965] sp : c1901de0  ip : 00000000  fp : c1903080
[   64.526163] r10: c1901e38  r9 : ffffe000  r8 : c19064ac
[   64.531363] r7 : 79746972  r6 : e98dd260  r5 : 00000100  r4 : c1a9e4a0
[   64.537859] r3 : c1900000  r2 : ffffa400  r1 : 79746972  r0 : e98dd260
[   64.544359] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   64.551460] Control: 10c5387d  Table: a8a2806a  DAC: 00000051
[   64.557176] Process swapper/0 (pid: 0, stack limit = 0x1ddb27f0)
[   64.563147] Stack: (0xc1901de0 to 0xc1902000)
[   64.567481] 1de0: eb6a4918 3d60d7c3 c1a9e554 e98dd260 eb6a34c0 c1a9e4a0 ffffa400 c19064ac
[   64.575616] 1e00: ffffe000 c03bd95c c1901e34 c1901e34 eb6a34c0 c1901e30 c1903d00 c186f4c0
[   64.583751] 1e20: c1906488 29e34000 c1903080 c03bdca4 00000000 eaa6f218 00000000 eb6a45c0
[   64.591886] 1e40: eb6a45c0 20010193 00000003 c03c0a68 20010193 3f7231be c1903084 00000002
[   64.600022] 1e60: 00000082 00000001 ffffe000 c1a9e0a4 00000100 c0302298 02b64722 0000000f
[   64.608157] 1e80: c186b3c8 c1877540 c19064ac 0000000a c186b350 ffffa401 c1903d00 c1107348
[   64.616292] 1ea0: 00200102 c0d87a14 ea823c00 ffffe000 00000012 00000000 00000000 ea810800
[   64.624427] 1ec0: f0803000 c1876ba8 00000000 c034c784 c18774b8 c039fb50 c1906c90 c1978aac
[   64.632562] 1ee0: f080200c f0802000 c1901f10 c0709ca8 c03091a0 60010013 ffffffff c1901f44
[   64.640697] 1f00: 00000000 c1900000 c1876ba8 c0301a8c 00000000 000070a0 eb6ac1a0 c031da60
[   64.648832] 1f20: ffffe000 c19064ac c19064f0 00000001 00000000 c1906488 c1876ba8 00000000
[   64.656967] 1f40: ffffffff c1901f60 c030919c c03091a0 60010013 ffffffff 00000051 00000000
[   64.665102] 1f60: ffffe000 c0376aa4 c1a9da37 ffffffff 00000037 3f7231be c1ab20c0 000000cc
[   64.673238] 1f80: c1906488 c1906480 ffffffff 00000037 c1ab20c0 c1ab20c0 00000001 c0376e1c
[   64.681373] 1fa0: c1ab2118 c1700ea8 ffffffff ffffffff 00000000 c1700754 c17dfa40 ebfffd80
[   64.689509] 1fc0: 00000000 c17dfa40 3f7733be 00000000 00000000 c1700330 00000051 10c0387d
[   64.697644] 1fe0: 00000000 8f000000 410fc075 10c5387d 00000000 00000000 00000000 00000000
[   64.705788] [<c03bd734>] (call_timer_fn) from [<c03bd95c>] (expire_timers+0xd8/0x144)
[   64.713579] [<c03bd95c>] (expire_timers) from [<c03bdca4>] (run_timer_softirq+0xe4/0x1dc)
[   64.721716] [<c03bdca4>] (run_timer_softirq) from [<c0302298>] (__do_softirq+0x130/0x3c8)
[   64.729854] [<c0302298>] (__do_softirq) from [<c034c784>] (irq_exit+0xbc/0xd8)
[   64.737040] [<c034c784>] (irq_exit) from [<c039fb50>] (__handle_domain_irq+0x60/0xb4)
[   64.744833] [<c039fb50>] (__handle_domain_irq) from [<c0709ca8>] (gic_handle_irq+0x58/0x9c)
[   64.753143] [<c0709ca8>] (gic_handle_irq) from [<c0301a8c>] (__irq_svc+0x6c/0x90)
[   64.760583] Exception stack(0xc1901f10 to 0xc1901f58)
[   64.765605] 1f00:                                     00000000 000070a0 eb6ac1a0 c031da60
[   64.773740] 1f20: ffffe000 c19064ac c19064f0 00000001 00000000 c1906488 c1876ba8 00000000
[   64.781873] 1f40: ffffffff c1901f60 c030919c c03091a0 60010013 ffffffff
[   64.788456] [<c0301a8c>] (__irq_svc) from [<c03091a0>] (arch_cpu_idle+0x38/0x3c)
[   64.795816] [<c03091a0>] (arch_cpu_idle) from [<c0376aa4>] (do_idle+0x1bc/0x298)
[   64.803175] [<c0376aa4>] (do_idle) from [<c0376e1c>] (cpu_startup_entry+0x18/0x1c)
[   64.810707] [<c0376e1c>] (cpu_startup_entry) from [<c1700ea8>] (start_kernel+0x480/0x4ac)
[   64.818839] Code: bad PC value
[   64.821890] ---[ end trace e226ed97b1c584cd ]---
[   64.826482] Kernel panic - not syncing: Fatal exception in interrupt
[   64.832807] CPU1: stopping
[   64.835501] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D           5.2.0-rc5-01634-ge3a2773ba9e5 #1246
[   64.845013] Hardware name: Freescale LS1021A
[   64.849266] [<c0312394>] (unwind_backtrace) from [<c030cc74>] (show_stack+0x10/0x14)
[   64.856972] [<c030cc74>] (show_stack) from [<c0ff4138>] (dump_stack+0xb4/0xc8)
[   64.864159] [<c0ff4138>] (dump_stack) from [<c0310854>] (handle_IPI+0x3bc/0x3dc)
[   64.871519] [<c0310854>] (handle_IPI) from [<c0709ce8>] (gic_handle_irq+0x98/0x9c)
[   64.879050] [<c0709ce8>] (gic_handle_irq) from [<c0301a8c>] (__irq_svc+0x6c/0x90)
[   64.886489] Exception stack(0xea8cbf60 to 0xea8cbfa8)
[   64.891514] bf60: 00000000 0000307c eb6c11a0 c031da60 ffffe000 c19064ac c19064f0 00000002
[   64.899649] bf80: 00000000 c1906488 c1876ba8 00000000 00000000 ea8cbfb0 c030919c c03091a0
[   64.907780] bfa0: 600d0013 ffffffff
[   64.911250] [<c0301a8c>] (__irq_svc) from [<c03091a0>] (arch_cpu_idle+0x38/0x3c)
[   64.918609] [<c03091a0>] (arch_cpu_idle) from [<c0376aa4>] (do_idle+0x1bc/0x298)
[   64.925967] [<c0376aa4>] (do_idle) from [<c0376e1c>] (cpu_startup_entry+0x18/0x1c)
[   64.933496] [<c0376e1c>] (cpu_startup_entry) from [<803025cc>] (0x803025cc)
[   64.940422] Rebooting in 3 seconds..

In this case, what happened is that the DSA driver failed to probe at
boot time due to a PHY issue during phylink_connect_phy:

[    2.245607] fsl-gianfar soc:ethernet@2d90000 eth2: error -19 setting up slave phy
[    2.258051] sja1105 spi0.1: failed to create slave for port 0.0

Fixes: bb77f36ac21d ("net: dsa: sja1105: Add support for the PTP clock")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_ptp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index c7ce1edd8471..d19cfdf681af 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -387,6 +387,7 @@ void sja1105_ptp_clock_unregister(struct sja1105_private *priv)
 	if (IS_ERR_OR_NULL(priv->clock))
 		return;
 
+	cancel_delayed_work_sync(&priv->refresh_work);
 	ptp_clock_unregister(priv->clock);
 	priv->clock = NULL;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 00/10] FDB, VLAN and PTP fixes for SJA1105 DSA
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
  To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean

This patchset is an assortment of fixes for the net-next version of the
sja1105 DSA driver:
- Avoid a kernel panic when the driver fails to probe or unregisters
- Finish Arnd Bermann's idea of compiling PTP support as part of the
  main DSA driver and not separately
- Better handling of initial port-based VLAN as well as VLANs for
  dsa_8021q FDB entries
- Fix address learning for the SJA1105 P/Q/R/S family
- Make static FDB entries persistent across switch resets
- Fix reporting of statically-added FDB entries in 'bridge fdb show'

Vladimir Oltean (10):
  net: dsa: sja1105: Build PTP support in main DSA driver
  net: dsa: sja1105: Cancel PTP delayed work on unregister
  net: dsa: sja1105: Make vid 1 the default pvid
  net: dsa: sja1105: Actually implement the P/Q/R/S FDB bits
  net: dsa: sja1105: Make P/Q/R/S learn MAC addresses
  net: dsa: sja1105: Back up static FDB entries in kernel memory
  net: dsa: sja1105: Add a high-level overview of the dynamic config
    interface
  net: dsa: sja1105: Populate is_static for FDB entries on P/Q/R/S
  net: dsa: sja1105: Use correct dsa_8021q VIDs for FDB commands
  net: dsa: sja1105: Implement is_static for FDB entries on E/T

 drivers/net/dsa/sja1105/Makefile              |   2 +-
 .../net/dsa/sja1105/sja1105_dynamic_config.c  | 154 +++++++++++-
 drivers/net/dsa/sja1105/sja1105_main.c        | 227 ++++++++++++++----
 drivers/net/dsa/sja1105/sja1105_ptp.c         |  13 +-
 drivers/net/dsa/sja1105/sja1105_spi.c         |   2 -
 .../net/dsa/sja1105/sja1105_static_config.c   |  12 +-
 .../net/dsa/sja1105/sja1105_static_config.h   |   3 +-
 7 files changed, 343 insertions(+), 70 deletions(-)

-- 
2.17.1


^ permalink raw reply

* Re: [PATCH net-next 10/18] ionic: Add management of rx filters
From: Jakub Kicinski @ 2019-06-25 23:37 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-11-snelson@pensando.io>

On Thu, 20 Jun 2019 13:24:16 -0700, Shannon Nelson wrote:
> +int ionic_rx_filter_save(struct lif *lif, u32 flow_id, u16 rxq_index,
> +			 u32 hash, struct ionic_admin_ctx *ctx)
> +{
> +	struct device *dev = lif->ionic->dev;
> +	struct hlist_head *head;
> +	struct rx_filter *f;
> +	unsigned int key;
> +
> +	f = devm_kzalloc(dev, sizeof(*f), GFP_KERNEL);
> +	if (!f)
> +		return -ENOMEM;
> +
> +	f->flow_id = flow_id;
> +	f->filter_id = le32_to_cpu(ctx->comp.rx_filter_add.filter_id);
> +	f->rxq_index = rxq_index;
> +	memcpy(&f->cmd, &ctx->cmd, sizeof(f->cmd));
> +
> +	INIT_HLIST_NODE(&f->by_hash);
> +	INIT_HLIST_NODE(&f->by_id);
> +
> +	switch (le16_to_cpu(f->cmd.match)) {
> +	case RX_FILTER_MATCH_VLAN:
> +		key = le16_to_cpu(f->cmd.vlan.vlan) & RX_FILTER_HLISTS_MASK;
> +		break;
> +	case RX_FILTER_MATCH_MAC:
> +		key = *(u32 *)f->cmd.mac.addr & RX_FILTER_HLISTS_MASK;
> +		break;
> +	case RX_FILTER_MATCH_MAC_VLAN:
> +		key = le16_to_cpu(f->cmd.mac_vlan.vlan) & RX_FILTER_HLISTS_MASK;
> +		break;
> +	default:

I know you use devm_kzalloc() but can't this potentially keep arbitrary
amounts of memory held until the device is removed (and it's the entire
device not just a LIF)?

> +		return -ENOTSUPP;

EOPNOTSUPP, please do not use ENOTSUPP in the drivers.  It's a high
error code, unknown to libc.  We should use EOPNOTSUPP or EINVAL.

> +	}

^ permalink raw reply

* Re: [PATCH net-next 09/18] ionic: Add the basic NDO callbacks for netdev support
From: Jakub Kicinski @ 2019-06-25 23:27 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-10-snelson@pensando.io>

On Thu, 20 Jun 2019 13:24:15 -0700, Shannon Nelson wrote:
> +static int ionic_set_features(struct net_device *netdev,
> +			      netdev_features_t features)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +	int err;
> +
> +	netdev_dbg(netdev, "%s: lif->features=0x%08llx new_features=0x%08llx\n",
> +		   __func__, (u64)lif->netdev->features, (u64)features);
> +
> +	err = ionic_set_nic_features(lif, features);

Presumably something gets added here in later patch?

> +	return err;
> +}
> +
> +static int ionic_set_mac_address(struct net_device *netdev, void *sa)
> +{
> +	netdev_info(netdev, "%s: stubbed\n", __func__);
> +	return 0;
> +}
> +
> +static int ionic_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +	struct ionic_admin_ctx ctx = {
> +		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
> +		.cmd.lif_setattr = {
> +			.opcode = CMD_OPCODE_LIF_SETATTR,
> +			.index = cpu_to_le16(lif->index),
> +			.attr = IONIC_LIF_ATTR_MTU,
> +			.mtu = cpu_to_le32(new_mtu),
> +		},
> +	};
> +	int err;
> +
> +	if (new_mtu < IONIC_MIN_MTU || new_mtu > IONIC_MAX_MTU) {
> +		netdev_err(netdev, "Invalid MTU %d\n", new_mtu);
> +		return -EINVAL;
> +	}

We do the min/max checks in the core now (netdev->min_mtu,
netdev->max_mtu).  You'll have to keep this if out of tree,
unfortunately.

> +	err = ionic_adminq_post_wait(lif, &ctx);
> +	if (err)
> +		return err;
> +
> +	netdev->mtu = new_mtu;
> +	err = ionic_reset_queues(lif);
> +
> +	return err;
> +}

^ permalink raw reply

* [PATCH bpf-next 2/2] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-06-25 23:26 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190625232601.3227055-1-andriin@fb.com>

Add test verifying perf buffer API functionality.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/perf_buffer.c    | 86 +++++++++++++++++++
 .../selftests/bpf/progs/test_perf_buffer.c    | 31 +++++++
 2 files changed, 117 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c

diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
new file mode 100644
index 000000000000..3ba3e26141ac
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <sched.h>
+#include <sys/socket.h>
+#include <test_progs.h>
+
+static void on_sample(void *ctx, void *data, __u32 size)
+{
+	cpu_set_t *cpu_seen = ctx;
+	int cpu = *(int *)data;
+
+	CPU_SET(cpu, cpu_seen);
+}
+
+void test_perf_buffer(void)
+{
+	int err, prog_fd, prog_pfd, nr_cpus, i, duration = 0;
+	const char *prog_name = "kprobe/sys_nanosleep";
+	const char *file = "./test_perf_buffer.o";
+	struct bpf_map *perf_buf_map;
+	cpu_set_t cpu_set, cpu_seen;
+	struct bpf_program *prog;
+	struct bpf_object *obj;
+	struct perf_buffer *pb;
+
+	nr_cpus = libbpf_num_possible_cpus();
+	if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
+		return;
+
+	/* load program */
+	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+		return;
+
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
+		goto out_close;
+
+	/* load map */
+	perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
+	if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
+		goto out_close;
+
+	/* attach kprobe */
+	prog_pfd = bpf_program__attach_kprobe(prog, false /* retprobe */,
+					      "sys_nanosleep");
+	if (CHECK(prog_pfd < 0, "attach_kprobe", "err %d\n", prog_pfd))
+		goto out_close;
+
+	/* set up perf buffer */
+	pb = perf_buffer__new(perf_buf_map, 1, on_sample, NULL, &cpu_seen);
+	if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
+		goto out_detach;
+
+	/* trigger kprobe on every CPU */
+	CPU_ZERO(&cpu_seen);
+	for (i = 0; i < nr_cpus; i++) {
+		CPU_ZERO(&cpu_set);
+		CPU_SET(i, &cpu_set);
+
+		err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
+					     &cpu_set);
+		if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
+				 i, err))
+			goto out_detach;
+
+		usleep(1);
+	}
+
+	/* read perf buffer */
+	err = perf_buffer__poll(pb, 100);
+	if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
+		goto out_free_pb;
+
+	if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
+		  "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
+		goto out_free_pb;
+
+out_free_pb:
+	perf_buffer__free(pb);
+out_detach:
+	libbpf_perf_event_disable_and_close(prog_pfd);
+out_close:
+	bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
new file mode 100644
index 000000000000..ba961f608fd5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+	int type;
+	int key_size;
+	int value_size;
+	int max_entries;
+} perf_buf_map SEC(".maps") = {
+	.type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.max_entries = 56,
+};
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+	int cpu = bpf_get_smp_processor_id();
+
+	bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
+			      &cpu, sizeof(cpu));
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 1/2] libbpf: add perf buffer reading API
From: Andrii Nakryiko @ 2019-06-25 23:26 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190625232601.3227055-1-andriin@fb.com>

BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
to user space for additional processing. libbpf already has very low-level API
to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
use and requires a lot of code to set everything up. This patch adds
perf_buffer abstraction on top of it, abstracting setting up and polling
per-CPU logic into simple and convenient API, similar to what BCC provides.

perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
map entries. It accepts two user-provided callbacks: one for handling raw
samples and one for get notifications of lost samples due to buffer overflow.

perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
utilizing epoll instance.

perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.

All APIs are not thread-safe. User should ensure proper locking/coordination if
used in multi-threaded set up.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c   | 282 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  12 ++
 tools/lib/bpf/libbpf.map |   5 +-
 3 files changed, 298 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9a4199b51300..c74cc535902a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,7 +32,9 @@
 #include <linux/limits.h>
 #include <linux/perf_event.h>
 #include <linux/ring_buffer.h>
+#include <sys/epoll.h>
 #include <sys/ioctl.h>
+#include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
@@ -4322,6 +4324,286 @@ bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 	return ret;
 }
 
+struct perf_cpu_buf {
+	int fd;
+	void *base; /* mmap()'ed memory */
+	void *buf; /* for reconstructing segmented data */
+	size_t buf_size;
+};
+
+struct perf_buffer {
+	perf_buffer_sample_fn sample_cb;
+	perf_buffer_lost_fn lost_cb;
+	void *ctx; /* passed into callbacks */
+
+	size_t page_size;
+	size_t mmap_size;
+	struct perf_cpu_buf **cpu_bufs;
+	struct epoll_event *events;
+	int cpu_cnt;
+	int epfd; /* perf event FD */
+	int mapfd; /* BPF_MAP_TYPE_PERF_EVENT_ARRAY BPF map FD */
+};
+
+static void perf_buffer__free_cpu_buf(struct perf_buffer *pb,
+				      struct perf_cpu_buf *cpu_buf, int cpu)
+{
+	if (!cpu_buf)
+		return;
+	if (cpu_buf->base &&
+	    munmap(cpu_buf->base, pb->mmap_size + pb->page_size))
+		pr_warning("failed to munmap cpu_buf #%d\n", cpu);
+	if (cpu_buf->fd >= 0) {
+		ioctl(cpu_buf->fd, PERF_EVENT_IOC_DISABLE, 0);
+		close(cpu_buf->fd);
+	}
+	free(cpu_buf->buf);
+	free(cpu_buf);
+}
+
+void perf_buffer__free(struct perf_buffer *pb)
+{
+	int i;
+
+	if (!pb)
+		return;
+	if (pb->cpu_bufs) {
+		for (i = 0; i < pb->cpu_cnt && pb->cpu_bufs[i]; i++) {
+			struct perf_cpu_buf *cpu_buf = pb->cpu_bufs[i];
+
+			bpf_map_delete_elem(pb->mapfd, &i);
+			perf_buffer__free_cpu_buf(pb, cpu_buf, i);
+		}
+		free(pb->cpu_bufs);
+	}
+	if (pb->epfd >= 0)
+		close(pb->epfd);
+	free(pb->events);
+	free(pb);
+}
+
+static struct perf_cpu_buf *perf_buffer__open_cpu_buf(struct perf_buffer *pb,
+						      int cpu)
+{
+	struct perf_event_attr attr = {};
+	struct perf_cpu_buf *cpu_buf;
+	char msg[STRERR_BUFSIZE];
+	int err;
+
+	cpu_buf = calloc(1, sizeof(*cpu_buf));
+	if (!cpu_buf)
+		return ERR_PTR(-ENOMEM);
+
+	attr.config = PERF_COUNT_SW_BPF_OUTPUT;
+	attr.type = PERF_TYPE_SOFTWARE;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+	cpu_buf->fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, cpu,
+			      -1, PERF_FLAG_FD_CLOEXEC);
+	if (cpu_buf->fd < 0) {
+		err = -errno;
+		pr_warning("failed to open perf buffer event on cpu #%d: %s\n",
+			   cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	cpu_buf->base = mmap(NULL, pb->mmap_size + pb->page_size,
+			     PROT_READ | PROT_WRITE, MAP_SHARED,
+			     cpu_buf->fd, 0);
+	if (cpu_buf->base == MAP_FAILED) {
+		cpu_buf->base = NULL;
+		err = -errno;
+		pr_warning("failed to mmap perf buffer on cpu #%d: %s\n",
+			   cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	if (ioctl(cpu_buf->fd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+		err = -errno;
+		pr_warning("failed to enable perf buffer event on cpu #%d: %s\n",
+			   cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	return cpu_buf;
+
+error:
+	perf_buffer__free_cpu_buf(pb, cpu_buf, cpu);
+	return (struct perf_cpu_buf *)ERR_PTR(err);
+}
+
+struct perf_buffer *perf_buffer__new(struct bpf_map *map, size_t page_cnt,
+				     perf_buffer_sample_fn sample_cb,
+				     perf_buffer_lost_fn lost_cb, void *ctx)
+{
+	char msg[STRERR_BUFSIZE];
+	struct perf_buffer *pb;
+	int err, cpu;
+
+	if (bpf_map__def(map)->type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
+		pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
+			   bpf_map__name(map));
+		return ERR_PTR(-EINVAL);
+	}
+	if (bpf_map__fd(map) < 0) {
+		pr_warning("map '%s' doesn't have associated FD\n",
+			   bpf_map__name(map));
+		return ERR_PTR(-EINVAL);
+	}
+	if (page_cnt & (page_cnt - 1)) {
+		pr_warning("page count should be power of two, but is %zu\n",
+			   page_cnt);
+		return ERR_PTR(-EINVAL);
+	}
+
+	pb = calloc(1, sizeof(*pb));
+	if (!pb)
+		return ERR_PTR(-ENOMEM);
+
+	pb->sample_cb = sample_cb;
+	pb->lost_cb = lost_cb;
+	pb->ctx = ctx;
+	pb->page_size = getpagesize();
+	pb->mmap_size = pb->page_size * page_cnt;
+	pb->mapfd = bpf_map__fd(map);
+
+	pb->epfd = epoll_create1(EPOLL_CLOEXEC);
+	if (pb->epfd < 0) {
+		err = -errno;
+		pr_warning("failed to create epoll instance: %s\n",
+			   libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	pb->cpu_cnt = libbpf_num_possible_cpus();
+	if (pb->cpu_cnt < 0) {
+		err = pb->cpu_cnt;
+		goto error;
+	}
+	pb->events = calloc(pb->cpu_cnt, sizeof(*pb->events));
+	if (!pb->events) {
+		err = -ENOMEM;
+		pr_warning("failed to allocate events: out of memory\n");
+		goto error;
+	}
+	pb->cpu_bufs = calloc(pb->cpu_cnt, sizeof(*pb->cpu_bufs));
+	if (!pb->cpu_bufs) {
+		err = -ENOMEM;
+		pr_warning("failed to allocate buffers: out of memory\n");
+		goto error;
+	}
+
+	for (cpu = 0; cpu < pb->cpu_cnt; cpu++) {
+		struct perf_cpu_buf *cpu_buf;
+
+		cpu_buf = perf_buffer__open_cpu_buf(pb, cpu);
+		if (IS_ERR(cpu_buf)) {
+			err = PTR_ERR(cpu_buf);
+			goto error;
+		}
+
+		pb->cpu_bufs[cpu] = cpu_buf;
+
+		err = bpf_map_update_elem(pb->mapfd, &cpu, &cpu_buf->fd, 0);
+		if (err) {
+			pr_warning("failed to set cpu #%d perf FD %d: %s\n",
+				   cpu, cpu_buf->fd,
+				   libbpf_strerror_r(err, msg, sizeof(msg)));
+			goto error;
+		}
+
+		pb->events[cpu].events = EPOLLIN;
+		pb->events[cpu].data.ptr = cpu_buf;
+		if (epoll_ctl(pb->epfd, EPOLL_CTL_ADD, cpu_buf->fd,
+			      &pb->events[cpu]) < 0) {
+			err = -errno;
+			pr_warning("failed to epoll_ctl cpu #%d perf FD %d: %s\n",
+				   cpu, cpu_buf->fd,
+				   libbpf_strerror_r(err, msg, sizeof(msg)));
+			goto error;
+		}
+	}
+
+	return pb;
+
+error:
+	if (pb)
+		perf_buffer__free(pb);
+	return ERR_PTR(err);
+}
+
+struct perf_sample_raw {
+	struct perf_event_header header;
+	uint32_t size;
+	char data[0];
+};
+
+struct perf_sample_lost {
+	struct perf_event_header header;
+	uint64_t id;
+	uint64_t lost;
+	uint64_t sample_id;
+};
+
+static enum bpf_perf_event_ret
+perf_buffer__process_record(struct perf_event_header *e, void *ctx)
+{
+	struct perf_buffer *pb = ctx;
+	void *data = e;
+
+	switch (e->type) {
+	case PERF_RECORD_SAMPLE: {
+		struct perf_sample_raw *s = data;
+
+		pb->sample_cb(pb->ctx, s->data, s->size);
+		break;
+	}
+	case PERF_RECORD_LOST: {
+		struct perf_sample_lost *s = data;
+
+		if (pb->lost_cb)
+			pb->lost_cb(pb->ctx, s->lost);
+		break;
+	}
+	default:
+		pr_warning("unknown perf sample type %d\n", e->type);
+		return LIBBPF_PERF_EVENT_ERROR;
+	}
+	return LIBBPF_PERF_EVENT_CONT;
+}
+
+static int perf_buffer__process_records(struct perf_buffer *pb,
+					struct perf_cpu_buf *cpu_buf)
+{
+	enum bpf_perf_event_ret ret;
+
+	ret = bpf_perf_event_read_simple(cpu_buf->base, pb->mmap_size,
+					 pb->page_size, &cpu_buf->buf,
+					 &cpu_buf->buf_size,
+					 perf_buffer__process_record, pb);
+	if (ret != LIBBPF_PERF_EVENT_CONT)
+		return ret;
+	return 0;
+}
+
+int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms)
+{
+	int cnt, err;
+
+	cnt = epoll_wait(pb->epfd, pb->events, pb->cpu_cnt, timeout_ms);
+	for (int i = 0; i < cnt; i++) {
+		struct perf_cpu_buf *cpu_buf = pb->events[i].data.ptr;
+
+		err = perf_buffer__process_records(pb, cpu_buf);
+		if (err) {
+			pr_warning("error while processing records: %d\n", err);
+			return err;
+		}
+	}
+	return cnt < 0 ? -errno : cnt;
+}
+
 struct bpf_prog_info_array_desc {
 	int	array_offset;	/* e.g. offset of jited_prog_insns */
 	int	count_offset;	/* e.g. offset of jited_prog_len */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bf7020a565c6..3bfde1a475ce 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -354,6 +354,18 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
 LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
 LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
 
+struct perf_buffer;
+typedef void (*perf_buffer_sample_fn)(void *ctx, void *data, __u32 size);
+typedef void (*perf_buffer_lost_fn)(void *ctx, __u64 cnt);
+
+LIBBPF_API struct perf_buffer *perf_buffer__new(struct bpf_map *map,
+						size_t page_cnt,
+						perf_buffer_sample_fn sample_cb,
+						perf_buffer_lost_fn lost_cb,
+						void *ctx);
+LIBBPF_API void perf_buffer__free(struct perf_buffer *pb);
+LIBBPF_API int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms);
+
 enum bpf_perf_event_ret {
 	LIBBPF_PERF_EVENT_DONE	= 0,
 	LIBBPF_PERF_EVENT_ERROR	= -1,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2382fbda4cbb..10f48103110a 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
 		btf_dump__dump_type;
 		btf_dump__free;
 		btf_dump__new;
-		btf__parse_elf;
 		bpf_object__load_xattr;
 		bpf_program__attach_kprobe;
 		bpf_program__attach_perf_event;
 		bpf_program__attach_raw_tracepoint;
 		bpf_program__attach_tracepoint;
 		bpf_program__attach_uprobe;
+		btf__parse_elf;
 		libbpf_num_possible_cpus;
 		libbpf_perf_event_disable_and_close;
+		perf_buffer__free;
+		perf_buffer__new;
+		perf_buffer__poll;
 } LIBBPF_0.0.3;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 0/2] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-06-25 23:25 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko

This patchset adds a high-level API for setting up and polling perf buffers
associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
described in corresponding commit.

Andrii Nakryiko (2):
  libbpf: add perf buffer reading API
  selftests/bpf: test perf buffer API

 tools/lib/bpf/libbpf.c                        | 282 ++++++++++++++++++
 tools/lib/bpf/libbpf.h                        |  12 +
 tools/lib/bpf/libbpf.map                      |   5 +-
 .../selftests/bpf/prog_tests/perf_buffer.c    |  86 ++++++
 .../selftests/bpf/progs/test_perf_buffer.c    |  31 ++
 5 files changed, 415 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c

-- 
2.17.1


^ permalink raw reply

* Re: [PATCH net-next 08/18] ionic: Add notifyq support
From: Jakub Kicinski @ 2019-06-25 23:21 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-9-snelson@pensando.io>

On Thu, 20 Jun 2019 13:24:14 -0700, Shannon Nelson wrote:
> +	case EVENT_OPCODE_HEARTBEAT:
> +		netdev_info(netdev, "Notifyq EVENT_OPCODE_HEARTBEAT eid=%lld\n",
> +			    eid);
> +		break;

I wonder how often this gets sent and whether the info log level is
really necessary for a correctly working heartbeat?

^ permalink raw reply

* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Vladimir Oltean @ 2019-06-25 23:13 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
	matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
	linux-mediatek, linux-mips
In-Reply-To: <CA+h21hq_w8-96ehKYxcziSq1TjOjoKduZ+pB3umBfjODaKWd+A@mail.gmail.com>

On Wed, 26 Jun 2019 at 02:10, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > > Hi Russell,
> > > > >
> > > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > > This should be removed - state->link is not for use in mac_config.
> > > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > > >
> > > > >
> > > > > What do you mean exactly that state->link is not for use, is that true in
> > > > > general?
> > > >
> > > > Yes.  mac_config() should not touch it; it is not always in a defined
> > > > state.  For example, if you set modes via ethtool (the
> > > > ethtool_ksettings_set API) then state->link will probably contain
> > > > zero irrespective of the true link state.
> > > >
> > >
> > > Experimentally, state->link is zero at the same time as state->speed
> > > is -1, so just ignoring !state->link made sense. This is not in-band
> > > AN. What is your suggestion? Should I proceed to try and configure the
> > > MAC for SPEED_UNKNOWN?
> >
> > What would you have done with a PHY when the link is down, what speed
> > would you have configured in the phylib adjust_link callback?  phylib
> > also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
> >
>
> With phylib, I'd make the driver ignore the speed and do nothing.
> With phylink, I'd make the core not call mac_config.
> But what happened is I saw phylink call mac_config anyway, said
> 'weird' and proceeded to ignore it as I would have for phylib.
> I'm just not understanding your position - it seems like you're
> implying there's a bug in phylink and the function call with
> MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken

I meant MLO_AN_PHY, sorry.

> place, which is what I wanted to confirm.
>
> > What we do in Marvell drivers is set to the lowest speed (10M) in such
> > cases, which is fine as the MAC supports 10M.
> >
> > It wouldn't be appropriate for phylink to force something on MAC
> > drivers, it's easier if the MAC just defaults SPEED_UNKNOWN to something
> > itself.
> >
> > >
> > > > It exists in this structure because it was convenient to just use one
> > > > structure to store all the link information in various parts of the
> > > > code, and when requesting the negotiated in-band MAC configuration.
> > > >
> > > > I've come to the conclusion that that decision was a mistake, based
> > > > on patches such as the above mistakenly thinking that everything in
> > > > the state structure is fair game.  I've since updated the docs to
> > > > explicitly spell it out, but I'm also looking at the feasibility of
> > > > changing the mac_config() interface entirely - splitting it into two
> > > > (mac_config_fixed() and mac_config_inband()) and passing only the
> > > > appropriate parameters to each.
> > > >
> > > > However, having looked at that, I think such a change will make some
> > > > MAC drivers quite a bit more complicated - having all the config
> > > > steps in one method appears to make the configuration of MAC drivers
> > > > easier (eg, mvneta, mvpp2.)
> > > >
> > > > > In drivers/net/dsa/sja1105/sja1105_main.c, if I remove the "if
> > > > > (!state->link)" guard, I see PHYLINK calls with a SPEED_UNKNOWN argument for
> > > > > ports that are BR_STATE_DISABLED. Is that normal?
> > > >
> > > > This looks like another driver which has been converted to phylink
> > > > without my review; I certainly wasn't aware of it.  It gets a few
> > > > things wrong, such as:
> > > >
> > > > 1) not checking state->interface in the validate callback - so it
> > > >    is basically saying that it can support any PHY interface mode
> > > >    that the kernel happens to support.
> > > >
> > >
> > > Partially true. It does check the DT bindings for supported MII modes
> > > in sja1105_init_mii_settings (for fundamental reasons... the switch
> > > expects an 'all-in-one' configuration buffer with the operating modes
> > > of all MACs - don't ask me to delay the uploading of this static
> > > config until all ports collected their interface_mode from phylink via
> > > the mac_config callback - it's a deadlock).
> >
> > Ok, so you need to reject interface modes that are not compatible
> > with the currently configured mode in the validate() callback, but
> > please keep PHY_INTERFACE_MODE_NA reporting back the capabilities.
> > (this is now documented.)
> >
>
> Ok.
>
> > > It is a gigabit switch with MII/RMII/RGMII MACs - I have never seen
> > > any PHY wired for these modes that can change system interface type.
> >
> > It is unlikely that MII/RMII/RGMII will switch modes, but in terms of
> > correct implementation, sticking to the way the function is expected
> > to behave means that I don't get surprises when changing phylink layer
> > in the future.
> >
> > >
> > > > 2) if phylink is configured to use in-band, then state->speed is
> > > >    undefined; this driver will fail.  (If folk don't want to support
> > > >    that, we ought to have a way to tell phylink to reject anything
> > > >    that attempts to set it to in-band mode!)
> > > >
> > >
> > > Ok.
> > >
> > > > 3) it doesn't implement phylink_mac_link_state DSA ops, so it doesn't
> > > >    support SGMII or 802.3z phy interface modes (see 1).
> > > >
> > >
> > > No, it doesn't.
> > > Some odd switch in this device family supports SGMII on 1 of its
> > > ports, however I haven't put my hands on it.
> > > When I do I'll add checks for strange scenarios, like connecting it to
> > > an Aquantia PHY that can switch between SGMII and USXGMII (although
> > > why would anyone pair a 10G capable PHY to a 1G capable MAC...)
> >
> > It's unlikely that it would switch between SGMII and USXGMII
> > dynamically, as USXGMII supports speeds from 10G down to 10M.
> >
> > Where interface mode switching tends to be used is with modes such
> > as 10GBASE-R, which doesn't support anything except 10G.  In order
> > for the PHY to operate at slower speeds, it has a few options:
> >
> > 1) perform rate adaption.
> > 2) dynamically switch interface type to an interface type that
> >    supports the desired speed.
> > 3) just not support slower speeds.
> >
>
> So am I reading this correctly - it kind of makes sense for gigabit
> MAC drivers to not check for the MII interface changing protocol?
>
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
>
> Thanks,
> -Vladimir

^ permalink raw reply

* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Vladimir Oltean @ 2019-06-25 23:10 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
	matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
	linux-mediatek, linux-mips
In-Reply-To: <20190625225759.zztqgnwtk4v7milp@shell.armlinux.org.uk>

On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > Hi Russell,
> > > >
> > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > This should be removed - state->link is not for use in mac_config.
> > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > >
> > > >
> > > > What do you mean exactly that state->link is not for use, is that true in
> > > > general?
> > >
> > > Yes.  mac_config() should not touch it; it is not always in a defined
> > > state.  For example, if you set modes via ethtool (the
> > > ethtool_ksettings_set API) then state->link will probably contain
> > > zero irrespective of the true link state.
> > >
> >
> > Experimentally, state->link is zero at the same time as state->speed
> > is -1, so just ignoring !state->link made sense. This is not in-band
> > AN. What is your suggestion? Should I proceed to try and configure the
> > MAC for SPEED_UNKNOWN?
>
> What would you have done with a PHY when the link is down, what speed
> would you have configured in the phylib adjust_link callback?  phylib
> also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
>

With phylib, I'd make the driver ignore the speed and do nothing.
With phylink, I'd make the core not call mac_config.
But what happened is I saw phylink call mac_config anyway, said
'weird' and proceeded to ignore it as I would have for phylib.
I'm just not understanding your position - it seems like you're
implying there's a bug in phylink and the function call with
MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
place, which is what I wanted to confirm.

> What we do in Marvell drivers is set to the lowest speed (10M) in such
> cases, which is fine as the MAC supports 10M.
>
> It wouldn't be appropriate for phylink to force something on MAC
> drivers, it's easier if the MAC just defaults SPEED_UNKNOWN to something
> itself.
>
> >
> > > It exists in this structure because it was convenient to just use one
> > > structure to store all the link information in various parts of the
> > > code, and when requesting the negotiated in-band MAC configuration.
> > >
> > > I've come to the conclusion that that decision was a mistake, based
> > > on patches such as the above mistakenly thinking that everything in
> > > the state structure is fair game.  I've since updated the docs to
> > > explicitly spell it out, but I'm also looking at the feasibility of
> > > changing the mac_config() interface entirely - splitting it into two
> > > (mac_config_fixed() and mac_config_inband()) and passing only the
> > > appropriate parameters to each.
> > >
> > > However, having looked at that, I think such a change will make some
> > > MAC drivers quite a bit more complicated - having all the config
> > > steps in one method appears to make the configuration of MAC drivers
> > > easier (eg, mvneta, mvpp2.)
> > >
> > > > In drivers/net/dsa/sja1105/sja1105_main.c, if I remove the "if
> > > > (!state->link)" guard, I see PHYLINK calls with a SPEED_UNKNOWN argument for
> > > > ports that are BR_STATE_DISABLED. Is that normal?
> > >
> > > This looks like another driver which has been converted to phylink
> > > without my review; I certainly wasn't aware of it.  It gets a few
> > > things wrong, such as:
> > >
> > > 1) not checking state->interface in the validate callback - so it
> > >    is basically saying that it can support any PHY interface mode
> > >    that the kernel happens to support.
> > >
> >
> > Partially true. It does check the DT bindings for supported MII modes
> > in sja1105_init_mii_settings (for fundamental reasons... the switch
> > expects an 'all-in-one' configuration buffer with the operating modes
> > of all MACs - don't ask me to delay the uploading of this static
> > config until all ports collected their interface_mode from phylink via
> > the mac_config callback - it's a deadlock).
>
> Ok, so you need to reject interface modes that are not compatible
> with the currently configured mode in the validate() callback, but
> please keep PHY_INTERFACE_MODE_NA reporting back the capabilities.
> (this is now documented.)
>

Ok.

> > It is a gigabit switch with MII/RMII/RGMII MACs - I have never seen
> > any PHY wired for these modes that can change system interface type.
>
> It is unlikely that MII/RMII/RGMII will switch modes, but in terms of
> correct implementation, sticking to the way the function is expected
> to behave means that I don't get surprises when changing phylink layer
> in the future.
>
> >
> > > 2) if phylink is configured to use in-band, then state->speed is
> > >    undefined; this driver will fail.  (If folk don't want to support
> > >    that, we ought to have a way to tell phylink to reject anything
> > >    that attempts to set it to in-band mode!)
> > >
> >
> > Ok.
> >
> > > 3) it doesn't implement phylink_mac_link_state DSA ops, so it doesn't
> > >    support SGMII or 802.3z phy interface modes (see 1).
> > >
> >
> > No, it doesn't.
> > Some odd switch in this device family supports SGMII on 1 of its
> > ports, however I haven't put my hands on it.
> > When I do I'll add checks for strange scenarios, like connecting it to
> > an Aquantia PHY that can switch between SGMII and USXGMII (although
> > why would anyone pair a 10G capable PHY to a 1G capable MAC...)
>
> It's unlikely that it would switch between SGMII and USXGMII
> dynamically, as USXGMII supports speeds from 10G down to 10M.
>
> Where interface mode switching tends to be used is with modes such
> as 10GBASE-R, which doesn't support anything except 10G.  In order
> for the PHY to operate at slower speeds, it has a few options:
>
> 1) perform rate adaption.
> 2) dynamically switch interface type to an interface type that
>    supports the desired speed.
> 3) just not support slower speeds.
>

So am I reading this correctly - it kind of makes sense for gigabit
MAC drivers to not check for the MII interface changing protocol?

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

Thanks,
-Vladimir

^ permalink raw reply

* Re: KASAN: slab-out-of-bounds Write in validate_chain
From: syzbot @ 2019-06-25 23:07 UTC (permalink / raw)
  To: akpm, ast, cai, crecklin, daniel, john.fastabend, keescook,
	linux-kernel, linux-mm, netdev, syzkaller-bugs
In-Reply-To: <000000000000e672c6058bd7ee45@google.com>

syzbot has bisected this bug to:

commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date:   Sat Jun 30 13:17:47 2018 +0000

     bpf: sockhash fix omitted bucket lock in sock_close

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14a4e9b5a00000
start commit:   abf02e29 Merge tag 'pm-5.2-rc6' of git://git.kernel.org/pu..
git tree:       upstream
final crash:    https://syzkaller.appspot.com/x/report.txt?x=16a4e9b5a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=12a4e9b5a00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=28ec3437a5394ee0
dashboard link: https://syzkaller.appspot.com/bug?extid=8893700724999566d6a9
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=167098b2a00000

Reported-by: syzbot+8893700724999566d6a9@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

^ permalink raw reply

* Re: [PATCH bpf-next v9 02/10] bpf: Add eBPF program subtype and is_valid_subtype() verifier
From: Alexei Starovoitov @ 2019-06-25 23:02 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: LKML, Aleksa Sarai, Alexander Viro, Alexei Starovoitov,
	Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
	Casey Schaufler, Daniel Borkmann, David Drysdale,
	David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
	Kernel Hardening, Linux API, Linux-Fsdevel, LSM List,
	Network Development
In-Reply-To: <20190625215239.11136-3-mic@digikod.net>

On Tue, Jun 25, 2019 at 3:04 PM Mickaël Salaün <mic@digikod.net> wrote:
>
> The goal of the program subtype is to be able to have different static
> fine-grained verifications for a unique program type.
>
> The struct bpf_verifier_ops gets a new optional function:
> is_valid_subtype(). This new verifier is called at the beginning of the
> eBPF program verification to check if the (optional) program subtype is
> valid.
>
> The new helper bpf_load_program_xattr() enables to verify a program with
> subtypes.
>
> For now, only Landlock eBPF programs are using a program subtype (see
> next commits) but this could be used by other program types in the
> future.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David S. Miller <davem@davemloft.net>
> Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
> ---
>
> Changes since v8:
> * use bpf_load_program_xattr() instead of bpf_load_program() and add
>   bpf_verify_program_xattr() to deal with subtypes
> * remove put_extra() since there is no more "previous" field (for now)
>
> Changes since v7:
> * rename LANDLOCK_SUBTYPE_* to LANDLOCK_*
> * move subtype in bpf_prog_aux and use only one bit for has_subtype
>   (suggested by Alexei Starovoitov)

sorry to say, but I don't think the landlock will ever land,
since posting huge patches once a year is missing a lot of development
that is happening during that time.
This 2/10 patch is an example.
subtype concept was useful 2 years ago when v6 was posted.
Since then bpf developers faced very similar problem in other parts
and it was solved with 'expected_attach_type' field.
See commit 5e43f899b03a ("bpf: Check attach type at prog load time")
dated March 2018.

^ permalink raw reply

* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Russell King - ARM Linux admin @ 2019-06-25 22:57 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
	matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
	linux-mediatek, linux-mips
In-Reply-To: <CA+h21hqK0VMtHpZ6eka9ESuMhsFTw2mx+c0GYmxq4_G_YmiVpg@mail.gmail.com>

On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > Hi Russell,
> > >
> > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > This should be removed - state->link is not for use in mac_config.
> > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > >
> > >
> > > What do you mean exactly that state->link is not for use, is that true in
> > > general?
> >
> > Yes.  mac_config() should not touch it; it is not always in a defined
> > state.  For example, if you set modes via ethtool (the
> > ethtool_ksettings_set API) then state->link will probably contain
> > zero irrespective of the true link state.
> >
> 
> Experimentally, state->link is zero at the same time as state->speed
> is -1, so just ignoring !state->link made sense. This is not in-band
> AN. What is your suggestion? Should I proceed to try and configure the
> MAC for SPEED_UNKNOWN?

What would you have done with a PHY when the link is down, what speed
would you have configured in the phylib adjust_link callback?  phylib
also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.

What we do in Marvell drivers is set to the lowest speed (10M) in such
cases, which is fine as the MAC supports 10M.

It wouldn't be appropriate for phylink to force something on MAC
drivers, it's easier if the MAC just defaults SPEED_UNKNOWN to something
itself.

> 
> > It exists in this structure because it was convenient to just use one
> > structure to store all the link information in various parts of the
> > code, and when requesting the negotiated in-band MAC configuration.
> >
> > I've come to the conclusion that that decision was a mistake, based
> > on patches such as the above mistakenly thinking that everything in
> > the state structure is fair game.  I've since updated the docs to
> > explicitly spell it out, but I'm also looking at the feasibility of
> > changing the mac_config() interface entirely - splitting it into two
> > (mac_config_fixed() and mac_config_inband()) and passing only the
> > appropriate parameters to each.
> >
> > However, having looked at that, I think such a change will make some
> > MAC drivers quite a bit more complicated - having all the config
> > steps in one method appears to make the configuration of MAC drivers
> > easier (eg, mvneta, mvpp2.)
> >
> > > In drivers/net/dsa/sja1105/sja1105_main.c, if I remove the "if
> > > (!state->link)" guard, I see PHYLINK calls with a SPEED_UNKNOWN argument for
> > > ports that are BR_STATE_DISABLED. Is that normal?
> >
> > This looks like another driver which has been converted to phylink
> > without my review; I certainly wasn't aware of it.  It gets a few
> > things wrong, such as:
> >
> > 1) not checking state->interface in the validate callback - so it
> >    is basically saying that it can support any PHY interface mode
> >    that the kernel happens to support.
> >
> 
> Partially true. It does check the DT bindings for supported MII modes
> in sja1105_init_mii_settings (for fundamental reasons... the switch
> expects an 'all-in-one' configuration buffer with the operating modes
> of all MACs - don't ask me to delay the uploading of this static
> config until all ports collected their interface_mode from phylink via
> the mac_config callback - it's a deadlock).

Ok, so you need to reject interface modes that are not compatible
with the currently configured mode in the validate() callback, but
please keep PHY_INTERFACE_MODE_NA reporting back the capabilities.
(this is now documented.)

> It is a gigabit switch with MII/RMII/RGMII MACs - I have never seen
> any PHY wired for these modes that can change system interface type.

It is unlikely that MII/RMII/RGMII will switch modes, but in terms of
correct implementation, sticking to the way the function is expected
to behave means that I don't get surprises when changing phylink layer
in the future.

> 
> > 2) if phylink is configured to use in-band, then state->speed is
> >    undefined; this driver will fail.  (If folk don't want to support
> >    that, we ought to have a way to tell phylink to reject anything
> >    that attempts to set it to in-band mode!)
> >
> 
> Ok.
> 
> > 3) it doesn't implement phylink_mac_link_state DSA ops, so it doesn't
> >    support SGMII or 802.3z phy interface modes (see 1).
> >
> 
> No, it doesn't.
> Some odd switch in this device family supports SGMII on 1 of its
> ports, however I haven't put my hands on it.
> When I do I'll add checks for strange scenarios, like connecting it to
> an Aquantia PHY that can switch between SGMII and USXGMII (although
> why would anyone pair a 10G capable PHY to a 1G capable MAC...)

It's unlikely that it would switch between SGMII and USXGMII
dynamically, as USXGMII supports speeds from 10G down to 10M.

Where interface mode switching tends to be used is with modes such
as 10GBASE-R, which doesn't support anything except 10G.  In order
for the PHY to operate at slower speeds, it has a few options:

1) perform rate adaption.
2) dynamically switch interface type to an interface type that
   supports the desired speed.
3) just not support slower speeds.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* [PATCH bpf-next] selftests/bpf: build tests with debug info
From: Andrii Nakryiko @ 2019-06-25 22:56 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf; +Cc: Andrii Nakryiko

Non-BPF (user land) part of selftests is built without debug info making
occasional debugging with gdb terrible. Build with debug info always.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f2dbe2043067..faeb8220daa2 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -15,7 +15,7 @@ LLC		?= llc
 LLVM_OBJCOPY	?= llvm-objcopy
 LLVM_READELF	?= llvm-readelf
 BTF_PAHOLE	?= pahole
-CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
+CFLAGS += -g -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
 	  -Dbpf_prog_load=bpf_prog_test_load \
 	  -Dbpf_load_program=bpf_test_load_program
 LDLIBS += -lcap -lelf -lrt -lpthread
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: kbuild test robot @ 2019-06-25 22:52 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: kbuild-all, alastair, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	David Airlie, Daniel Vetter, Dan Carpenter, Karsten Keil,
	Jassi Brar, Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <20190625031726.12173-5-alastair@au1.ibm.com>

Hi Alastair,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.2-rc6 next-20190625]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   sound/soc/intel/skylake/skl-debug.c:191:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@    expected void [noderef] <asn:2> *to @@    got eref] <asn:2> *to @@
   sound/soc/intel/skylake/skl-debug.c:191:34: sparse:    expected void [noderef] <asn:2> *to
   sound/soc/intel/skylake/skl-debug.c:191:34: sparse:    got unsigned char *
   sound/soc/intel/skylake/skl-debug.c:191:51: sparse: sparse: incorrect type in argument 2 (different address spaces) @@    expected void const *from @@    got void [noderef] <asn:2> void const *from @@
   sound/soc/intel/skylake/skl-debug.c:191:51: sparse:    expected void const *from
   sound/soc/intel/skylake/skl-debug.c:191:51: sparse:    got void [noderef] <asn:2> *[assigned] fw_reg_addr
>> sound/soc/intel/skylake/skl-debug.c:195:35: sparse: sparse: too many arguments for function hex_dump_to_buffer
--
>> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c:93:27: sparse: sparse: too many arguments for function hex_dump_to_buffer
--
>> sound/soc/sof/xtensa/core.c:125:35: sparse: sparse: too many arguments for function hex_dump_to_buffer

vim +195 sound/soc/intel/skylake/skl-debug.c

d14700a0 Vinod Koul  2017-06-30  170  
bdd0384a Vunny Sodhi 2017-06-30  171  static ssize_t fw_softreg_read(struct file *file, char __user *user_buf,
bdd0384a Vunny Sodhi 2017-06-30  172  			       size_t count, loff_t *ppos)
bdd0384a Vunny Sodhi 2017-06-30  173  {
bdd0384a Vunny Sodhi 2017-06-30  174  	struct skl_debug *d = file->private_data;
bdd0384a Vunny Sodhi 2017-06-30  175  	struct sst_dsp *sst = d->skl->skl_sst->dsp;
bdd0384a Vunny Sodhi 2017-06-30  176  	size_t w0_stat_sz = sst->addr.w0_stat_sz;
bdd0384a Vunny Sodhi 2017-06-30  177  	void __iomem *in_base = sst->mailbox.in_base;
bdd0384a Vunny Sodhi 2017-06-30  178  	void __iomem *fw_reg_addr;
bdd0384a Vunny Sodhi 2017-06-30  179  	unsigned int offset;
bdd0384a Vunny Sodhi 2017-06-30  180  	char *tmp;
bdd0384a Vunny Sodhi 2017-06-30  181  	ssize_t ret = 0;
bdd0384a Vunny Sodhi 2017-06-30  182  
bdd0384a Vunny Sodhi 2017-06-30  183  	tmp = kzalloc(FW_REG_BUF, GFP_KERNEL);
bdd0384a Vunny Sodhi 2017-06-30  184  	if (!tmp)
bdd0384a Vunny Sodhi 2017-06-30  185  		return -ENOMEM;
bdd0384a Vunny Sodhi 2017-06-30  186  
bdd0384a Vunny Sodhi 2017-06-30  187  	fw_reg_addr = in_base - w0_stat_sz;
bdd0384a Vunny Sodhi 2017-06-30  188  	memset(d->fw_read_buff, 0, FW_REG_BUF);
bdd0384a Vunny Sodhi 2017-06-30  189  
bdd0384a Vunny Sodhi 2017-06-30  190  	if (w0_stat_sz > 0)
bdd0384a Vunny Sodhi 2017-06-30 @191  		__iowrite32_copy(d->fw_read_buff, fw_reg_addr, w0_stat_sz >> 2);
bdd0384a Vunny Sodhi 2017-06-30  192  
bdd0384a Vunny Sodhi 2017-06-30  193  	for (offset = 0; offset < FW_REG_SIZE; offset += 16) {
bdd0384a Vunny Sodhi 2017-06-30  194  		ret += snprintf(tmp + ret, FW_REG_BUF - ret, "%#.4x: ", offset);
bdd0384a Vunny Sodhi 2017-06-30 @195  		hex_dump_to_buffer(d->fw_read_buff + offset, 16, 16, 4,
bdd0384a Vunny Sodhi 2017-06-30  196  				   tmp + ret, FW_REG_BUF - ret, 0);
bdd0384a Vunny Sodhi 2017-06-30  197  		ret += strlen(tmp + ret);
bdd0384a Vunny Sodhi 2017-06-30  198  
bdd0384a Vunny Sodhi 2017-06-30  199  		/* print newline for each offset */
bdd0384a Vunny Sodhi 2017-06-30  200  		if (FW_REG_BUF - ret > 0)
bdd0384a Vunny Sodhi 2017-06-30  201  			tmp[ret++] = '\n';
bdd0384a Vunny Sodhi 2017-06-30  202  	}
bdd0384a Vunny Sodhi 2017-06-30  203  
bdd0384a Vunny Sodhi 2017-06-30  204  	ret = simple_read_from_buffer(user_buf, count, ppos, tmp, ret);
bdd0384a Vunny Sodhi 2017-06-30  205  	kfree(tmp);
bdd0384a Vunny Sodhi 2017-06-30  206  
bdd0384a Vunny Sodhi 2017-06-30  207  	return ret;
bdd0384a Vunny Sodhi 2017-06-30  208  }
bdd0384a Vunny Sodhi 2017-06-30  209  

:::::: The code at line 195 was first introduced by commit
:::::: bdd0384a5ada8bb5745e5f29c10a5ba88827efad ASoC: Intel: Skylake: Add support to read firmware registers

:::::: TO: Vunny Sodhi <vunnyx.sodhi@intel.com>
:::::: CC: Mark Brown <broonie@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* Re: [PATCH bpf-next v9 05/10] bpf,landlock: Add a new map type: inode
From: Al Viro @ 2019-06-25 22:52 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Andrew Morton,
	Andy Lutomirski, Arnaldo Carvalho de Melo, Casey Schaufler,
	Daniel Borkmann, David Drysdale, David S . Miller,
	Eric W . Biederman, James Morris, Jann Horn, John Johansen,
	Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
	kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
	netdev
In-Reply-To: <20190625215239.11136-6-mic@digikod.net>

On Tue, Jun 25, 2019 at 11:52:34PM +0200, Mickaël Salaün wrote:
> +/* must call iput(inode) after this call */
> +static struct inode *inode_from_fd(int ufd, bool check_access)
> +{
> +	struct inode *ret;
> +	struct fd f;
> +	int deny;
> +
> +	f = fdget(ufd);
> +	if (unlikely(!f.file || !file_inode(f.file))) {
> +		ret = ERR_PTR(-EBADF);
> +		goto put_fd;
> +	}

Just when does one get a NULL file_inode()?  The reason I'm asking is
that arseloads of code would break if one managed to create such
a beast...

Incidentally, that should be return ERR_PTR(-EBADF); fdput() is wrong there.

> +	}
> +	/* check if the FD is tied to a mount point */
> +	/* TODO: add this check when called from an eBPF program too */
> +	if (unlikely(!f.file->f_path.mnt

Again, the same question - when the hell can that happen?  If you are
sitting on an exploitable roothole, do share it...

 || f.file->f_path.mnt->mnt_flags &
> +				MNT_INTERNAL)) {
> +		ret = ERR_PTR(-EINVAL);
> +		goto put_fd;

What does it have to do with mountpoints, anyway?

> +/* called from syscall */
> +static int sys_inode_map_delete_elem(struct bpf_map *map, struct inode *key)
> +{
> +	struct inode_array *array = container_of(map, struct inode_array, map);
> +	struct inode *inode;
> +	int i;
> +
> +	WARN_ON_ONCE(!rcu_read_lock_held());
> +	for (i = 0; i < array->map.max_entries; i++) {
> +		if (array->elems[i].inode == key) {
> +			inode = xchg(&array->elems[i].inode, NULL);
> +			array->nb_entries--;

Umm...  Is that intended to be atomic in any sense?

> +			iput(inode);
> +			return 0;
> +		}
> +	}
> +	return -ENOENT;
> +}
> +
> +/* called from syscall */
> +int bpf_inode_map_delete_elem(struct bpf_map *map, int *key)
> +{
> +	struct inode *inode;
> +	int err;
> +
> +	inode = inode_from_fd(*key, false);
> +	if (IS_ERR(inode))
> +		return PTR_ERR(inode);
> +	err = sys_inode_map_delete_elem(map, inode);
> +	iput(inode);
> +	return err;
> +}

Wait a sec...  So we have those beasties that can have long-term
references to arbitrary inodes stuck in them?  What will happen
if you get umount(2) called while such a thing exists?

^ permalink raw reply

* Re: [RFC PATCH v4 net-next 10/11] ARM: dts: am57xx-idk: add dt nodes for new cpsw switch dev driver
From: Ivan Khoronzhuk @ 2019-06-25 22:49 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: netdev, Ilias Apalodimas, Andrew Lunn, David S . Miller,
	Jiri Pirko, Florian Fainelli, Sekhar Nori, linux-kernel,
	linux-omap, Murali Karicheri, Ivan Vecera, Rob Herring,
	devicetree
In-Reply-To: <20190621181314.20778-11-grygorii.strashko@ti.com>

On Fri, Jun 21, 2019 at 09:13:13PM +0300, Grygorii Strashko wrote:
>Add DT nodes for new cpsw switch dev driver.
>
>Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>---
> arch/arm/boot/dts/am571x-idk.dts         | 28 +++++++++++++
> arch/arm/boot/dts/am572x-idk.dts         |  5 +++
> arch/arm/boot/dts/am574x-idk.dts         |  5 +++
> arch/arm/boot/dts/am57xx-idk-common.dtsi |  2 +-
> arch/arm/boot/dts/dra7-l4.dtsi           | 53 ++++++++++++++++++++++++
> 5 files changed, 92 insertions(+), 1 deletion(-)
>

[...]

>diff --git a/arch/arm/boot/dts/am57xx-idk-common.dtsi 
>b/arch/arm/boot/dts/am57xx-idk-common.dtsi
>index f7bd26458915..5c7663699efa 100644
>--- a/arch/arm/boot/dts/am57xx-idk-common.dtsi
>+++ b/arch/arm/boot/dts/am57xx-idk-common.dtsi
>@@ -367,7 +367,7 @@
> };
>
> &mac {
>-	status = "okay";
>+//	status = "okay";
?


-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [PATCH bpf-next 0/4] sys_bpf() access control via /dev/bpf
From: Alexei Starovoitov @ 2019-06-25 22:47 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Song Liu, netdev@vger.kernel.org, bpf@vger.kernel.org,
	ast@kernel.org, daniel@iogearbox.net, Kernel Team
In-Reply-To: <20190625211948.GE10487@mini-arch>

On 6/25/19 2:19 PM, Stanislav Fomichev wrote:
> On 06/25, Alexei Starovoitov wrote:
>> On 6/25/19 1:51 PM, Stanislav Fomichev wrote:
>>> On 06/25, Song Liu wrote:
>>>> Currently, most access to sys_bpf() is limited to root. However, there are
>>>> use cases that would benefit from non-privileged use of sys_bpf(), e.g.
>>>> systemd.
>>>>
>>>> This set introduces a new model to control the access to sys_bpf(). A
>>>> special device, /dev/bpf, is introduced to manage access to sys_bpf().
>>>> Users with access to open /dev/bpf will be able to access most of
>>>> sys_bpf() features. The use can get access to sys_bpf() by opening /dev/bpf
>>>> and use ioctl to get/put permission.
>>>>
>>>> The permission to access sys_bpf() is marked by bit TASK_BPF_FLAG_PERMITTED
>>>> in task_struct. During fork(), child will not inherit this bit.
>>> 2c: if we are going to have an fd, I'd vote for a proper fd based access
>>> checks instead of a per-task flag, so we can do:
>>> 	ioctl(fd, BPF_MAP_CREATE, uattr, sizeof(uattr))
>>>
>>> (and pass this fd around)
>>>
>>> I do understand that it breaks current assumptions that libbpf has,
>>> but maybe we can extend _xattr variants to accept optinal fd (and try
>>> to fallback to sysctl if it's absent/not working)?
>>
>> both of these ideas were discussed at lsfmm where you were present.
>> I'm not sure why you're bring it up again?
> Did we actually settle on anything? In that case feel free to ignore me,
> maybe I missed that. I remember there were pros/cons for both implementations.

yes. That was my understanding from lsfmm.
Which was:
1. replicating all commands via ioctl is not going to work.
   Also ioctl cannot return fd.
2. adding fd to all structs inside bpf_attr is a big churn on uapi.
   all future structs would need to have this extra fd as well.
   I don't like that kind of crutch to be carried over and over again.

The only thing we can consider instead of ioctl is to add single
new command for bpf syscall that will take that fd and apply
the attribute to task struct.
ioctl on that fd or new command look equivalent to me.

^ permalink raw reply

* Re: [PATCH bpf-next v5 1/2] bpf: Allow bpf_skb_event_output for a few prog types
From: Alexei Starovoitov @ 2019-06-25 22:37 UTC (permalink / raw)
  To: allanzhang
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, Network Development, bpf, LKML
In-Reply-To: <20190625222329.209732-2-allanzhang@google.com>

On Tue, Jun 25, 2019 at 3:23 PM allanzhang <allanzhang@google.com> wrote:
>
> Signed-off-by: allanzhang <allanzhang@google.com>

As Daniel mentioned SOB needs to have real name.
If you keep ignoring feedback we'll keep rejecting patches without comments.

^ permalink raw reply

* Re: [PATCH v4 net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Willem de Bruijn @ 2019-06-25 22:30 UTC (permalink / raw)
  To: Neil Horman; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <20190625215749.22840-1-nhorman@tuxdriver.com>

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index a29d66da7394..a7ca6a003ebe 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2401,6 +2401,9 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
>
>                 ts = __packet_set_timestamp(po, ph, skb);
>                 __packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts);
> +
> +               if (!packet_read_pending(&po->tx_ring))
> +                       complete(&po->skb_completion);
>         }
>
>         sock_wfree(skb);
> @@ -2585,7 +2588,7 @@ static int tpacket_parse_header(struct packet_sock *po, void *frame,
>
>  static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  {
> -       struct sk_buff *skb;
> +       struct sk_buff *skb = NULL;
>         struct net_device *dev;
>         struct virtio_net_hdr *vnet_hdr = NULL;
>         struct sockcm_cookie sockc;
> @@ -2600,6 +2603,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>         int len_sum = 0;
>         int status = TP_STATUS_AVAILABLE;
>         int hlen, tlen, copylen = 0;
> +       long timeo = 0;
>
>         mutex_lock(&po->pg_vec_lock);
>
> @@ -2646,12 +2650,21 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>         if ((size_max > dev->mtu + reserve + VLAN_HLEN) && !po->has_vnet_hdr)
>                 size_max = dev->mtu + reserve + VLAN_HLEN;
>
> +       reinit_completion(&po->skb_completion);
> +
>         do {
>                 ph = packet_current_frame(po, &po->tx_ring,
>                                           TP_STATUS_SEND_REQUEST);
>                 if (unlikely(ph == NULL)) {
> -                       if (need_wait && need_resched())
> -                               schedule();
> +                       if (need_wait && skb) {
> +                               timeo = sock_sndtimeo(&po->sk, msg->msg_flags & MSG_DONTWAIT);
> +                               timeo = wait_for_completion_interruptible_timeout(&po->skb_completion, timeo);

This looks really nice.

But isn't it still susceptible to the race where tpacket_destruct_skb
is called in between po->xmit and this
wait_for_completion_interruptible_timeout?

The test for skb is shorthand for packet_read_pending  != 0, right?

> +                               if (timeo <= 0) {
> +                                       err = !timeo ? -ETIMEDOUT : -ERESTARTSYS;
> +                                       goto out_put;
> +                               }
> +                       }
> +                       /* check for additional frames */
>                         continue;
>                 }
>

^ permalink raw reply

* [PATCH bpf-next v9 06/10] landlock: Handle filesystem access control
From: Mickaël Salaün @ 2019-06-25 21:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Aleksa Sarai, Alexander Viro,
	Alexei Starovoitov, Andrew Morton, Andy Lutomirski,
	Arnaldo Carvalho de Melo, Casey Schaufler, Daniel Borkmann,
	David Drysdale, David S . Miller, Eric W . Biederman,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, Michael Kerrisk, Mickaël Salaün, Paul Moore,
	Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Stephen Smalley,
	Tejun Heo, Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
	kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
	netdev
In-Reply-To: <20190625215239.11136-1-mic@digikod.net>

This add two Landlock hooks: FS_WALK and FS_PICK.

The FS_WALK hook is used to walk through a file path. A program tied to
this hook will be evaluated for each directory traversal except the last
one if it is the leaf of the path.  It is important to differentiate
this hook from FS_PICK to enable more powerful path evaluation in the
future (cf. Landlock patch v8).

The FS_PICK hook is used to validate a set of actions requested on a
file. This actions are defined with triggers (e.g. read, write, open,
append...).

The Landlock LSM hook registration is done after other LSM to only run
actions from user-space, via eBPF programs, if the access was granted by
major (privileged) LSMs.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
---

Changes since v8:
* add a new LSM_ORDER_LAST, cf. commit e2bc445b66ca ("LSM: Introduce
  enum lsm_order")
* add WARN_ON() for pointer dereferencement
* remove the FS_GET subtype which rely on program chaining
* remove the subtype option which was only used for chaining (with the
  "previous" field)
* remove inode_lookup which depends on the (removed) nameidata security
  blob
* remove eBPF helpers to get and set Landlock inode tags
* do not use task LSM credentials (for now)

Changes since v7:
* major rewrite with clean Landlock hooks able to deal with file paths

Changes since v6:
* add 3 more sub-events: IOCTL, LOCK, FCNTL
  https://lkml.kernel.org/r/2fbc99a6-f190-f335-bd14-04bdeed35571@digikod.net
* use the new security_add_hooks()
* explain the -Werror=unused-function
* constify pointers
* cleanup headers

Changes since v5:
* split hooks.[ch] into hooks.[ch] and hooks_fs.[ch]
* add more documentation
* cosmetic fixes
* rebase (SCALAR_VALUE)

Changes since v4:
* add LSM hook abstraction called Landlock event
  * use the compiler type checking to verify hooks use by an event
  * handle all filesystem related LSM hooks (e.g. file_permission,
    mmap_file, sb_mount...)
* register BPF programs for Landlock just after LSM hooks registration
* move hooks registration after other LSMs
* add failsafes to check if a hook is not used by the kernel
* allow partial raw value access form the context (needed for programs
  generated by LLVM)

Changes since v3:
* split commit
* add hooks dealing with struct inode and struct path pointers:
  inode_permission and inode_getattr
* add abstraction over eBPF helper arguments thanks to wrapping structs
---
 include/linux/lsm_hooks.h    |   1 +
 security/landlock/Makefile   |   3 +-
 security/landlock/common.h   |  10 +
 security/landlock/hooks.c    |  95 ++++++
 security/landlock/hooks.h    |  31 ++
 security/landlock/hooks_fs.c | 568 +++++++++++++++++++++++++++++++++++
 security/landlock/hooks_fs.h |  31 ++
 security/landlock/init.c     |  47 +++
 security/security.c          |  15 +
 9 files changed, 800 insertions(+), 1 deletion(-)
 create mode 100644 security/landlock/hooks.c
 create mode 100644 security/landlock/hooks.h
 create mode 100644 security/landlock/hooks_fs.c
 create mode 100644 security/landlock/hooks_fs.h

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 47f58cfb6a19..eefe9f214f05 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2092,6 +2092,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,
 enum lsm_order {
 	LSM_ORDER_FIRST = -1,	/* This is only for capabilities. */
 	LSM_ORDER_MUTABLE = 0,
+	LSM_ORDER_LAST = 1,	/* potentially-unprivileged LSM */
 };
 
 struct lsm_info {
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 2a1a7082a365..270ece5d93de 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 
 landlock-y := init.o \
-	enforce.o enforce_seccomp.o
+	enforce.o enforce_seccomp.o \
+	hooks.o hooks_fs.o
diff --git a/security/landlock/common.h b/security/landlock/common.h
index 0c9b5904e7f5..49b892515144 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -11,6 +11,7 @@
 
 #include <linux/bpf.h> /* enum bpf_prog_aux */
 #include <linux/filter.h> /* bpf_prog */
+#include <linux/lsm_hooks.h> /* lsm_blob_sizes */
 #include <linux/refcount.h> /* refcount_t */
 #include <uapi/linux/landlock.h> /* enum landlock_hook_type */
 
@@ -68,4 +69,13 @@ static inline enum landlock_hook_type get_type(struct bpf_prog *prog)
 	return prog->aux->extra->subtype.landlock_hook.type;
 }
 
+__maybe_unused
+static bool current_has_prog_type(enum landlock_hook_type hook_type)
+{
+	struct landlock_prog_set *prog_set;
+
+	prog_set = current->seccomp.landlock_prog_set;
+	return (prog_set && prog_set->programs[get_index(hook_type)]);
+}
+
 #endif /* _SECURITY_LANDLOCK_COMMON_H */
diff --git a/security/landlock/hooks.c b/security/landlock/hooks.c
new file mode 100644
index 000000000000..a1620d0481eb
--- /dev/null
+++ b/security/landlock/hooks.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - hook helpers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <asm/current.h>
+#include <linux/bpf.h> /* enum bpf_prog_aux */
+#include <linux/errno.h>
+#include <linux/filter.h> /* BPF_PROG_RUN() */
+#include <linux/rculist.h> /* list_add_tail_rcu */
+#include <uapi/linux/landlock.h> /* struct landlock_context */
+
+#include "common.h" /* struct landlock_rule, get_index() */
+#include "hooks.h" /* landlock_hook_ctx */
+
+#include "hooks_fs.h"
+
+/* return a Landlock program context (e.g. hook_ctx->fs_walk.prog_ctx) */
+static const void *get_ctx(enum landlock_hook_type hook_type,
+		struct landlock_hook_ctx *hook_ctx)
+{
+	switch (hook_type) {
+	case LANDLOCK_HOOK_FS_WALK:
+		return landlock_get_ctx_fs_walk(hook_ctx->fs_walk);
+	case LANDLOCK_HOOK_FS_PICK:
+		return landlock_get_ctx_fs_pick(hook_ctx->fs_pick);
+	}
+	WARN_ON(1);
+	return NULL;
+}
+
+/**
+ * landlock_access_deny - run Landlock programs tied to a hook
+ *
+ * @hook_idx: hook index in the programs array
+ * @ctx: non-NULL valid eBPF context
+ * @prog_set: Landlock program set pointer
+ * @triggers: a bitmask to check if a program should be run
+ *
+ * Return true if at least one program return deny.
+ */
+static bool landlock_access_deny(enum landlock_hook_type hook_type,
+		struct landlock_hook_ctx *hook_ctx,
+		struct landlock_prog_set *prog_set, u64 triggers)
+{
+	struct landlock_prog_list *prog_list, *prev_list = NULL;
+	u32 hook_idx = get_index(hook_type);
+
+	if (!prog_set)
+		return false;
+
+	for (prog_list = prog_set->programs[hook_idx];
+			prog_list; prog_list = prog_list->prev) {
+		u32 ret;
+		const void *prog_ctx;
+
+		/* check if @prog expect at least one of this triggers */
+		if (triggers && !(triggers & prog_list->prog->aux->extra->
+					subtype.landlock_hook.triggers))
+			continue;
+		prog_ctx = get_ctx(hook_type, hook_ctx);
+		if (!prog_ctx || WARN_ON(IS_ERR(prog_ctx)))
+			return true;
+		rcu_read_lock();
+		ret = BPF_PROG_RUN(prog_list->prog, prog_ctx);
+		rcu_read_unlock();
+		/* deny access if a program returns a value different than 0 */
+		if (ret)
+			return true;
+		if (prev_list && prog_list->prev && prog_list->prev->prog->
+				aux->extra->subtype.landlock_hook.type ==
+				prev_list->prog->aux->extra->
+				subtype.landlock_hook.type)
+			WARN_ON(prog_list->prev != prev_list);
+		prev_list = prog_list;
+	}
+	return false;
+}
+
+int landlock_decide(enum landlock_hook_type hook_type,
+		struct landlock_hook_ctx *hook_ctx, u64 triggers)
+{
+	bool deny = false;
+
+#ifdef CONFIG_SECCOMP_FILTER
+	deny = landlock_access_deny(hook_type, hook_ctx,
+			current->seccomp.landlock_prog_set, triggers);
+#endif /* CONFIG_SECCOMP_FILTER */
+
+	/* should we use -EPERM or -EACCES? */
+	return deny ? -EACCES : 0;
+}
diff --git a/security/landlock/hooks.h b/security/landlock/hooks.h
new file mode 100644
index 000000000000..31446e6629fb
--- /dev/null
+++ b/security/landlock/hooks.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - hooks helpers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <asm/current.h>
+#include <linux/sched.h> /* struct task_struct */
+#include <linux/seccomp.h>
+
+#include "hooks_fs.h"
+
+struct landlock_hook_ctx {
+	union {
+		struct landlock_hook_ctx_fs_walk *fs_walk;
+		struct landlock_hook_ctx_fs_pick *fs_pick;
+	};
+};
+
+static inline bool landlocked(const struct task_struct *task)
+{
+#ifdef CONFIG_SECCOMP_FILTER
+	return !!(task->seccomp.landlock_prog_set);
+#else
+	return false;
+#endif /* CONFIG_SECCOMP_FILTER */
+}
+
+int landlock_decide(enum landlock_hook_type, struct landlock_hook_ctx *, u64);
diff --git a/security/landlock/hooks_fs.c b/security/landlock/hooks_fs.c
new file mode 100644
index 000000000000..c3f0f60d72a7
--- /dev/null
+++ b/security/landlock/hooks_fs.c
@@ -0,0 +1,568 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - filesystem hooks
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/bpf.h> /* enum bpf_access_type */
+#include <linux/kernel.h> /* ARRAY_SIZE */
+#include <linux/lsm_hooks.h>
+#include <linux/rcupdate.h> /* synchronize_rcu() */
+#include <linux/stat.h> /* S_ISDIR */
+#include <linux/stddef.h> /* offsetof */
+#include <linux/types.h> /* uintptr_t */
+#include <linux/workqueue.h> /* INIT_WORK() */
+
+/* permissions translation */
+#include <linux/fs.h> /* MAY_* */
+#include <linux/mman.h> /* PROT_* */
+#include <linux/namei.h>
+
+/* hook arguments */
+#include <linux/dcache.h> /* struct dentry */
+#include <linux/fs.h> /* struct inode, struct iattr */
+#include <linux/mm_types.h> /* struct vm_area_struct */
+#include <linux/mount.h> /* struct vfsmount */
+#include <linux/path.h> /* struct path */
+#include <linux/sched.h> /* struct task_struct */
+#include <linux/time.h> /* struct timespec */
+
+#include "common.h"
+#include "hooks_fs.h"
+#include "hooks.h"
+
+/* fs_pick */
+
+#include <asm/page.h> /* PAGE_SIZE */
+#include <asm/syscall.h>
+#include <linux/dcache.h> /* d_path, dentry_path_raw */
+#include <linux/err.h> /* *_ERR */
+#include <linux/gfp.h> /* __get_free_page, GFP_KERNEL */
+#include <linux/path.h> /* struct path */
+
+bool landlock_is_valid_access_fs_pick(int off, enum bpf_access_type type,
+		enum bpf_reg_type *reg_type, int *max_size)
+{
+	switch (off) {
+	case offsetof(struct landlock_ctx_fs_pick, inode):
+		if (type != BPF_READ)
+			return false;
+		*reg_type = PTR_TO_INODE;
+		*max_size = sizeof(u64);
+		return true;
+	default:
+		return false;
+	}
+}
+
+bool landlock_is_valid_access_fs_walk(int off, enum bpf_access_type type,
+		enum bpf_reg_type *reg_type, int *max_size)
+{
+	switch (off) {
+	case offsetof(struct landlock_ctx_fs_walk, inode):
+		if (type != BPF_READ)
+			return false;
+		*reg_type = PTR_TO_INODE;
+		*max_size = sizeof(u64);
+		return true;
+	default:
+		return false;
+	}
+}
+
+/* fs_walk */
+
+struct landlock_hook_ctx_fs_walk {
+	struct landlock_ctx_fs_walk prog_ctx;
+};
+
+const struct landlock_ctx_fs_walk *landlock_get_ctx_fs_walk(
+		const struct landlock_hook_ctx_fs_walk *hook_ctx)
+{
+	if (WARN_ON(!hook_ctx))
+		return NULL;
+
+	return &hook_ctx->prog_ctx;
+}
+
+static int decide_fs_walk(int may_mask, struct inode *inode)
+{
+	struct landlock_hook_ctx_fs_walk fs_walk = {};
+	struct landlock_hook_ctx hook_ctx = {
+		.fs_walk = &fs_walk,
+	};
+	const enum landlock_hook_type hook_type = LANDLOCK_HOOK_FS_WALK;
+
+	if (!current_has_prog_type(hook_type))
+		/* no fs_walk */
+		return 0;
+	if (WARN_ON(!inode))
+		return -EFAULT;
+
+	/* init common data: inode, is_dot, is_dotdot, is_root */
+	fs_walk.prog_ctx.inode = (uintptr_t)inode;
+	return landlock_decide(hook_type, &hook_ctx, 0);
+}
+
+/* fs_pick */
+
+struct landlock_hook_ctx_fs_pick {
+	__u64 triggers;
+	struct landlock_ctx_fs_pick prog_ctx;
+};
+
+const struct landlock_ctx_fs_pick *landlock_get_ctx_fs_pick(
+		const struct landlock_hook_ctx_fs_pick *hook_ctx)
+{
+	if (WARN_ON(!hook_ctx))
+		return NULL;
+
+	return &hook_ctx->prog_ctx;
+}
+
+static int decide_fs_pick(__u64 triggers, struct inode *inode)
+{
+	struct landlock_hook_ctx_fs_pick fs_pick = {};
+	struct landlock_hook_ctx hook_ctx = {
+		.fs_pick = &fs_pick,
+	};
+	const enum landlock_hook_type hook_type = LANDLOCK_HOOK_FS_PICK;
+
+	if (WARN_ON(!triggers))
+		return 0;
+	if (!current_has_prog_type(hook_type))
+		/* no fs_pick */
+		return 0;
+	if (WARN_ON(!inode))
+		return -EFAULT;
+
+	fs_pick.triggers = triggers,
+	/* init common data: inode */
+	fs_pick.prog_ctx.inode = (uintptr_t)inode;
+	return landlock_decide(hook_type, &hook_ctx, fs_pick.triggers);
+}
+
+/* helpers */
+
+static u64 fs_may_to_triggers(int may_mask, umode_t mode)
+{
+	u64 ret = 0;
+
+	if (may_mask & MAY_EXEC)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_EXECUTE;
+	if (may_mask & MAY_READ) {
+		if (S_ISDIR(mode))
+			ret |= LANDLOCK_TRIGGER_FS_PICK_READDIR;
+		else
+			ret |= LANDLOCK_TRIGGER_FS_PICK_READ;
+	}
+	if (may_mask & MAY_WRITE)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_WRITE;
+	if (may_mask & MAY_APPEND)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_APPEND;
+	if (may_mask & MAY_OPEN)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_OPEN;
+	if (may_mask & MAY_CHROOT)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_CHROOT;
+	else if (may_mask & MAY_CHDIR)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_CHDIR;
+	/* XXX: ignore MAY_ACCESS */
+	WARN_ON(!ret);
+	return ret;
+}
+
+static inline u64 mem_prot_to_triggers(unsigned long prot, bool private)
+{
+	u64 ret = LANDLOCK_TRIGGER_FS_PICK_MAP;
+
+	/* private mapping do not write to files */
+	if (!private && (prot & PROT_WRITE))
+		ret |= LANDLOCK_TRIGGER_FS_PICK_WRITE;
+	if (prot & PROT_READ)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_READ;
+	if (prot & PROT_EXEC)
+		ret |= LANDLOCK_TRIGGER_FS_PICK_EXECUTE;
+	WARN_ON(!ret);
+	return ret;
+}
+
+/* binder hooks */
+
+static int hook_binder_transfer_file(struct task_struct *from,
+		struct task_struct *to, struct file *file)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!file))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_TRANSFER,
+			file_inode(file));
+}
+
+/* sb hooks */
+
+static int hook_sb_statfs(struct dentry *dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+			dentry->d_inode);
+}
+
+/* TODO: handle mount source and remount */
+static int hook_sb_mount(const char *dev_name, const struct path *path,
+		const char *type, unsigned long flags, void *data)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!path))
+		return 0;
+	if (WARN_ON(!path->dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_MOUNTON,
+			path->dentry->d_inode);
+}
+
+/*
+ * The @old_path is similar to a destination mount point.
+ */
+static int hook_sb_pivotroot(const struct path *old_path,
+		const struct path *new_path)
+{
+	int err;
+
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!old_path))
+		return 0;
+	if (WARN_ON(!old_path->dentry))
+		return 0;
+	err = decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_MOUNTON,
+			old_path->dentry->d_inode);
+	if (err)
+		return err;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CHROOT,
+			new_path->dentry->d_inode);
+}
+
+/* inode hooks */
+
+/* a directory inode contains only one dentry */
+static int hook_inode_create(struct inode *dir, struct dentry *dentry,
+		umode_t mode)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_link(struct dentry *old_dentry, struct inode *dir,
+		struct dentry *new_dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!old_dentry)) {
+		int ret = decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_LINK,
+				old_dentry->d_inode);
+		if (ret)
+			return ret;
+	}
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_LINKTO, dir);
+}
+
+static int hook_inode_unlink(struct inode *dir, struct dentry *dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_UNLINK,
+			dentry->d_inode);
+}
+
+static int hook_inode_symlink(struct inode *dir, struct dentry *dentry,
+		const char *old_name)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_mkdir(struct inode *dir, struct dentry *dentry,
+		umode_t mode)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RMDIR, dentry->d_inode);
+}
+
+static int hook_inode_mknod(struct inode *dir, struct dentry *dentry,
+		umode_t mode, dev_t dev)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
+		struct inode *new_dir, struct dentry *new_dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	/* TODO: add artificial walk session from old_dir to old_dentry */
+	if (!WARN_ON(!old_dentry)) {
+		int ret = decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RENAME,
+				old_dentry->d_inode);
+		if (ret)
+			return ret;
+	}
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RENAMETO, new_dir);
+}
+
+static int hook_inode_readlink(struct dentry *dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_READ, dentry->d_inode);
+}
+
+/*
+ * ignore the inode_follow_link hook (could set is_symlink in the fs_walk
+ * context)
+ */
+
+static int hook_inode_permission(struct inode *inode, int mask)
+{
+	u64 triggers;
+
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!inode))
+		return 0;
+
+	triggers = fs_may_to_triggers(mask, inode->i_mode);
+	/*
+	 * decide_fs_walk() is exclusive with decide_fs_pick(): in a path walk,
+	 * ignore execute-only access on directory for any fs_pick program
+	 */
+	if (triggers == LANDLOCK_TRIGGER_FS_PICK_EXECUTE &&
+			S_ISDIR(inode->i_mode))
+		return decide_fs_walk(mask, inode);
+
+	return decide_fs_pick(triggers, inode);
+}
+
+static int hook_inode_setattr(struct dentry *dentry, struct iattr *attr)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR,
+			dentry->d_inode);
+}
+
+static int hook_inode_getattr(const struct path *path)
+{
+	/* TODO: link parent inode and path */
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!path))
+		return 0;
+	if (WARN_ON(!path->dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+			path->dentry->d_inode);
+}
+
+static int hook_inode_setxattr(struct dentry *dentry, const char *name,
+		const void *value, size_t size, int flags)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR,
+			dentry->d_inode);
+}
+
+static int hook_inode_getxattr(struct dentry *dentry, const char *name)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+			dentry->d_inode);
+}
+
+static int hook_inode_listxattr(struct dentry *dentry)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+			dentry->d_inode);
+}
+
+static int hook_inode_removexattr(struct dentry *dentry, const char *name)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!dentry))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR,
+			dentry->d_inode);
+}
+
+static int hook_inode_getsecurity(struct inode *inode, const char *name,
+		void **buffer, bool alloc)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR, inode);
+}
+
+static int hook_inode_setsecurity(struct inode *inode, const char *name,
+		const void *value, size_t size, int flag)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR, inode);
+}
+
+static int hook_inode_listsecurity(struct inode *inode, char *buffer,
+		size_t buffer_size)
+{
+	if (!landlocked(current))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR, inode);
+}
+
+/* file hooks */
+
+static int hook_file_ioctl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!file))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_IOCTL,
+			file_inode(file));
+}
+
+static int hook_file_lock(struct file *file, unsigned int cmd)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!file))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_LOCK, file_inode(file));
+}
+
+static int hook_file_fcntl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!file))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_FCNTL,
+			file_inode(file));
+}
+
+static int hook_mmap_file(struct file *file, unsigned long reqprot,
+		unsigned long prot, unsigned long flags)
+{
+	if (!landlocked(current))
+		return 0;
+	/* file can be null for anonymous mmap */
+	if (!file)
+		return 0;
+	return decide_fs_pick(mem_prot_to_triggers(prot, flags & MAP_PRIVATE),
+			file_inode(file));
+}
+
+static int hook_file_mprotect(struct vm_area_struct *vma,
+		unsigned long reqprot, unsigned long prot)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!vma))
+		return 0;
+	if (!vma->vm_file)
+		return 0;
+	return decide_fs_pick(mem_prot_to_triggers(prot,
+				!(vma->vm_flags & VM_SHARED)),
+			file_inode(vma->vm_file));
+}
+
+static int hook_file_receive(struct file *file)
+{
+	if (!landlocked(current))
+		return 0;
+	if (WARN_ON(!file))
+		return 0;
+	return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RECEIVE,
+			file_inode(file));
+}
+
+static struct security_hook_list landlock_hooks[] = {
+	LSM_HOOK_INIT(binder_transfer_file, hook_binder_transfer_file),
+
+	LSM_HOOK_INIT(sb_statfs, hook_sb_statfs),
+	LSM_HOOK_INIT(sb_mount, hook_sb_mount),
+	LSM_HOOK_INIT(sb_pivotroot, hook_sb_pivotroot),
+
+	LSM_HOOK_INIT(inode_create, hook_inode_create),
+	LSM_HOOK_INIT(inode_link, hook_inode_link),
+	LSM_HOOK_INIT(inode_unlink, hook_inode_unlink),
+	LSM_HOOK_INIT(inode_symlink, hook_inode_symlink),
+	LSM_HOOK_INIT(inode_mkdir, hook_inode_mkdir),
+	LSM_HOOK_INIT(inode_rmdir, hook_inode_rmdir),
+	LSM_HOOK_INIT(inode_mknod, hook_inode_mknod),
+	LSM_HOOK_INIT(inode_rename, hook_inode_rename),
+	LSM_HOOK_INIT(inode_readlink, hook_inode_readlink),
+	LSM_HOOK_INIT(inode_permission, hook_inode_permission),
+	LSM_HOOK_INIT(inode_setattr, hook_inode_setattr),
+	LSM_HOOK_INIT(inode_getattr, hook_inode_getattr),
+	LSM_HOOK_INIT(inode_setxattr, hook_inode_setxattr),
+	LSM_HOOK_INIT(inode_getxattr, hook_inode_getxattr),
+	LSM_HOOK_INIT(inode_listxattr, hook_inode_listxattr),
+	LSM_HOOK_INIT(inode_removexattr, hook_inode_removexattr),
+	LSM_HOOK_INIT(inode_getsecurity, hook_inode_getsecurity),
+	LSM_HOOK_INIT(inode_setsecurity, hook_inode_setsecurity),
+	LSM_HOOK_INIT(inode_listsecurity, hook_inode_listsecurity),
+
+	/* do not handle file_permission for now */
+	LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
+	LSM_HOOK_INIT(file_lock, hook_file_lock),
+	LSM_HOOK_INIT(file_fcntl, hook_file_fcntl),
+	LSM_HOOK_INIT(mmap_file, hook_mmap_file),
+	LSM_HOOK_INIT(file_mprotect, hook_file_mprotect),
+	LSM_HOOK_INIT(file_receive, hook_file_receive),
+	/* file_open is not handled, use inode_permission instead */
+};
+
+__init void landlock_add_hooks_fs(void)
+{
+	security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+			LANDLOCK_NAME);
+}
diff --git a/security/landlock/hooks_fs.h b/security/landlock/hooks_fs.h
new file mode 100644
index 000000000000..eeae4dcd842f
--- /dev/null
+++ b/security/landlock/hooks_fs.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - filesystem hooks
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/bpf.h> /* enum bpf_access_type */
+
+__init void landlock_add_hooks_fs(void);
+
+/* fs_pick */
+
+struct landlock_hook_ctx_fs_pick;
+
+bool landlock_is_valid_access_fs_pick(int off, enum bpf_access_type type,
+		enum bpf_reg_type *reg_type, int *max_size);
+
+const struct landlock_ctx_fs_pick *landlock_get_ctx_fs_pick(
+		const struct landlock_hook_ctx_fs_pick *hook_ctx);
+
+/* fs_walk */
+
+struct landlock_hook_ctx_fs_walk;
+
+bool landlock_is_valid_access_fs_walk(int off, enum bpf_access_type type,
+		enum bpf_reg_type *reg_type, int *max_size);
+
+const struct landlock_ctx_fs_walk *landlock_get_ctx_fs_walk(
+		const struct landlock_hook_ctx_fs_walk *hook_ctx);
diff --git a/security/landlock/init.c b/security/landlock/init.c
index 03073cd0fc4e..68def2a7af71 100644
--- a/security/landlock/init.c
+++ b/security/landlock/init.c
@@ -9,8 +9,10 @@
 #include <linux/bpf.h> /* enum bpf_access_type */
 #include <linux/capability.h> /* capable */
 #include <linux/filter.h> /* struct bpf_prog */
+#include <linux/lsm_hooks.h>
 
 #include "common.h" /* LANDLOCK_* */
+#include "hooks_fs.h"
 
 static bool bpf_landlock_is_valid_access(int off, int size,
 		enum bpf_access_type type, const struct bpf_prog *prog,
@@ -29,6 +31,23 @@ static bool bpf_landlock_is_valid_access(int off, int size,
 	if (size <= 0 || size > sizeof(__u64))
 		return false;
 
+	/* set register type and max size */
+	switch (prog_subtype->landlock_hook.type) {
+	case LANDLOCK_HOOK_FS_PICK:
+		if (!landlock_is_valid_access_fs_pick(off, type, &reg_type,
+					&max_size))
+			return false;
+		break;
+	case LANDLOCK_HOOK_FS_WALK:
+		if (!landlock_is_valid_access_fs_walk(off, type, &reg_type,
+					&max_size))
+			return false;
+		break;
+	default:
+		WARN_ON(1);
+		return false;
+	}
+
 	/* check memory range access */
 	switch (reg_type) {
 	case NOT_INIT:
@@ -98,6 +117,18 @@ static const struct bpf_func_proto *bpf_landlock_func_proto(
 	default:
 		break;
 	}
+
+	switch (hook_type) {
+	case LANDLOCK_HOOK_FS_WALK:
+	case LANDLOCK_HOOK_FS_PICK:
+		switch (func_id) {
+		case BPF_FUNC_inode_map_lookup:
+			return &bpf_inode_map_lookup_proto;
+		default:
+			break;
+		}
+		break;
+	}
 	return NULL;
 }
 
@@ -108,3 +139,19 @@ const struct bpf_verifier_ops landlock_verifier_ops = {
 };
 
 const struct bpf_prog_ops landlock_prog_ops = {};
+
+static int __init landlock_init(void)
+{
+	pr_info(LANDLOCK_NAME ": Initializing (sandbox with seccomp)\n");
+	landlock_add_hooks_fs();
+	return 0;
+}
+
+struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {};
+
+DEFINE_LSM(LANDLOCK_NAME) = {
+	.name = LANDLOCK_NAME,
+	.order = LSM_ORDER_LAST,
+	.blobs = &landlock_blob_sizes,
+	.init = landlock_init,
+};
diff --git a/security/security.c b/security/security.c
index f493db0bf62a..05a23995407d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -263,6 +263,21 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
 		}
 	}
 
+	/*
+	 * In case of an unprivileged access-control, we don't want to give the
+	 * ability to any process to do some checks (e.g. through an eBPF
+	 * program) on kernel objects (e.g. files) if a privileged security
+	 * policy forbid their access.  We must then load
+	 * potentially-unprivileged security modules after all other LSMs.
+	 *
+	 * LSM_ORDER_LAST is always last and does not appear in the modifiable
+	 * ordered list of enabled LSMs.
+	 */
+	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+		if (lsm->order == LSM_ORDER_LAST)
+			append_ordered_lsm(lsm, "last");
+	}
+
 	/* Disable all LSMs not in the ordered list. */
 	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
 		if (exists_ordered_lsm(lsm))
-- 
2.20.1


^ permalink raw reply related

* [PATCH bpf-next v5 1/2] bpf: Allow bpf_skb_event_output for a few prog types
From: allanzhang @ 2019-06-25 22:23 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
  Cc: allanzhang
In-Reply-To: <20190625222329.209732-1-allanzhang@google.com>

Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.

Added socket_filter, cg_skb, sk_skb prog types to generate sw event.

Test bpf code is generated from code snippet:

struct TMP {
    uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
                      &tt, sizeof(tt));
return 1;

the bpf assembly from llvm is:
       0:       b7 02 00 00 05 00 00 00         r2 = 5
       1:       7b 2a f8 ff 00 00 00 00         *(u64 *)(r10 - 8) = r2
       2:       bf a4 00 00 00 00 00 00         r4 = r10
       3:       07 04 00 00 f8 ff ff ff         r4 += -8
       4:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00    r2 = 0ll
       6:       b7 03 00 00 00 00 00 00         r3 = 0
       7:       b7 05 00 00 08 00 00 00         r5 = 8
       8:       85 00 00 00 19 00 00 00         call 25
       9:       b7 00 00 00 01 00 00 00         r0 = 1
      10:       95 00 00 00 00 00 00 00         exit

Signed-off-by: allanzhang <allanzhang@google.com>
---
 net/core/filter.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..b75fcf412628 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5958,6 +5958,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_socket_cookie_proto;
 	case BPF_FUNC_get_socket_uid:
 		return &bpf_get_socket_uid_proto;
+	case BPF_FUNC_perf_event_output:
+		return &bpf_skb_event_output_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
@@ -5978,6 +5980,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_sk_storage_get_proto;
 	case BPF_FUNC_sk_storage_delete:
 		return &bpf_sk_storage_delete_proto;
+	case BPF_FUNC_perf_event_output:
+		return &bpf_skb_event_output_proto;
 #ifdef CONFIG_SOCK_CGROUP_DATA
 	case BPF_FUNC_skb_cgroup_id:
 		return &bpf_skb_cgroup_id_proto;
@@ -6226,6 +6230,8 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_sk_redirect_map_proto;
 	case BPF_FUNC_sk_redirect_hash:
 		return &bpf_sk_redirect_hash_proto;
+	case BPF_FUNC_perf_event_output:
+		return &bpf_skb_event_output_proto;
 #ifdef CONFIG_INET
 	case BPF_FUNC_sk_lookup_tcp:
 		return &bpf_sk_lookup_tcp_proto;
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH bpf-next v5 2/2] bpf: Add selftests for bpf_perf_event_output
From: allanzhang @ 2019-06-25 22:23 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
  Cc: allanzhang
In-Reply-To: <20190625222329.209732-1-allanzhang@google.com>

Software event output is only enabled by a few prog types.
This test is to ensure that all supported types are enabled for
bpf_perf_event_output successfully.

Signed-off-by: allanzhang <allanzhang@google.com>
---
 tools/testing/selftests/bpf/test_verifier.c   | 33 ++++++-
 .../selftests/bpf/verifier/event_output.c     | 94 +++++++++++++++++++
 2 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..901a188e1eea 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -50,7 +50,7 @@
 #define MAX_INSNS	BPF_MAXINSNS
 #define MAX_TEST_INSNS	1000000
 #define MAX_FIXUPS	8
-#define MAX_NR_MAPS	18
+#define MAX_NR_MAPS	19
 #define MAX_TEST_RUNS	8
 #define POINTER_VALUE	0xcafe4all
 #define TEST_DATA_LEN	64
@@ -84,6 +84,7 @@ struct bpf_test {
 	int fixup_map_array_wo[MAX_FIXUPS];
 	int fixup_map_array_small[MAX_FIXUPS];
 	int fixup_sk_storage_map[MAX_FIXUPS];
+	int fixup_map_event_output[MAX_FIXUPS];
 	const char *errstr;
 	const char *errstr_unpriv;
 	uint32_t retval, retval_unpriv, insn_processed;
@@ -604,6 +605,28 @@ static int create_sk_storage_map(void)
 	return fd;
 }
 
+static int create_event_output_map(void)
+{
+	struct bpf_create_map_attr attr = {
+		.name = "test_map",
+		.map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+		.key_size = 4,
+		.value_size = 4,
+		.max_entries = 1,
+	};
+	int fd, btf_fd;
+
+	btf_fd = load_btf();
+	if (btf_fd < 0)
+		return -1;
+	attr.btf_fd = btf_fd;
+	fd = bpf_create_map_xattr(&attr);
+	close(attr.btf_fd);
+	if (fd < 0)
+		printf("Failed to create event_output\n");
+	return fd;
+}
+
 static char bpf_vlog[UINT_MAX >> 8];
 
 static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
@@ -627,6 +650,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
 	int *fixup_map_array_wo = test->fixup_map_array_wo;
 	int *fixup_map_array_small = test->fixup_map_array_small;
 	int *fixup_sk_storage_map = test->fixup_sk_storage_map;
+	int *fixup_map_event_output = test->fixup_map_event_output;
 
 	if (test->fill_helper) {
 		test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -788,6 +812,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
 			fixup_sk_storage_map++;
 		} while (*fixup_sk_storage_map);
 	}
+	if (*fixup_map_event_output) {
+		map_fds[18] = create_event_output_map();
+		do {
+			prog[*fixup_map_event_output].imm = map_fds[18];
+			fixup_map_event_output++;
+		} while (*fixup_map_event_output);
+	}
 }
 
 static int set_admin(bool admin)
diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
new file mode 100644
index 000000000000..059cc70addf9
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/event_output.c
@@ -0,0 +1,94 @@
+/* instructions used to output a skb based software event, produced
+ * from code snippet:
+ * struct TMP {
+ *  uint64_t tmp;
+ * } tt;
+ * tt.tmp = 5;
+ * bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
+ * 			 &tt, sizeof(tt));
+ * return 1;
+ *
+ * the bpf assembly from llvm is:
+ *        0:       b7 02 00 00 05 00 00 00         r2 = 5
+ *        1:       7b 2a f8 ff 00 00 00 00         *(u64 *)(r10 - 8) = r2
+ *        2:       bf a4 00 00 00 00 00 00         r4 = r10
+ *        3:       07 04 00 00 f8 ff ff ff         r4 += -8
+ *        4:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00    r2 = 0ll
+ *        6:       b7 03 00 00 00 00 00 00         r3 = 0
+ *        7:       b7 05 00 00 08 00 00 00         r5 = 8
+ *        8:       85 00 00 00 19 00 00 00         call 25
+ *        9:       b7 00 00 00 01 00 00 00         r0 = 1
+ *       10:       95 00 00 00 00 00 00 00         exit
+ *
+ *     The reason I put the code here instead of fill_helpers is that map fixup
+ *     is against the insns, instead of filled prog.
+*/
+
+#define __PERF_EVENT_INSNS__					\
+	BPF_MOV64_IMM(BPF_REG_2, 5),				\
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8),		\
+	BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),			\
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8),			\
+	BPF_LD_MAP_FD(BPF_REG_2, 0),				\
+	BPF_MOV64_IMM(BPF_REG_3, 0),				\
+	BPF_MOV64_IMM(BPF_REG_5, 8),				\
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,		\
+		     BPF_FUNC_perf_event_output),		\
+	BPF_MOV64_IMM(BPF_REG_0, 1),				\
+	BPF_EXIT_INSN(),
+{
+	"perfevent for sockops",
+	.insns = { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_SOCK_OPS,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"perfevent for tc",
+	.insns =  { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"perfevent for lwt out",
+	.insns =  { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_LWT_OUT,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"perfevent for xdp",
+	.insns =  { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_XDP,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"perfevent for socket filter",
+	.insns =  { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"perfevent for sk_skb",
+	.insns =  { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_SK_SKB,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"perfevent for cgroup skb",
+	.insns =  { __PERF_EVENT_INSNS__ },
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	.fixup_map_event_output = { 4 },
+	.result = ACCEPT,
+	.retval = 1,
+},
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox