* kernel BUG at net/rxrpc/local_object.c:LINE!
From: syzbot @ 2019-06-28 2:47 UTC (permalink / raw)
To: davem, dhowells, linux-afs, linux-kernel, netdev, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 249155c2 Merge branch 'parisc-5.2-4' of git://git.kernel.o..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14fabe45a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=e7c31a94f66cc0aa
dashboard link: https://syzkaller.appspot.com/bug?extid=1e0edc4b8b7494c28450
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13e2738da00000
The bug was bisected to:
commit 46894a13599a977ac35411b536fb3e0b2feefa95
Author: David Howells <dhowells@redhat.com>
Date: Thu Oct 4 08:32:28 2018 +0000
rxrpc: Use IPv4 addresses throught the IPv6
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=152fabe3a00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=172fabe3a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=132fabe3a00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1e0edc4b8b7494c28450@syzkaller.appspotmail.com
Fixes: 46894a13599a ("rxrpc: Use IPv4 addresses throught the IPv6")
rxrpc: Assertion failed
------------[ cut here ]------------
kernel BUG at net/rxrpc/local_object.c:468!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 16 Comm: ksoftirqd/1 Not tainted 5.2.0-rc6+ #60
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:rxrpc_local_rcu net/rxrpc/local_object.c:468 [inline]
RIP: 0010:rxrpc_local_rcu.cold+0x11/0x13 net/rxrpc/local_object.c:462
Code: 83 eb 20 e9 74 ff ff ff e8 58 aa 2d fb eb cc 4c 89 ef e8 6e aa 2d fb
eb e2 e8 d7 fd f4 fa 48 c7 c7 20 8c 15 88 e8 6f 03 df fa <0f> 0b e8 c4 fd
f4 fa 48 c7 c7 20 8c 15 88 e8 5c 03 df fa 0f 0b e8
RSP: 0018:ffff8880a9917c98 EFLAGS: 00010286
RAX: 0000000000000017 RBX: 0000000000000001 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815ad926 RDI: ffffed1015322f85
RBP: ffff8880a9917ca8 R08: 0000000000000017 R09: ffffed1015d260a1
R10: ffffed1015d260a0 R11: ffff8880ae930507 R12: ffff888099033b40
R13: ffff888099033b40 R14: ffffffff867b8f10 R15: ffff8880a9917d28
FS: 0000000000000000(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f380ebda000 CR3: 000000007ad50000 CR4: 00000000001406e0
Call Trace:
__rcu_reclaim kernel/rcu/rcu.h:222 [inline]
rcu_do_batch kernel/rcu/tree.c:2092 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2310 [inline]
rcu_core+0xba5/0x1500 kernel/rcu/tree.c:2291
__do_softirq+0x25c/0x94c kernel/softirq.c:292
run_ksoftirqd kernel/softirq.c:603 [inline]
run_ksoftirqd+0x8e/0x110 kernel/softirq.c:595
smpboot_thread_fn+0x6a3/0xa30 kernel/smpboot.c:165
kthread+0x354/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace 0e784d6285151217 ]---
RIP: 0010:rxrpc_local_rcu net/rxrpc/local_object.c:468 [inline]
RIP: 0010:rxrpc_local_rcu.cold+0x11/0x13 net/rxrpc/local_object.c:462
Code: 83 eb 20 e9 74 ff ff ff e8 58 aa 2d fb eb cc 4c 89 ef e8 6e aa 2d fb
eb e2 e8 d7 fd f4 fa 48 c7 c7 20 8c 15 88 e8 6f 03 df fa <0f> 0b e8 c4 fd
f4 fa 48 c7 c7 20 8c 15 88 e8 5c 03 df fa 0f 0b e8
RSP: 0018:ffff8880a9917c98 EFLAGS: 00010286
RAX: 0000000000000017 RBX: 0000000000000001 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815ad926 RDI: ffffed1015322f85
RBP: ffff8880a9917ca8 R08: 0000000000000017 R09: ffffed1015d260a1
R10: ffffed1015d260a0 R11: ffff8880ae930507 R12: ffff888099033b40
R13: ffff888099033b40 R14: ffffffff867b8f10 R15: ffff8880a9917d28
FS: 0000000000000000(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f380ebda000 CR3: 000000007ad50000 CR4: 00000000001406e0
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH v2 03/27] atm: idt77252: remove memset after dma_alloc_coherent
From: Fuqian Huang @ 2019-06-28 2:46 UTC (permalink / raw)
Cc: Fuqian Huang, Chas Williams, linux-atm-general, netdev,
linux-kernel
In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
drivers/atm/idt77252.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index 43a14579e80e..df51680e8931 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -1379,7 +1379,6 @@ init_tsq(struct idt77252_dev *card)
printk("%s: can't allocate TSQ.\n", card->name);
return -1;
}
- memset(card->tsq.base, 0, TSQSIZE);
card->tsq.last = card->tsq.base + TSQ_NUM_ENTRIES - 1;
card->tsq.next = card->tsq.last;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 2/5] net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with regmap
From: Florian Fainelli @ 2019-06-28 2:44 UTC (permalink / raw)
To: Marek Vasut, netdev; +Cc: Andrew Lunn, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-3-marex@denx.de>
On 6/27/2019 2:55 PM, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> Cc: Woojung Huh <Woojung.Huh@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/5] net: dsa: microchip: Replace ad-hoc polling with regmap
From: Florian Fainelli @ 2019-06-28 2:43 UTC (permalink / raw)
To: Marek Vasut, netdev; +Cc: Andrew Lunn, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-2-marex@denx.de>
On 6/27/2019 2:55 PM, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register,
> use in instead of reimplementing it.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> Cc: Woojung Huh <Woojung.Huh@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH net-next] net/ipv6: Fix misuse of proc_dointvec "flowlabel_reflect"
From: Eiichi Tsukata @ 2019-06-28 2:37 UTC (permalink / raw)
To: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel; +Cc: Eiichi Tsukata
/proc/sys/net/ipv6/flowlabel_reflect assumes written value to be in the
range of 0 to 3. Use proc_dointvec_minmax instead of proc_dointvec.
Fixes: 323a53c41292 ("ipv6: tcp: enable flowlabel reflection in some RST packets")
Signed-off-by: Eiichi Tsukata <devel@etsukata.com>
---
net/ipv6/sysctl_net_ipv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 6d86fac472e7..831573461e19 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -114,7 +114,7 @@ static struct ctl_table ipv6_table_template[] = {
.data = &init_net.ipv6.sysctl.flowlabel_reflect,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &three,
},
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 4/6 bfp-next] Simplify AF_XDP umem allocation path for Intel drivers.
From: Jonathan Lemon @ 2019-06-28 2:36 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer,
kernel-team
In-Reply-To: <20190627154206.5d458e94@cakuba.netronome.com>
On 27 Jun 2019, at 15:42, Jakub Kicinski wrote:
> On Thu, 27 Jun 2019 15:08:34 -0700, Jonathan Lemon wrote:
>> Now that the recycle stack is always used for the driver umem path, the
>> driver code path can be simplified.
>>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>
> I guess it's a question to Bjorn and Magnus whether they want Intel
> drivers to always go through the reuse queue..
I did pass this by them earlier.
> Could you be more explicit on the motivation? I'd call this patch set
> "make all drivers use reuse queue" rather than "clean up".
The motivation is to have packets which were received on a zero-copy
AF_XDP socket, and which returned a TX verdict from the bpf program,
queued directly on the TX ring (if they're in the same napi context).
When these TX packets are completed, they are placed back onto the
reuse queue, as there isn't really any other place to handle them.
It also addresses Maxim's concern about having buffers end up sitting
on the rq after a ring resize.
I was going to send the TX change out as part of this patch, but
figured it would be better split unto its own series.
> Also when you're changing code please make sure you CC the author.
Who did I miss?
--
Jonathan
^ permalink raw reply
* Re: [PATCH 2/6 bpf-next] Clean up xsk reuseq API
From: Jonathan Lemon @ 2019-06-28 2:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer,
kernel-team
In-Reply-To: <20190627153856.1f4d4709@cakuba.netronome.com>
On 27 Jun 2019, at 15:38, Jakub Kicinski wrote:
> On Thu, 27 Jun 2019 15:08:32 -0700, Jonathan Lemon wrote:
>> The reuseq is actually a recycle stack, only accessed from the kernel
>> side.
>> Also, the implementation details of the stack should belong to the
>> umem
>> object, and not exposed to the caller.
>>
>> Clean up and rename for consistency in preparation for the next
>> patch.
>>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>
> Prepare/swap is to cater to how drivers should be written - being able
> to allocate resources independently of those currently used. Allowing
> for changing ring sizes and counts on the fly. This patch makes it
> harder to write drivers in the way we are encouraging people to.
>
> IOW no, please don't do this.
The main reason I rewrote this was to provide the same type
of functionality as realloc() - no need to allocate/initialize a new
array if the old one would still end up being used. This would seem
to be a win for the typical case of having the interface go up/down.
Perhaps I should have named the function differently?
--
Jonathan
^ permalink raw reply
* RE: [EXT] [PATCH 73/87] ethernet: freescale: Remove memset after dma_alloc_coherent
From: Andy Duan @ 2019-06-28 2:26 UTC (permalink / raw)
To: Fuqian Huang
Cc: David S. Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190627174641.6474-1-huangfq.daxian@gmail.com>
From: Fuqian Huang <huangfq.daxian@gmail.com> Sent: Friday, June 28, 2019 1:47 AM
> In commit af7ddd8a627c
> ("Merge tag 'dma-mapping-4.21' of
> git://git.infradead.org/users/hch/dma-mapping"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed.
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
Acked-by: Fugang Duan <fugang.duan@nxp.com>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 38f10f7dcbc3..ec87b8b78d21 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3143,8 +3143,6 @@ static int fec_enet_init(struct net_device *ndev)
> return -ENOMEM;
> }
>
> - memset(cbd_base, 0, bd_size);
> -
> /* Get the Ethernet address */
> fec_get_mac(ndev);
> /* make sure MAC we just acquired is programmed into the hw */
> --
> 2.11.0
^ permalink raw reply
* Re: [PATCH 0/5] net: dsa: microchip: Further regmap cleanups
From: David Miller @ 2019-06-28 2:25 UTC (permalink / raw)
To: marex; +Cc: netdev, andrew, f.fainelli, Tristram.Ha, Woojung.Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>
From: Marek Vasut <marex@denx.de>
Date: Thu, 27 Jun 2019 23:55:51 +0200
> This patchset cleans up KSZ9477 switch driver by replacing various
> ad-hoc polling implementations and register RMW with regmap functions.
>
> Each polling function is replaced separately to make it easier to review
> and possibly bisect, but maybe the patches can be squashed.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Series applied, thanks.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2019-06-28 1:45 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Dmitry Bogdanov, Igor Russkikh
[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got conflicts in:
drivers/net/ethernet/aquantia/atlantic/aq_nic.c
drivers/net/ethernet/aquantia/atlantic/aq_nic.h
between commit:
48dd73d08d4d ("net: aquantia: fix vlans not working over bridged network")
from the net tree and commit:
d3ed7c5cf79b ("net: aquantia: adding fields and device features for vlan offload")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 41172fbebddd,746f85e6de13..000000000000
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@@ -126,7 -126,8 +126,9 @@@ void aq_nic_cfg_start(struct aq_nic_s *
cfg->link_speed_msk &= cfg->aq_hw_caps->link_speed_msk;
cfg->features = cfg->aq_hw_caps->hw_features;
+ cfg->is_vlan_force_promisc = true;
+ cfg->is_vlan_rx_strip = !!(cfg->features & NETIF_F_HW_VLAN_CTAG_RX);
+ cfg->is_vlan_tx_insert = !!(cfg->features & NETIF_F_HW_VLAN_CTAG_TX);
}
static int aq_nic_update_link_status(struct aq_nic_s *self)
diff --cc drivers/net/ethernet/aquantia/atlantic/aq_nic.h
index 0f22f5d5691b,26c72f298684..000000000000
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
@@@ -35,7 -35,8 +35,9 @@@ struct aq_nic_cfg_s
u32 flow_control;
u32 link_speed_msk;
u32 wol;
+ bool is_vlan_force_promisc;
+ u8 is_vlan_rx_strip;
+ u8 is_vlan_tx_insert;
u16 is_mc_list_enabled;
u16 mc_list_count;
bool is_autoneg;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] net/sched: flower: fix infinite loop in fl_walk()
From: Cong Wang @ 2019-06-28 1:24 UTC (permalink / raw)
To: Davide Caratti
Cc: Vlad Buslov, David S. Miller, Linux Kernel Network Developers,
Lucas Bates
In-Reply-To: <94260c8898834cfa8e5421933a2b5ea59680b970.camel@redhat.com>
On Thu, Jun 27, 2019 at 3:10 PM Davide Caratti <dcaratti@redhat.com> wrote:
>
> On Wed, 2019-06-26 at 14:15 -0700, Cong Wang wrote:
> > Hi, Davide
> >
> > On Tue, Jun 25, 2019 at 12:29 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > It should handle this overflow case more gracefully, I hope.
> > >
> >
> > Please try this attached one and let me know if it works.
> > Hope I get it right this time.
> >
> > Thanks!
>
> hello Cong, and thanks a lot for the patch!
> I see it uses
>
> (tmp <= id)
>
> as the condition to detect the overflow, and at each iteration it does
>
> tmp = id, ++id
>
> so that 'tmp' contains the last IDR found in the tree and 'id' is the next
> tentative value to be searched for. When 'id' overflows, (tmp <= id)
> becomes false, and the 'for' loop exits.
Yes, thanks for testing it with tc actions.
> I tested it successfully with TC actions having the highest possible
> index: 'tc actions show' doesn't loop anymore. But with cls_flower (that
> uses idr_for_each_entry_continue_ul() ) I still see the infinite loop:
> even when idr_for_each_entry_continue_ul() is used, fl_get_next_filter()
> never returns NULL, because
>
> (tmp <= id) && (((entry) = idr_get_next_ul(idr, &(id))) != NULL)
>
> calls idr_get_next_ul(idr, &(id)) at least once. So, even if
> idr_for_each_entry_continue_ul() detected the overflow of 'id' after the
> first iteration, and bailouts the for loop, fl_get_next_filter()
> repeatedly returns a pointer to the idr slot with index equal to
> 0xffffffff. Because of that, the while() loop in fl_walk() keeps dumping
> the same rule.
Good catch, it is actually the arg->cookie++ which causes the trouble
here.
> In my original patch I found easier to check for the overflow of
> arg->cookie in fl_walk(), before the self-increment, so I was sure that
>
> arg->fn(tp, f, arg)
>
> was already called once when 'f' was the slot having the highest possible
> IDR. Now, I didn't check it, but I guess
>
> refcount_inc_not_zero(&f->refcnt))
>
> in fl_get_next_filter() is always true during my test, so the inner
> while() loop is not endless, even when the idr has a slot with id equal to
> ULONG_MAX. Probably, to stay on the safe side, cls_flower needs both tests
> to be in place, what do you think?
I think we can just fold the nested loops into one for cls_flower and remove
the arg->cookie++.
What's more, arg->cookie could overflow too,seems we have to switch back to
arg->skip. I am not sure, if this is really a problem we can fix it separately.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 02/13] dt-bindings: net: Add a YAML schemas for the generic PHY options
From: Rob Herring @ 2019-06-28 1:16 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mark Rutland, Frank Rowand, David S . Miller, Chen-Yu Tsai,
Maxime Coquelin, Alexandre Torgue, netdev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
devicetree, linux-stm32, Maxime Chevallier, Antoine Ténart,
Andrew Lunn, Florian Fainelli, Heiner Kallweit
In-Reply-To: <945e54f02cc36a543b4c0bfd960475147359f7ff.1561649505.git-series.maxime.ripard@bootlin.com>
On Thu, Jun 27, 2019 at 9:32 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> The networking PHYs have a number of available device tree properties that
> can be used in their device tree node. Add a YAML schemas for those.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
> Documentation/devicetree/bindings/net/ethernet-phy.yaml | 179 +++++++++-
> Documentation/devicetree/bindings/net/phy.txt | 80 +----
> 2 files changed, 180 insertions(+), 79 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> new file mode 100644
> index 000000000000..81d2016d7232
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -0,0 +1,179 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ethernet-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ethernet PHY Generic Binding
> +
> +maintainers:
> + - Andrew Lunn <andrew@lunn.ch>
> + - Florian Fainelli <f.fainelli@gmail.com>
> + - Heiner Kallweit <hkallweit1@gmail.com>
> +
> +# The dt-schema tools will generate a select statement first by using
> +# the compatible, and second by using the node name if any. In our
> +# case, the node name is the one we want to match on, while the
> +# compatible is optional.
> +select:
> + properties:
> + $nodename:
> + pattern: "^ethernet-phy(@[a-f0-9]+)?$"
> +
> + required:
> + - $nodename
> +
> +properties:
> + $nodename:
> + pattern: "^ethernet-phy(@[a-f0-9]+)?$"
> +
> + compatible:
> + oneOf:
> + - const: ethernet-phy-ieee802.3-c22
> + description: PHYs that implement IEEE802.3 clause 22
> + - const: ethernet-phy-ieee802.3-c45
> + description: PHYs that implement IEEE802.3 clause 45
> + - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
> + description:
> + If the PHY reports an incorrect ID (or none at all) then the
> + compatible list may contain an entry with the correct PHY ID
> + in the above form.
> + The first group of digits is the 16 bit Phy Identifier 1
> + register, this is the chip vendor OUI bits 3:18. The
> + second group of digits is the Phy Identifier 2 register,
> + this is the chip vendor OUI bits 19:24, followed by 10
> + bits of a vendor specific ID.
> + - items:
> + - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
> + - const: ethernet-phy-ieee802.3-c45
> +
> + reg:
> + maxItems: 1
> + minimum: 0
> + maximum: 31
> + description:
> + The ID number for the PHY.
Mixing array and scalar properties is something we shouldn't be doing,
so I dropped maxItems as that is implied. I have a meta-schema check
for this once a couple of occurrences are fixed in the tree.
Rob
^ permalink raw reply
* [PATCH v3 net-next] net: link_watch: prevent starvation when processing linkwatch wq
From: Yunsheng Lin @ 2019-06-28 1:13 UTC (permalink / raw)
To: davem
Cc: hkallweit1, gregkh, tglx, netdev, linux-kernel, pbonzini, rkrcmar,
kvm
When user has configured a large number of virtual netdev, such
as 4K vlans, the carrier on/off operation of the real netdev
will also cause it's virtual netdev's link state to be processed
in linkwatch. Currently, the processing is done in a work queue,
which may cause rtnl locking starvation problem and worker
starvation problem for other work queue, such as irqfd_inject wq.
This patch releases the cpu when link watch worker has processed
a fixed number of netdev' link watch event, and schedule the
work queue again when there is still link watch event remaining.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
V2: use cond_resched and rtnl_unlock after processing a fixed
number of events
V3: fall back to v1 and change commit log to reflect that.
---
---
net/core/link_watch.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 04fdc95..f153e06 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -163,9 +163,16 @@ static void linkwatch_do_dev(struct net_device *dev)
static void __linkwatch_run_queue(int urgent_only)
{
+#define MAX_DO_DEV_PER_LOOP 100
+
+ int do_dev = MAX_DO_DEV_PER_LOOP;
struct net_device *dev;
LIST_HEAD(wrk);
+ /* Give urgent case more budget */
+ if (urgent_only)
+ do_dev += MAX_DO_DEV_PER_LOOP;
+
/*
* Limit the number of linkwatch events to one
* per second so that a runaway driver does not
@@ -184,7 +191,7 @@ static void __linkwatch_run_queue(int urgent_only)
spin_lock_irq(&lweventlist_lock);
list_splice_init(&lweventlist, &wrk);
- while (!list_empty(&wrk)) {
+ while (!list_empty(&wrk) && do_dev > 0) {
dev = list_first_entry(&wrk, struct net_device, link_watch_list);
list_del_init(&dev->link_watch_list);
@@ -195,9 +202,13 @@ static void __linkwatch_run_queue(int urgent_only)
}
spin_unlock_irq(&lweventlist_lock);
linkwatch_do_dev(dev);
+ do_dev--;
spin_lock_irq(&lweventlist_lock);
}
+ /* Add the remaining work back to lweventlist */
+ list_splice_init(&wrk, &lweventlist);
+
if (!list_empty(&lweventlist))
linkwatch_schedule_work(0);
spin_unlock_irq(&lweventlist_lock);
--
2.8.1
^ permalink raw reply related
* [PATCH bpf-next] selftests/bpf: fix -Wstrict-aliasing in test_sockopt_sk.c
From: Stanislav Fomichev @ 2019-06-28 1:12 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Let's use union with u8[4] and u32 members for sockopt buffer,
that should fix any possible aliasing issues.
test_sockopt_sk.c: In function ‘getsetsockopt’:
test_sockopt_sk.c:115:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
if (*(__u32 *)buf != 0x55AA*2) {
^~
test_sockopt_sk.c:116:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
log_err("Unexpected getsockopt(SO_SNDBUF) 0x%x != 0x55AA*2",
^~~~~~~
Fixes: 8a027dc0d8f5 ("selftests/bpf: add sockopt test that exercises sk helpers")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_sockopt_sk.c | 51 +++++++++----------
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sockopt_sk.c b/tools/testing/selftests/bpf/test_sockopt_sk.c
index 12e79ed075ce..036b652e5ca9 100644
--- a/tools/testing/selftests/bpf/test_sockopt_sk.c
+++ b/tools/testing/selftests/bpf/test_sockopt_sk.c
@@ -22,7 +22,10 @@
static int getsetsockopt(void)
{
int fd, err;
- char buf[4] = {};
+ union {
+ char u8[4];
+ __u32 u32;
+ } buf = {};
socklen_t optlen;
fd = socket(AF_INET, SOCK_STREAM, 0);
@@ -33,31 +36,31 @@ static int getsetsockopt(void)
/* IP_TOS - BPF bypass */
- buf[0] = 0x08;
- err = setsockopt(fd, SOL_IP, IP_TOS, buf, 1);
+ buf.u8[0] = 0x08;
+ err = setsockopt(fd, SOL_IP, IP_TOS, &buf, 1);
if (err) {
log_err("Failed to call setsockopt(IP_TOS)");
goto err;
}
- buf[0] = 0x00;
+ buf.u8[0] = 0x00;
optlen = 1;
- err = getsockopt(fd, SOL_IP, IP_TOS, buf, &optlen);
+ err = getsockopt(fd, SOL_IP, IP_TOS, &buf, &optlen);
if (err) {
log_err("Failed to call getsockopt(IP_TOS)");
goto err;
}
- if (buf[0] != 0x08) {
+ if (buf.u8[0] != 0x08) {
log_err("Unexpected getsockopt(IP_TOS) buf[0] 0x%02x != 0x08",
- buf[0]);
+ buf.u8[0]);
goto err;
}
/* IP_TTL - EPERM */
- buf[0] = 1;
- err = setsockopt(fd, SOL_IP, IP_TTL, buf, 1);
+ buf.u8[0] = 1;
+ err = setsockopt(fd, SOL_IP, IP_TTL, &buf, 1);
if (!err || errno != EPERM) {
log_err("Unexpected success from setsockopt(IP_TTL)");
goto err;
@@ -65,16 +68,16 @@ static int getsetsockopt(void)
/* SOL_CUSTOM - handled by BPF */
- buf[0] = 0x01;
- err = setsockopt(fd, SOL_CUSTOM, 0, buf, 1);
+ buf.u8[0] = 0x01;
+ err = setsockopt(fd, SOL_CUSTOM, 0, &buf, 1);
if (err) {
log_err("Failed to call setsockopt");
goto err;
}
- buf[0] = 0x00;
+ buf.u32 = 0x00;
optlen = 4;
- err = getsockopt(fd, SOL_CUSTOM, 0, buf, &optlen);
+ err = getsockopt(fd, SOL_CUSTOM, 0, &buf, &optlen);
if (err) {
log_err("Failed to call getsockopt");
goto err;
@@ -84,37 +87,31 @@ static int getsetsockopt(void)
log_err("Unexpected optlen %d != 1", optlen);
goto err;
}
- if (buf[0] != 0x01) {
- log_err("Unexpected buf[0] 0x%02x != 0x01", buf[0]);
+ if (buf.u8[0] != 0x01) {
+ log_err("Unexpected buf[0] 0x%02x != 0x01", buf.u8[0]);
goto err;
}
/* SO_SNDBUF is overwritten */
- buf[0] = 0x01;
- buf[1] = 0x01;
- buf[2] = 0x01;
- buf[3] = 0x01;
- err = setsockopt(fd, SOL_SOCKET, SO_SNDBUF, buf, 4);
+ buf.u32 = 0x01010101;
+ err = setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &buf, 4);
if (err) {
log_err("Failed to call setsockopt(SO_SNDBUF)");
goto err;
}
- buf[0] = 0x00;
- buf[1] = 0x00;
- buf[2] = 0x00;
- buf[3] = 0x00;
+ buf.u32 = 0x00;
optlen = 4;
- err = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, buf, &optlen);
+ err = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &buf, &optlen);
if (err) {
log_err("Failed to call getsockopt(SO_SNDBUF)");
goto err;
}
- if (*(__u32 *)buf != 0x55AA*2) {
+ if (buf.u32 != 0x55AA*2) {
log_err("Unexpected getsockopt(SO_SNDBUF) 0x%x != 0x55AA*2",
- *(__u32 *)buf);
+ buf.u32);
goto err;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH 2/2 nf-next v3] netfilter:nft_meta: Add NFT_META_VLAN support
From: wenxu @ 2019-06-28 0:49 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561682975-21790-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch provide a meta vlan to set the vlan tag of the packet.
for q-in-q outer vlan id 20:
meta vlan set 0x88a8:20
set the default 0x8100 vlan type with vlan id 20
meta vlan set 20
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 4 ++++
net/netfilter/nft_meta.c | 27 ++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 0b18646..cf037f2 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -797,6 +797,7 @@ enum nft_exthdr_attributes {
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
* @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
+ * @NFT_META_VLAN: packet vlan metadata
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -829,6 +830,7 @@ enum nft_meta_keys {
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
NFT_META_BRI_IIFVPROTO,
+ NFT_META_VLAN,
};
/**
@@ -895,12 +897,14 @@ enum nft_hash_attributes {
* @NFTA_META_DREG: destination register (NLA_U32)
* @NFTA_META_KEY: meta data item to load (NLA_U32: nft_meta_keys)
* @NFTA_META_SREG: source register (NLA_U32)
+ * @NFTA_META_SREG2: source register (NLA_U32)
*/
enum nft_meta_attributes {
NFTA_META_UNSPEC,
NFTA_META_DREG,
NFTA_META_KEY,
NFTA_META_SREG,
+ NFTA_META_SREG2,
__NFTA_META_MAX
};
#define NFTA_META_MAX (__NFTA_META_MAX - 1)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index e3adf6a..29a6679 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -28,7 +28,10 @@ struct nft_meta {
enum nft_meta_keys key:8;
union {
enum nft_registers dreg:8;
- enum nft_registers sreg:8;
+ struct {
+ enum nft_registers sreg:8;
+ enum nft_registers sreg2:8;
+ };
};
};
@@ -312,6 +315,17 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
skb->secmark = value;
break;
#endif
+ case NFT_META_VLAN: {
+ u32 *sreg2 = ®s->data[meta->sreg2];
+ __be16 vlan_proto;
+ u16 vlan_tci;
+
+ vlan_tci = nft_reg_load16(sreg);
+ vlan_proto = nft_reg_load16(sreg2);
+
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ break;
+ }
default:
WARN_ON(1);
}
@@ -321,6 +335,7 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
[NFTA_META_DREG] = { .type = NLA_U32 },
[NFTA_META_KEY] = { .type = NLA_U32 },
[NFTA_META_SREG] = { .type = NLA_U32 },
+ [NFTA_META_SREG2] = { .type = NLA_U32 },
};
static int nft_meta_get_init(const struct nft_ctx *ctx,
@@ -483,6 +498,13 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
case NFT_META_PKTTYPE:
len = sizeof(u8);
break;
+ case NFT_META_VLAN:
+ len = sizeof(u16);
+ priv->sreg2 = nft_parse_register(tb[NFTA_META_SREG2]);
+ err = nft_validate_register_load(priv->sreg2, len);
+ if (err < 0)
+ return err;
+ break;
default:
return -EOPNOTSUPP;
}
@@ -521,6 +543,9 @@ static int nft_meta_set_dump(struct sk_buff *skb, const struct nft_expr *expr)
goto nla_put_failure;
if (nft_dump_register(skb, NFTA_META_SREG, priv->sreg))
goto nla_put_failure;
+ if (priv->key == NFT_META_VLAN &&
+ nft_dump_register(skb, NFTA_META_SREG2, priv->sreg2))
+ goto nla_put_failure;
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: wenxu @ 2019-06-28 0:49 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
This patch provide a meta to get the bridge vlan proto
nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/netfilter/nft_meta.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 8859535..0b18646 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -796,6 +796,7 @@ enum nft_exthdr_attributes {
* @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
+ * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -827,6 +828,7 @@ enum nft_meta_keys {
NFT_META_IIFKIND,
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
+ NFT_META_BRI_IIFVPROTO,
};
/**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 4f8116d..e3adf6a 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -248,6 +248,14 @@ void nft_meta_get_eval(const struct nft_expr *expr,
return;
}
goto err;
+ case NFT_META_BRI_IIFVPROTO:
+ if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL)
+ goto err;
+ if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) {
+ nft_reg_store16(dest, p->br->vlan_proto);
+ return;
+ }
+ goto err;
#endif
case NFT_META_IIFKIND:
if (in == NULL || in->rtnl_link_ops == NULL)
@@ -376,6 +384,7 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
len = IFNAMSIZ;
break;
case NFT_META_BRI_PVID:
+ case NFT_META_BRI_IIFVPROTO:
if (ctx->family != NFPROTO_BRIDGE)
return -EOPNOTSUPP;
len = sizeof(u16);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 5/5] net: dsa: microchip: Replace bit RMW with regmap
From: Andrew Lunn @ 2019-06-28 0:42 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-6-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:56PM +0200, Marek Vasut wrote:
> Regmap provides read-modify-write function to update bitfields in
> registers. Replace ad-hoc read-modify-write with regmap_update_bits()
> where applicable.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 4/5] net: dsa: microchip: Replace ksz9477_wait_alu_sta_ready polling with regmap
From: Andrew Lunn @ 2019-06-28 0:41 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-5-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:55PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 3/5] net: dsa: microchip: Replace ksz9477_wait_alu_ready polling with regmap
From: Andrew Lunn @ 2019-06-28 0:40 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-4-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:54PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 2/5] net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with regmap
From: Andrew Lunn @ 2019-06-28 0:39 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-3-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:53PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 1/5] net: dsa: microchip: Replace ad-hoc polling with regmap
From: Andrew Lunn @ 2019-06-28 0:38 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-2-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:52PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register,
> use in instead of reimplementing it.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next v3 1/2] xsk: remove AF_XDP socket from map when the socket is released
From: Daniel Borkmann @ 2019-06-28 0:33 UTC (permalink / raw)
To: Björn Töpel, ast, netdev
Cc: Björn Töpel, magnus.karlsson, bruce.richardson,
songliubraving, bpf
In-Reply-To: <20190620100652.31283-2-bjorn.topel@gmail.com>
On 06/20/2019 12:06 PM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> When an AF_XDP socket is released/closed the XSKMAP still holds a
> reference to the socket in a "released" state. The socket will still
> use the netdev queue resource, and block newly created sockets from
> attaching to that queue, but no user application can access the
> fill/complete/rx/tx queues. This results in that all applications need
> to explicitly clear the map entry from the old "zombie state"
> socket. This should be done automatically.
>
> After this patch, when a socket is released, it will remove itself
> from all the XSKMAPs it resides in, allowing the socket application to
> remove the code that cleans the XSKMAP entry.
>
> This behavior is also closer to that of SOCKMAP, making the two socket
> maps more consistent.
>
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Sorry for the bit of delay in reviewing, few comments inline:
> ---
> include/net/xdp_sock.h | 3 ++
> kernel/bpf/xskmap.c | 101 +++++++++++++++++++++++++++++++++++------
> net/xdp/xsk.c | 25 ++++++++++
> 3 files changed, 116 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index ae0f368a62bb..011a1b08d7c9 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -68,6 +68,8 @@ struct xdp_sock {
> */
> spinlock_t tx_completion_lock;
> u64 rx_dropped;
> + struct list_head map_list;
> + spinlock_t map_list_lock;
> };
>
> struct xdp_buff;
> @@ -87,6 +89,7 @@ struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
> struct xdp_umem_fq_reuse *newq);
> void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
> struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev, u16 queue_id);
> +void xsk_map_delete_from_node(struct xdp_sock *xs, struct list_head *node);
>
> static inline char *xdp_umem_get_data(struct xdp_umem *umem, u64 addr)
> {
> diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
> index ef7338cebd18..af802c89ebab 100644
> --- a/kernel/bpf/xskmap.c
> +++ b/kernel/bpf/xskmap.c
> @@ -13,8 +13,58 @@ struct xsk_map {
> struct bpf_map map;
> struct xdp_sock **xsk_map;
> struct list_head __percpu *flush_list;
> + spinlock_t lock;
> };
>
> +/* Nodes are linked in the struct xdp_sock map_list field, and used to
> + * track which maps a certain socket reside in.
> + */
> +struct xsk_map_node {
> + struct list_head node;
> + struct xsk_map *map;
> + struct xdp_sock **map_entry;
> +};
> +
> +static struct xsk_map_node *xsk_map_node_alloc(void)
> +{
> + return kzalloc(sizeof(struct xsk_map_node), GFP_ATOMIC | __GFP_NOWARN);
> +}
> +
> +static void xsk_map_node_free(struct xsk_map_node *node)
> +{
> + kfree(node);
> +}
> +
> +static void xsk_map_node_init(struct xsk_map_node *node,
> + struct xsk_map *map,
> + struct xdp_sock **map_entry)
> +{
> + node->map = map;
> + node->map_entry = map_entry;
> +}
> +
> +static void xsk_map_add_node(struct xdp_sock *xs, struct xsk_map_node *node)
> +{
> + spin_lock_bh(&xs->map_list_lock);
> + list_add_tail(&node->node, &xs->map_list);
> + spin_unlock_bh(&xs->map_list_lock);
> +}
> +
> +static void xsk_map_del_node(struct xdp_sock *xs, struct xdp_sock **map_entry)
> +{
> + struct xsk_map_node *n, *tmp;
> +
> + spin_lock_bh(&xs->map_list_lock);
> + list_for_each_entry_safe(n, tmp, &xs->map_list, node) {
> + if (map_entry == n->map_entry) {
> + list_del(&n->node);
> + xsk_map_node_free(n);
> + }
> + }
> + spin_unlock_bh(&xs->map_list_lock);
> +
> +}
> +
> static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
> {
> struct xsk_map *m;
> @@ -34,6 +84,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
> return ERR_PTR(-ENOMEM);
>
> bpf_map_init_from_attr(&m->map, attr);
> + spin_lock_init(&m->lock);
>
> cost = (u64)m->map.max_entries * sizeof(struct xdp_sock *);
> cost += sizeof(struct list_head) * num_possible_cpus();
> @@ -76,15 +127,16 @@ static void xsk_map_free(struct bpf_map *map)
> bpf_clear_redirect_map(map);
> synchronize_net();
>
> + spin_lock_bh(&m->lock);
> for (i = 0; i < map->max_entries; i++) {
> - struct xdp_sock *xs;
> -
> - xs = m->xsk_map[i];
> - if (!xs)
> - continue;
> + struct xdp_sock **map_entry = &m->xsk_map[i];
> + struct xdp_sock *old_xs;
>
> - sock_put((struct sock *)xs);
> + old_xs = xchg(map_entry, NULL);
> + if (old_xs)
> + xsk_map_del_node(old_xs, map_entry);
> }
> + spin_unlock_bh(&m->lock);
>
> free_percpu(m->flush_list);
> bpf_map_area_free(m->xsk_map);
> @@ -166,7 +218,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> {
> struct xsk_map *m = container_of(map, struct xsk_map, map);
> u32 i = *(u32 *)key, fd = *(u32 *)value;
> - struct xdp_sock *xs, *old_xs;
> + struct xdp_sock *xs, *old_xs, **entry;
> + struct xsk_map_node *node;
> struct socket *sock;
> int err;
>
> @@ -193,11 +246,20 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> return -EOPNOTSUPP;
> }
>
> - sock_hold(sock->sk);
> + node = xsk_map_node_alloc();
> + if (!node) {
> + sockfd_put(sock);
> + return -ENOMEM;
> + }
>
> - old_xs = xchg(&m->xsk_map[i], xs);
> + spin_lock_bh(&m->lock);
> + entry = &m->xsk_map[i];
> + xsk_map_node_init(node, m, entry);
> + xsk_map_add_node(xs, node);
> + old_xs = xchg(entry, xs);
> if (old_xs)
> - sock_put((struct sock *)old_xs);
> + xsk_map_del_node(old_xs, entry);
> + spin_unlock_bh(&m->lock);
>
> sockfd_put(sock);
> return 0;
> @@ -206,19 +268,32 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> static int xsk_map_delete_elem(struct bpf_map *map, void *key)
> {
> struct xsk_map *m = container_of(map, struct xsk_map, map);
> - struct xdp_sock *old_xs;
> + struct xdp_sock *old_xs, **map_entry;
> int k = *(u32 *)key;
>
> if (k >= map->max_entries)
> return -EINVAL;
>
> - old_xs = xchg(&m->xsk_map[k], NULL);
> + spin_lock_bh(&m->lock);
> + map_entry = &m->xsk_map[k];
> + old_xs = xchg(map_entry, NULL);
> if (old_xs)
> - sock_put((struct sock *)old_xs);
> + xsk_map_del_node(old_xs, map_entry);
> + spin_unlock_bh(&m->lock);
>
> return 0;
> }
>
> +void xsk_map_delete_from_node(struct xdp_sock *xs, struct list_head *node)
> +{
> + struct xsk_map_node *n = list_entry(node, struct xsk_map_node, node);
> +
> + spin_lock_bh(&n->map->lock);
> + *n->map_entry = NULL;
> + spin_unlock_bh(&n->map->lock);
> + xsk_map_node_free(n);
> +}
> +
> const struct bpf_map_ops xsk_map_ops = {
> .map_alloc = xsk_map_alloc,
> .map_free = xsk_map_free,
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index a14e8864e4fa..1931d98a7754 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -335,6 +335,27 @@ static int xsk_init_queue(u32 entries, struct xsk_queue **queue,
> return 0;
> }
>
> +static struct list_head *xsk_map_list_pop(struct xdp_sock *xs)
> +{
> + struct list_head *node = NULL;
> +
> + spin_lock_bh(&xs->map_list_lock);
> + if (!list_empty(&xs->map_list)) {
> + node = xs->map_list.next;
> + list_del(node);
> + }
> + spin_unlock_bh(&xs->map_list_lock);
> + return node;
> +}
> +
> +static void xsk_delete_from_maps(struct xdp_sock *xs)
> +{
> + struct list_head *node;
> +
> + while ((node = xsk_map_list_pop(xs)))
> + xsk_map_delete_from_node(xs, node);
> +}
> +
I stared at this set for a while and I think there are still two
issues in the design unless I'm missing something obvious.
1) xs teardown and parallel map update:
- CPU0 is in xsk_release(), calls xsk_delete_from_maps().
- CPU1 is in xsk_map_update_elem(), both access the same map slot.
- CPU0 does the xsk_map_list_pop() for that given slot, gets
interrupted before calling into xsk_map_delete_from_node().
- CPU1 takes m->lock in updates, *entry = xs to the new sock,
does xsk_map_del_node() to check on the xs (which CPU0 tears
down). Given this was popped off the list, it doesn't do
anything here, all good. It unlocks m->lock and succeeds.
- CPU0 now continues in xsk_map_delete_from_node(), takes
m->lock, zeroes *n->map_entry, releases m->lock, and frees
n. However, at this point *n->map_entry contains the xs that
we've just updated on CPU1. So zero'ing it will 1) remove
the wrong entry, and ii) leak it since it goes out of reach.
2) Inconsistent use of xchg() and friends:
- AF_XDP fast-path is doing READ_ONCE(m->xsk_map[key]) without
taking m->lock. This is also why you have xchg() for example
inside m->lock region since both protect different things (should
probably be commented). However, this is not consistently used.
E.g. xsk_map_delete_from_node() or xsk_map_update_elem() have
plain assignment, so compiler could in theory happily perform
store tearing and the READ_ONCE() would see garbage. This needs
to be consistently paired.
> static int xsk_release(struct socket *sock)
> {
> struct sock *sk = sock->sk;
> @@ -354,6 +375,7 @@ static int xsk_release(struct socket *sock)
> sock_prot_inuse_add(net, sk->sk_prot, -1);
> local_bh_enable();
>
> + xsk_delete_from_maps(xs);
> if (xs->dev) {
> struct net_device *dev = xs->dev;
>
> @@ -767,6 +789,9 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
> mutex_init(&xs->mutex);
> spin_lock_init(&xs->tx_completion_lock);
>
> + INIT_LIST_HEAD(&xs->map_list);
> + spin_lock_init(&xs->map_list_lock);
> +
> mutex_lock(&net->xdp.lock);
> sk_add_node_rcu(sk, &net->xdp.list);
> mutex_unlock(&net->xdp.lock);
>
^ permalink raw reply
* Re: [GIT] Networking
From: pr-tracker-bot @ 2019-06-28 0:30 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20190626.195006.2073691861982062351.davem@davemloft.net>
The pull request you sent on Wed, 26 Jun 2019 19:50:06 -0700 (PDT):
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net refs/heads/master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c84afab02c311b08b5cb8ea758cc177f81c95d11
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH 1/2] tls: remove close callback sock unlock/lock and flush_sync
From: Jakub Kicinski @ 2019-06-27 23:44 UTC (permalink / raw)
To: John Fastabend; +Cc: daniel, ast, netdev, edumazet, bpf
In-Reply-To: <156165700197.32598.17496423044615153967.stgit@john-XPS-13-9370>
On Thu, 27 Jun 2019 10:36:42 -0700, John Fastabend wrote:
> The tls close() callback currently drops the sock lock, makes a
> cancel_delayed_work_sync() call, and then relocks the sock. This
> seems suspect at best. The lock_sock() is applied to stop concurrent
> operations on the socket while tearing the sock down. Further we
> will need to add support for unhash() shortly and this complicates
> matters because the lock may or may not be held then.
>
> So to fix the above situation and simplify the next patch to add
> unhash this patch creates a function tls_sk_proto_cleanup() that
> tears down the socket without calling lock_sock/release_sock. In
> order to flush the workqueue then we do the following,
>
> - Add a new bit to ctx, BIT_TX_CLOSING that is set when the
> tls resources are being removed.
> - Check this bit before scheduling any new work. This way we
> avoid queueing new work after tear down has started.
> - With the BIT_TX_CLOSING ensuring no new work is being added
> convert the cancel_delayed_work_sync to flush_delayed_work()
> - Finally call tlx_tx_records() to complete any available records
> before,
> - releasing and removing tls ctx.
>
> The above is implemented for the software case namely any of
> the following configurations from build_protos,
>
> prot[TLS_SW][TLS_BASE]
> prot[TLS_BASE][TLS_SW]
> prot[TLS_SW][TLS_SW]
>
> The implication is a follow up patch is needed to resolve the
> hardware offload case.
>
> Tested with net selftests and bpf selftests.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> include/net/tls.h | 4 ++--
> net/tls/tls_main.c | 54 ++++++++++++++++++++++++++--------------------------
> net/tls/tls_sw.c | 50 ++++++++++++++++++++++++++++++++----------------
> 3 files changed, 62 insertions(+), 46 deletions(-)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index 4a55ce6a303f..6fe1f5c96f4a 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -105,9 +105,7 @@ struct tls_device {
> enum {
> TLS_BASE,
> TLS_SW,
> -#ifdef CONFIG_TLS_DEVICE
> TLS_HW,
> -#endif
> TLS_HW_RECORD,
> TLS_NUM_CONFIG,
> };
> @@ -160,6 +158,7 @@ struct tls_sw_context_tx {
> int async_capable;
>
> #define BIT_TX_SCHEDULED 0
BTW do you understand why we track this bit separately? Just to avoid
the irq operations in the workqueue code?
> +#define BIT_TX_CLOSING 1
But since we do have the above, and I think it's tested everywhere,
wouldn't setting SCHEDULED without accentually scheduling have
effectively the same result?
> unsigned long tx_bitmask;
> };
>
> @@ -327,6 +326,7 @@ void tls_sw_close(struct sock *sk, long timeout);
> void tls_sw_free_resources_tx(struct sock *sk);
> void tls_sw_free_resources_rx(struct sock *sk);
> void tls_sw_release_resources_rx(struct sock *sk);
> +void tls_sw_release_strp_rx(struct tls_context *tls_ctx);
> int tls_sw_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> int nonblock, int flags, int *addr_len);
> bool tls_sw_stream_read(const struct sock *sk);
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index fc81ae18cc44..51cb19e24dd9 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -261,24 +261,9 @@ static void tls_ctx_free(struct tls_context *ctx)
> kfree(ctx);
> }
>
> -static void tls_sk_proto_close(struct sock *sk, long timeout)
> +static void tls_sk_proto_cleanup(struct sock *sk,
> + struct tls_context *ctx, long timeo)
> {
> - struct tls_context *ctx = tls_get_ctx(sk);
> - long timeo = sock_sndtimeo(sk, 0);
> - void (*sk_proto_close)(struct sock *sk, long timeout);
> - bool free_ctx = false;
> -
> - lock_sock(sk);
> - sk_proto_close = ctx->sk_proto_close;
> -
> - if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
> - goto skip_tx_cleanup;
> -
> - if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
> - free_ctx = true;
> - goto skip_tx_cleanup;
> - }
> -
> if (!tls_complete_pending_work(sk, ctx, 0, &timeo))
> tls_handle_open_record(sk, 0);
>
> @@ -299,22 +284,37 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
> #ifdef CONFIG_TLS_DEVICE
> if (ctx->rx_conf == TLS_HW)
> tls_device_offload_cleanup_rx(sk);
> -
> - if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW) {
> -#else
> - {
> #endif
> - tls_ctx_free(ctx);
> - ctx = NULL;
> +}
> +
> +static void tls_sk_proto_close(struct sock *sk, long timeout)
> +{
> + struct tls_context *ctx = tls_get_ctx(sk);
> + long timeo = sock_sndtimeo(sk, 0);
> + void (*sk_proto_close)(struct sock *sk, long timeout);
> + bool free_ctx = false;
Set but not used?
> +
> + lock_sock(sk);
> + sk_proto_close = ctx->sk_proto_close;
> +
> + if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
> + goto skip_tx_cleanup;
> +
> + if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
> + free_ctx = true;
> + goto skip_tx_cleanup;
> }
>
> + tls_sk_proto_cleanup(sk, ctx, timeo);
> +
> skip_tx_cleanup:
> release_sock(sk);
> + if (ctx->rx_conf == TLS_SW)
> + tls_sw_release_strp_rx(ctx);
> sk_proto_close(sk, timeout);
> - /* free ctx for TLS_HW_RECORD, used by tcp_set_state
> - * for sk->sk_prot->unhash [tls_hw_unhash]
> - */
> - if (free_ctx)
> +
> + if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW &&
> + ctx->tx_conf != TLS_HW_RECORD && ctx->rx_conf != TLS_HW_RECORD)
> tls_ctx_free(ctx);
> }
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 455a782c7658..d234a6b818e6 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -473,7 +473,8 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
> return;
>
> /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
> + if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask) &&
> + !test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask))
Probably doesn't matter but seems like CLOSING test should be before
the test_and_set().
> schedule_delayed_work(&ctx->tx_work.work, 1);
> }
>
> @@ -2058,16 +2059,26 @@ void tls_sw_free_resources_tx(struct sock *sk)
> struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> struct tls_rec *rec, *tmp;
>
> + /* Set TX CLOSING bit to stop tx_work from being scheduled
> + * while tearing down TX context. We will flush any pending
> + * work before free'ing ctx anyways. If already set then
> + * another call is already free'ing resources.
> + */
Oh, can we get multiple calls here? Is this prep for unhash?
> + if (test_and_set_bit(BIT_TX_CLOSING, &ctx->tx_bitmask))
> + return;
> +
> /* Wait for any pending async encryptions to complete */
> smp_store_mb(ctx->async_notify, true);
> if (atomic_read(&ctx->encrypt_pending))
> crypto_wait_req(-EINPROGRESS, &ctx->async_wait);
>
> - release_sock(sk);
> - cancel_delayed_work_sync(&ctx->tx_work.work);
> - lock_sock(sk);
> -
> - /* Tx whatever records we can transmit and abandon the rest */
> + /* Flush work queue and then Tx whatever records we can
> + * transmit and abandon the rest, lock_sock(sk) must be
> + * held here. We ensure no further work is enqueue by
> + * checking CLOSING bit before queueing new work and
> + * setting it above.
> + */
> + flush_delayed_work(&ctx->tx_work.work);
> tls_tx_records(sk, -1);
>
> /* Free up un-sent records in tx_list. First, free
> @@ -2111,22 +2122,22 @@ void tls_sw_release_resources_rx(struct sock *sk)
> write_lock_bh(&sk->sk_callback_lock);
> sk->sk_data_ready = ctx->saved_data_ready;
> write_unlock_bh(&sk->sk_callback_lock);
> - release_sock(sk);
> - strp_done(&ctx->strp);
> - lock_sock(sk);
> }
> }
>
> -void tls_sw_free_resources_rx(struct sock *sk)
> +void tls_sw_release_strp_rx(struct tls_context *tls_ctx)
> {
> - struct tls_context *tls_ctx = tls_get_ctx(sk);
> struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
>
> - tls_sw_release_resources_rx(sk);
> -
> + strp_done(&ctx->strp);
> kfree(ctx);
> }
>
> +void tls_sw_free_resources_rx(struct sock *sk)
> +{
> + tls_sw_release_resources_rx(sk);
> +}
I don't understand the RX side well enough, but perhaps a separate
patch would make sense here?
> /* The work handler to transmitt the encrypted records in tx_list */
> static void tx_work_handler(struct work_struct *work)
> {
> @@ -2140,9 +2151,14 @@ static void tx_work_handler(struct work_struct *work)
> if (!test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
> return;
>
> - lock_sock(sk);
> + /* If we are running from a socket close operation then the
> + * lock is already held so we do not need to hold it.
> + */
> + if (likely(!test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)))
> + lock_sock(sk);
CPU 0 (free) CPU 1 (wq)
test_bit()
lock(sk)
set_bit()
lock(sk)
flush_work()
No?
> tls_tx_records(sk, -1);
> - release_sock(sk);
> + if (likely(!test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)))
> + release_sock(sk);
> }
>
> void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> @@ -2152,8 +2168,8 @@ void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> /* Schedule the transmission if tx list is ready */
> if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED,
> - &tx_ctx->tx_bitmask))
> + if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask) &&
> + !test_bit(BIT_TX_CLOSING, &tx_ctx->tx_bitmask))
> schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> }
> }
>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-06-27 23:42 UTC (permalink / raw)
To: Andy Lutomirski, Kees Cook, Linux API
Cc: Song Liu, Network Development, bpf, Alexei Starovoitov,
Daniel Borkmann, kernel-team, lmb, Jann Horn, Greg KH
In-Reply-To: <21894f45-70d8-dfca-8c02-044f776c5e05@kernel.org>
[sigh, I finally set up lore nntp, and I goofed some addresses. Hi
Kees and linux-api.]
On Thu, Jun 27, 2019 at 4:40 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On 6/27/19 1:19 PM, Song Liu wrote:
> > This patch introduce unprivileged BPF access. The access control is
> > achieved via device /dev/bpf. Users with write access to /dev/bpf are able
> > to call sys_bpf().
> >
> > Two ioctl command are added to /dev/bpf:
> >
> > The two commands enable/disable permission to call sys_bpf() for current
> > task. This permission is noted by bpf_permitted in task_struct. This
> > permission is inherited during clone(CLONE_THREAD).
> >
> > Helper function bpf_capable() is added to check whether the task has got
> > permission via /dev/bpf.
> >
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 0e079b2298f8..79dc4d641cf3 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
> > env->insn_aux_data[i].orig_idx = i;
> > env->prog = *prog;
> > env->ops = bpf_verifier_ops[env->prog->type];
> > - is_priv = capable(CAP_SYS_ADMIN);
> > + is_priv = bpf_capable(CAP_SYS_ADMIN);
>
> Huh? This isn't a hardening measure -- the "is_priv" verifier mode
> allows straight-up leaks of private kernel state to user mode.
>
> (For that matter, the pending lockdown stuff should possibly consider
> this a "confidentiality" issue.)
>
>
> I have a bigger issue with this patch, though: it's a really awkward way
> to pretend to have capabilities. For bpf, it seems like you could make
> this be a *real* capability without too much pain since there's only one
> syscall there. Just find a way to pass an fd to /dev/bpf into the
> syscall. If this means you need a new bpf_with_cap() syscall that takes
> an extra argument, so be it. The old bpf() syscall can just translate
> to bpf_with_cap(..., -1).
>
> For a while, I've considered a scheme I call "implicit rights". There
> would be a directory in /dev called /dev/implicit_rights. This would
> either be part of devtmpfs or a whole new filesystem -- it would *not*
> be any other filesystem. The contents would be files that can't be read
> or written and exist only in memory. You create them with a privileged
> syscall. Certain actions that are sensitive but not at the level of
> CAP_SYS_ADMIN (use of large-attack-surface bpf stuff, creation of user
> namespaces, profiling the kernel, etc) could require an "implicit
> right". When you do them, if you don't have CAP_SYS_ADMIN, the kernel
> would do a path walk for, say, /dev/implicit_rights/bpf and, if the
> object exists, can be opened, and actually refers to the "bpf" rights
> object, then the action is allowed. Otherwise it's denied.
>
> This is extensible, and it doesn't require the rather ugly per-task
> state of whether it's enabled.
>
> For things like creation of user namespaces, there's an existing API,
> and the default is that it works without privilege. Switching it to an
> implicit right has the benefit of not requiring code changes to programs
> that already work as non-root.
>
> But, for BPF in particular, this type of compatibility issue doesn't
> exist now. You already can't use most eBPF functionality without
> privilege. New bpf-using programs meant to run without privilege are
> *new*, so they can use a new improved API. So, rather than adding this
> obnoxious ioctl, just make the API explicit, please.
>
> Also, please cc: linux-abi next time.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox