Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] net: socket: Fix potential spectre v1 gadget in sock_is_registered
From: Jeremy Cline @ 2018-08-13 19:03 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: David S . Miller, netdev, linux-kernel, stable
In-Reply-To: <20180813171642.wlxmnzgsg2rkwe4o@treble>

On 08/13/2018 06:16 PM, Josh Poimboeuf wrote:
> On Sun, Jul 29, 2018 at 11:59:36AM -0400, Jeremy Cline wrote:
>> On 07/29/2018 09:59 AM, Josh Poimboeuf wrote:
>>> On Fri, Jul 27, 2018 at 10:43:02PM +0000, Jeremy Cline wrote:
>>>> 'family' can be a user-controlled value, so sanitize it after the bounds
>>>> check to avoid speculative out-of-bounds access.
>>>>
>>>> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Jeremy Cline <jcline@redhat.com>
>>>> ---
>>>>  net/socket.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/socket.c b/net/socket.c
>>>> index f15d5cbb3ba4..608e29ae6baf 100644
>>>> --- a/net/socket.c
>>>> +++ b/net/socket.c
>>>> @@ -2672,7 +2672,8 @@ EXPORT_SYMBOL(sock_unregister);
>>>>  
>>>>  bool sock_is_registered(int family)
>>>>  {
>>>> -	return family < NPROTO && rcu_access_pointer(net_families[family]);
>>>> +	return family < NPROTO &&
>>>> +		rcu_access_pointer(net_families[array_index_nospec(family, NPROTO)]);
>>>>  }
>>>>  
>>>>  static int __init sock_init(void)
>>>
>>> This is another one where I think it would be better to do the nospec
>>> clamp higher up the call chain.  The untrusted 'family' value comes from
>>> __sock_diag_cmd():
>>>
>>> __sock_diag_cmd
>>>   sock_load_diag_module
>>>     sock_is_registered
>>>
>>> That function has a bounds check, and also uses the value in some other
>>> array accesses:
>>>
>>> 	if (req->sdiag_family >= AF_MAX)
>>> 		return -EINVAL;
>>>
>>> 	if (sock_diag_handlers[req->sdiag_family] == NULL)
>>> 		sock_load_diag_module(req->sdiag_family, 0);
>>>
>>> 	mutex_lock(&sock_diag_table_mutex);
>>> 	hndl = sock_diag_handlers[req->sdiag_family];
>>> 	...
>>>
>>> So I think clamping 'req->sdiag_family' right after the bounds check
>>> would be the way to go.
>>>
>>
>> Indeed, the clamp there would cover this clamp. I had a scheme that I
>> quickly fix all the gadgets in functions with local comparisons, but
>> clearly that's going to result in call chains with multiple clamps.
>>
>> I can fix this in a follow-up with a clamp here, or respin this patch
>> set, whatever is easier for David.
> 
> Hi Jeremy,
> 
> Just checking up on this... since this patch was merged, will you be
> doing a followup patch?
> 

Yes, apologies, I've been traveling. I'll have a patch tomorrow.

^ permalink raw reply

* Containers and checkpoint/restart micro-conference at LPC2018
From: Stéphane Graber @ 2018-08-13 16:10 UTC (permalink / raw)
  To: lxc-devel, lxc-users, containers, linux-security-module,
	linux-fsdevel, netdev

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

Hello,

This year's edition of the Linux Plumbers Conference will once again
have a containers micro-conference but this time around we'll have twice
the usual amount of time and will include the content that would
traditionally go into the checkpoint/restore micro-conference.

LPC2018 will be held in Vancouver, Canada from the 13th to the 15th of
November, co-located with the Linux Kernel Summit.


We're looking for discussion topics around kernel work related to
containers and namespacing, resource control, access control,
checkpoint/restore of kernel structures, filesystem/mount handling for
containers and any related userspace work.


The format of the event will mostly be discussions where someone
introduces a given topic/problem and it then gets discussed for 20-30min
before moving on to something else. There will also be limited room for
short demos of recent work with shorter 15min slots.


Details can be found here:

  https://discuss.linuxcontainers.org/t/containers-micro-conference-at-linux-plumbers-2018/2417


Looking forward to seeing you in Vancouver!

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [net-next,v2] cpumask: make cpumask_next_wrap available without smp
From: Krzysztof Kozlowski @ 2018-08-13 16:01 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, davem, caleb.raitto, lkp, Willem de Bruijn
In-Reply-To: <20180812131403.123852-1-willemdebruijn.kernel@gmail.com>

On Sun, Aug 12, 2018 at 09:14:03AM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> The kbuild robot shows build failure on machines without CONFIG_SMP:

If this was reported by kbuild robot, then could you credit him with
Reported-by?

> 
>   drivers/net/virtio_net.c:1916:10: error:
>     implicit declaration of function 'cpumask_next_wrap'
> 
> cpumask_next_wrap is exported from lib/cpumask.o, which has
> 
>     lib-$(CONFIG_SMP) += cpumask.o
> 
> same as other functions, also define it as static inline in the
> NR_CPUS==1 branch in include/linux/cpumask.h.
> 
> If wrap is true and next == start, return nr_cpumask_bits, or 1.
> Else wrap across the range of valid cpus, here [0].
> 
> Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  include/linux/cpumask.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>

Tested-by: Krzysztof Kozlowski <krzk@kernel.org>

This fixes second build break of virtio driver in recent few days... I
wonder what happened with build-testing of own commits... or at least
reading kbuild robot mails.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH net-next 00/13] net: sched: actions rename for grep-ability and consistency
From: David Miller @ 2018-08-13 16:00 UTC (permalink / raw)
  To: jhs; +Cc: xiyou.wangcong, jiri, netdev, kernel
In-Reply-To: <20180812133501.15278-1-jhs@emojatatu.com>

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sun, 12 Aug 2018 09:34:48 -0400

> Having a structure (example tcf_mirred) and a function with the same name is
> not good for readability or grepability.
> 
> This long overdue patchset improves it and make sure there is consistency
> across all actions

Series applied.

^ permalink raw reply

* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-13 15:58 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, netdev
In-Reply-To: <20180813133817.GA18645@lunn.ch>

Hi Andrew/Florain,

On Mon, Aug 13, 2018 at 2:38 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > I agree, this should be padding packets correctly, can you still
> > > instrument cpsw to make sure that what comes to its ndo_start_xmit() is
> > > ETH_ZLEN + tag_len or more?
> > >
> > Yes I can confirm the skb->len is always >= 62 (ETH_ZLEN + 2)
>
> Which switch are you using?
>
> Marvell switches use either 4 or 8 bytes of tag. Broadcom has 4, KSZ
> has 1 for packets going to the switch, lan9303 has 4, mtd uses 4, qca
> has 2.
>
I am using the KSZ switch. for Ingress it has 1 byte and for Egress it
has 2 bytes.
I came across patch [1] and padded 2 more bytes in ksz_xmit() and I was
successfully able to ping from lan4 to PC. Thank you very much for
your guidance/support.

Now I have stumbled into a different issue:

Case 1 Works:
=================
lan0 = 192.168.0.1
PC1 = 192.168.0.10
For the above ping works from both directions.

CASE 2 Doesn’t Work:
=========================
lan0 = 192.168.0.1
PC1 = 192.168.0.10
lan4 = 192.168.0.4
PC2 = 192.168.0.11

Ping from lan0 to PC1 and PC1 to lan0 works
But ping from PC2 to lan4 and lan4 to PC2 fails.

CASE 3 Works:
=========================
lan0 = 192.168.0.1
PC1 = 192.168.0.10
lan4 = 192.168.4.4
PC2 = 192.168.4.11

With the above setup ping works.

[Query] Why does ping fail in case 2. Any thoughts what I am missing here ?
or is it the expected behaviour ?

[1] https://lore.kernel.org/patchwork/patch/851457/

Cheers,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH net-next v2] cpumask: make cpumask_next_wrap available without smp
From: David Miller @ 2018-08-13 15:58 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, caleb.raitto, lkp, willemb
In-Reply-To: <20180812131403.123852-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Sun, 12 Aug 2018 09:14:03 -0400

> From: Willem de Bruijn <willemb@google.com>
> 
> The kbuild robot shows build failure on machines without CONFIG_SMP:
> 
>   drivers/net/virtio_net.c:1916:10: error:
>     implicit declaration of function 'cpumask_next_wrap'
> 
> cpumask_next_wrap is exported from lib/cpumask.o, which has
> 
>     lib-$(CONFIG_SMP) += cpumask.o
> 
> same as other functions, also define it as static inline in the
> NR_CPUS==1 branch in include/linux/cpumask.h.
> 
> If wrap is true and next == start, return nr_cpumask_bits, or 1.
> Else wrap across the range of valid cpus, here [0].
> 
> Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied, thanks Willem.

^ permalink raw reply

* Re: [PATCH net] r8169: don't use MSI-X on RTL8168g
From: David Miller @ 2018-08-13 15:58 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <c2dab5e3-2444-8b34-7433-c7fdcf5f4a44@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 12 Aug 2018 13:26:26 +0200

> There have been two reports that network doesn't come back on resume
> from suspend when using MSI-X. Both cases affect the same chip version
> (RTL8168g - version 40), on different systems. Falling back to MSI
> fixes the issue.
> Even though we don't really have a proof yet that the network chip
> version is to blame, let's disable MSI-X for this version.
> 
> Reported-by: Steve Dodd <steved424@gmail.com>
> Reported-by: Lou Reed <gogen@disroot.org>
> Tested-by: Steve Dodd <steved424@gmail.com>
> Tested-by: Lou Reed <gogen@disroot.org>
> Fixes: 6c6aa15fdea5 ("r8169: improve interrupt handling")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net v2] l2tp: use sk_dst_check() to avoid race on sk->sk_dst_cache
From: David Miller @ 2018-08-13 15:49 UTC (permalink / raw)
  To: weiwan; +Cc: netdev, kafai, g.nault, dsahern, xiyou.wangcong
In-Reply-To: <20180810181456.76250-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>
Date: Fri, 10 Aug 2018 11:14:56 -0700

> From: Wei Wang <weiwan@google.com>
> 
> In l2tp code, if it is a L2TP_UDP_ENCAP tunnel, tunnel->sk points to a
> UDP socket. User could call sendmsg() on both this tunnel and the UDP
> socket itself concurrently. As l2tp_xmit_skb() holds socket lock and call
> __sk_dst_check() to refresh sk->sk_dst_cache, while udpv6_sendmsg() is
> lockless and call sk_dst_check() to refresh sk->sk_dst_cache, there
> could be a race and cause the dst cache to be freed multiple times.
> So we fix l2tp side code to always call sk_dst_check() to garantee
> xchg() is called when refreshing sk->sk_dst_cache to avoid race
> conditions.
> 
> Syzkaller reported stack trace:
 ...
> 
> Fixes: 71b1391a4128 ("l2tp: ensure sk->dst is still valid")
> Reported-by: syzbot+05f840f3b04f211bad55@syzkaller.appspotmail.com
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
 ...
> ---
> v1->v2: Removed dst_clone() as Guillaume Nault suggested

Applied and queued up for -stable, thank you.

^ permalink raw reply

* Re: [PATCH net-next v3 1/1] net/tls: Combined memory allocation for decryption request
From: David Miller @ 2018-08-13 15:42 UTC (permalink / raw)
  To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson
In-Reply-To: <20180810151641.14580-2-vakul.garg@nxp.com>

From: Vakul Garg <vakul.garg@nxp.com>
Date: Fri, 10 Aug 2018 20:46:41 +0530

> For preparing decryption request, several memory chunks are required
> (aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
> an accelerator, it is required that the buffers which are read by the
> accelerator must be dma-able and not come from stack. The buffers for
> aad and iv can be separately kmalloced each, but it is inefficient.
> This patch does a combined allocation for preparing decryption request
> and then segments into aead_req || sgin || sgout || iv || aad.
> 
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> ---
> 
> Changes since v2:
> 	- Rebased to not require following Doron Roberts-Kedes's patch.
> 	"net/tls: Calculate nsg for zerocopy path without skb_cow_data."

Applied.

^ permalink raw reply

* Re: [PATCH bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
From: Andrey Ignatov @ 2018-08-13 15:39 UTC (permalink / raw)
  To: Tejun Heo; +Cc: netdev, ast, daniel, guro, kernel-team
In-Reply-To: <20180813144635.GI3978217@devbig004.ftw2.facebook.com>

Tejun Heo <tj@kernel.org> [Mon, 2018-08-13 07:47 -0700]:
> Hello, Andrey.

Hey Tejun!

> On Fri, Aug 10, 2018 at 10:35:23PM -0700, Andrey Ignatov wrote:
> > +static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> > +					     int ancestor_level)
> > +{
> > +	struct cgroup *ptr;
> > +
> > +	if (cgrp->level < ancestor_level)
> > +		return NULL;
> > +
> > +	for (ptr = cgrp;
> > +	     ptr && ptr->level > ancestor_level;
> > +	     ptr = cgroup_parent(ptr))
> > +		;
> > +
> > +	if (ptr && ptr->level == ancestor_level)
> > +		return ptr;
> > +
> > +	return NULL;
> > +}
> 
> I don't have any objections functionanlity-wise but can we do sth like
> the following instead?
> 
> static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> 					     int ancestor_level)
> {
> 	if (cgrp->level < ancestor_level)
> 		return NULL;
> 
> 	while (cgrp->level > ancestor_level)
> 		cgrp = cgroup_parent(cgrp);
> 	return cgrp;
> }

Sure, no problem. I'll send follow-up patch to simplify this part when
bpf-next is open again.

> 
> Thanks.
> 
> -- 
> tejun

-- 
Andrey Ignatov

^ permalink raw reply

* Re: [V9fs-developer] [PATCH 2/2] 9p: Add refcount to p9_req_t
From: Tomas Bortoli @ 2018-08-13 18:14 UTC (permalink / raw)
  To: Dmitry Vyukov, Dominique Martinet
  Cc: piaojun, Eric Van Hensbergen, Ron Minnich, Latchesar Ionkov,
	Dominique Martinet, netdev, LKML, syzkaller, v9fs-developer,
	David Miller
In-Reply-To: <CACT4Y+YQckNWVcBoW1WtH=5pEU5KXB8J8A=QssWx8nbp5GtNnw@mail.gmail.com>

On 08/13/2018 03:04 PM, Dmitry Vyukov wrote:
> On Mon, Aug 13, 2018 at 3:48 AM, Dominique Martinet
> <asmadeus@codewreck.org> wrote:
>> piaojun wrote on Mon, Aug 13, 2018:
>>> Could you help paste the reason of the crash bug to help others
>>> understand more clearly? And I have another question below.
>>
>> The problem for tcp (but other transports have a similar problem) is
>> that with a malicious server like syzkaller they can try to submit
>> replies before the request came in.
>>
>> This leads in the writer thread trying to write a buffer that has
>> already been freed, and if memory has been reused could potentially leak
>> some information.
>>
>> Now, with the previous patches this is based on this would be a slab and
>> the likeliness of it being sensitive information is rather low (it would
>> likely be some other packet being sent twice, or a mix and match of two
>> packets that would have been sent anyway), but it would nevertheless be
>> a use after free.
>>
>>
>> There is a second advantage to this reference counting, that is now we
>> have this system we will be able to implement flush asynchronously.
>> This will remove the need for the 'goto again' in p9_client_rpc which
>> was making 9p threads unkillable in practice if the server would not
>> reply to the flush requests.
> 
> 
> Fixing unkillalble task would be nice. Don't know how much they are of
> a problem in real life, but fixing them would allow fuzzer to find
> other, potentially more critical bugs in 9p. These "task hung" crashes
> are quite unpleasant for the fuzzer.
> 
> Thanks for all recent 9p work, Tomas!
> 

You are welcome, I have to thank Dominique that helped me a lot, I like
to help here, it's educative.

> 
>> Even if the server replies I've always found myself needing to hit ^C
>> multiple times to exit a process doing I/Os and I think fixing that
>> behaviour will make 9p more comfortable to use.
>>
>>
>>>> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
>>>> index 20f46f13fe83..686e24e355d0 100644
>>>> --- a/net/9p/trans_fd.c
>>>> +++ b/net/9p/trans_fd.c
>>>> @@ -132,6 +132,7 @@ struct p9_conn {
>>>>     struct list_head req_list;
>>>>     struct list_head unsent_req_list;
>>>>     struct p9_req_t *req;
>>>> +   struct p9_req_t *wreq;
>>>
>>> Why adding a wreq for write work? And I wonder we should rename req to
>>> rreq?
>>
>> We need to store a pointer to the request for the write thread because
>> we need to put the reference to it when we're done writing its content.
>>
>> Previously, the worker would only store the write buffer there but
>> that's not enough to figure what request to dereference.
>>
>>
>> I personally don't think renaming req to rreq would bring much but it
>> could be done in another patch if you think that'd be helpful; I think
>> it shouldn't be done here at least to make the patch more readable.
>>
>> --
>> Dominique
>>
>> --
>> You received this message because you are subscribed to the Google Groups "syzkaller" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v1 0/3] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-08-13 17:55 UTC (permalink / raw)
  To: James Bottomley; +Cc: linux-kernel, netdev, davem, linux-crypto
In-Reply-To: <1534181830.7872.10.camel@HansenPartnership.com>

> but it's very hard for a flow classifier because you have to

The construction and identifier strings might not obviously help with
the extremely narrow idea you've brought up, but it is very important
for safely introducing additional versions. Namely, it prevents
against cross-protocol key reuse attacks and type confusion bugs. So
don't be too quick to dismiss the importance of these for
accomplishing what we're after.

> so lets pick one of the above and try it out.

We have, multiple times, and it's absolutely trivial to do and works
well. The exact thing you're concerned about has already been
researched and worked with on live systems quite a bit over the last 3
years, and it works in a pretty straight forward way. I'm not sure
there's much more to add here: the thing you want is already there and
has been tested extensively. At this point the "pick one and let's try
it out!" is an old story, and the focus now is on making sure the code
quality and netdev api usage is correct for merging

^ permalink raw reply

* pull request: bluetooth-next 2018-08-13
From: Johan Hedberg @ 2018-08-13 15:00 UTC (permalink / raw)
  To: davem; +Cc: linux-bluetooth, netdev

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

Hi Dave,

There was one pretty bad bug that slipped into the MediaTek HCI driver
in the last bluetooth-next pull request. Would it be possible to get
this one-liner fix pulled to net-next before you make your first 4.19
pull request for Linus? Thanks.

Johan

---
The following changes since commit a487711aac3b720b4a3a63d737604f47cd8dc36c:

  Merge branch 'r8169-smaller-improvements' (2018-08-10 14:32:35 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream

for you to fetch changes up to 330ad75f6a79d46f11f7bf8937852ebb4673b1d5:

  Bluetooth: mediatek: pass correct size to h4_recv_buf() (2018-08-13 15:59:39 +0200)

----------------------------------------------------------------
Dan Carpenter (1):
      Bluetooth: mediatek: pass correct size to h4_recv_buf()

 drivers/bluetooth/btmtkuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v1 0/3] WireGuard: Secure Network Tunnel
From: James Bottomley @ 2018-08-13 17:37 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, davem, linux-crypto
In-Reply-To: <CAHmME9raa81A_hH_Zykp2r8zOk0ySroFEsk+8384vnVOCuSa=g@mail.gmail.com>

On Mon, 2018-08-13 at 10:02 -0700, Jason A. Donenfeld wrote:
> > Could we please build planning for this crypto failure day into
> > wireguard now rather than have to do it later?  It doesn't need to
> > be full cipher agility, it just needs to be the ability to handle
> > multiple protocol versions ... two should do it because that gives
> > a template to follow (and test version to try to find bugs in the
> > implementation). It looks like the protocol could simply be updated
> > to put the version into one (or more) of the three reserved bytes
> > in the handshake headers, so perhaps doing this before they get
> > used for something else would be a good first step?
> > 
> > James
> > 
> > 
> 
> Indeed the answer is in fact along the lines of what you've suggested
> in your question: the protocol is very strictly versioned. This means
> that while there intentionally isn't negotiation of ciphers --
> something historically very bug-prone -- there is ample room for
> updating the protocol. This is enabled via 4 aspects of the protocol:
> 
> - An explicit "identifier" string is hashed in as part of the first
> step of cryptographic operations, containing a "v1" as well as the
> protocol designer's email.
> - An explicit "construction" string is hashed in as part of the first
> step of cryptographic operations, containing the Noise handshake
> pattern and a list of the cryptographic primitives used.

Any hash involving other parameters allows you to check for a version
mismatch, but it's very hard for a flow classifier because you have to
do the hash at the point you classify.  If we're running concurrent
versions we need an easy way to separate them.

> - A type field at the beginning of each message. Newer message types
> (corresponding with newer versions) can easily be introduced via this
> field, and they can even coexist with older ones need be.
> - Three unused reserved fields ready to be utilised in the event
> they're needed.

Either of these will work for easy classification.

> In other words, there's ample room for such contingency measures
> within the protocol.

I have a preference for explicit versioning, having dealt with some
protocol issues before.  However, I'm much less concerned with *how*
it's done than that it *be* done in the kernel patch so we can test out
 rolling the version number to change the algorithms in a backward
compatible way, so lets pick one of the above and try it out.

Regards,

James

^ permalink raw reply

* Re: [PATCH bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
From: Tejun Heo @ 2018-08-13 14:46 UTC (permalink / raw)
  To: Andrey Ignatov; +Cc: netdev, ast, daniel, guro, kernel-team
In-Reply-To: <b7abb909209e02651d873d90fe8825b432c73f0e.1533965421.git.rdna@fb.com>

Hello, Andrey.

On Fri, Aug 10, 2018 at 10:35:23PM -0700, Andrey Ignatov wrote:
> +static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> +					     int ancestor_level)
> +{
> +	struct cgroup *ptr;
> +
> +	if (cgrp->level < ancestor_level)
> +		return NULL;
> +
> +	for (ptr = cgrp;
> +	     ptr && ptr->level > ancestor_level;
> +	     ptr = cgroup_parent(ptr))
> +		;
> +
> +	if (ptr && ptr->level == ancestor_level)
> +		return ptr;
> +
> +	return NULL;
> +}

I don't have any objections functionanlity-wise but can we do sth like
the following instead?

static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
					     int ancestor_level)
{
	if (cgrp->level < ancestor_level)
		return NULL;

	while (cgrp->level > ancestor_level)
		cgrp = cgroup_parent(cgrp);
	return cgrp;
}

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v2 iproute2-next] sch_cake: Make gso-splitting configurable
From: David Ahern @ 2018-08-13 14:45 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: netdev, Dave Taht
In-Reply-To: <20180813113617.13073-1-toke@toke.dk>

On 8/13/18 5:36 AM, Toke Høiland-Jørgensen wrote:
> This patch makes sch_cake's gso/gro splitting configurable
> from userspace.
> 
> To disable breaking apart superpackets in sch_cake:
> 
> tc qdisc replace dev whatever root cake no-split-gso
> 
> to enable:
> 
> tc qdisc replace dev whatever root cake split-gso
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> Signed-off-by: Dave Taht <dave.taht@gmail.com>
> ---

applied to iproute2-next. Thanks

I think you also need to display it if the attribute is returned.

^ permalink raw reply

* Re: [PATCH 2/2] net: socket: Fix potential spectre v1 gadget in sock_is_registered
From: Josh Poimboeuf @ 2018-08-13 17:16 UTC (permalink / raw)
  To: Jeremy Cline; +Cc: David S . Miller, netdev, linux-kernel, stable
In-Reply-To: <914d34af-ba80-93b9-6f17-413eef8bf210@redhat.com>

On Sun, Jul 29, 2018 at 11:59:36AM -0400, Jeremy Cline wrote:
> On 07/29/2018 09:59 AM, Josh Poimboeuf wrote:
> > On Fri, Jul 27, 2018 at 10:43:02PM +0000, Jeremy Cline wrote:
> >> 'family' can be a user-controlled value, so sanitize it after the bounds
> >> check to avoid speculative out-of-bounds access.
> >>
> >> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Jeremy Cline <jcline@redhat.com>
> >> ---
> >>  net/socket.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/socket.c b/net/socket.c
> >> index f15d5cbb3ba4..608e29ae6baf 100644
> >> --- a/net/socket.c
> >> +++ b/net/socket.c
> >> @@ -2672,7 +2672,8 @@ EXPORT_SYMBOL(sock_unregister);
> >>  
> >>  bool sock_is_registered(int family)
> >>  {
> >> -	return family < NPROTO && rcu_access_pointer(net_families[family]);
> >> +	return family < NPROTO &&
> >> +		rcu_access_pointer(net_families[array_index_nospec(family, NPROTO)]);
> >>  }
> >>  
> >>  static int __init sock_init(void)
> > 
> > This is another one where I think it would be better to do the nospec
> > clamp higher up the call chain.  The untrusted 'family' value comes from
> > __sock_diag_cmd():
> > 
> > __sock_diag_cmd
> >   sock_load_diag_module
> >     sock_is_registered
> > 
> > That function has a bounds check, and also uses the value in some other
> > array accesses:
> > 
> > 	if (req->sdiag_family >= AF_MAX)
> > 		return -EINVAL;
> > 
> > 	if (sock_diag_handlers[req->sdiag_family] == NULL)
> > 		sock_load_diag_module(req->sdiag_family, 0);
> > 
> > 	mutex_lock(&sock_diag_table_mutex);
> > 	hndl = sock_diag_handlers[req->sdiag_family];
> > 	...
> > 
> > So I think clamping 'req->sdiag_family' right after the bounds check
> > would be the way to go.
> > 
> 
> Indeed, the clamp there would cover this clamp. I had a scheme that I
> quickly fix all the gadgets in functions with local comparisons, but
> clearly that's going to result in call chains with multiple clamps.
> 
> I can fix this in a follow-up with a clamp here, or respin this patch
> set, whatever is easier for David.

Hi Jeremy,

Just checking up on this... since this patch was merged, will you be
doing a followup patch?

-- 
Josh

^ permalink raw reply

* Re: [PATCH] mt76: Enable NL80211_EXT_FEATURE_CQM_RSSI_LIST
From: Kristian Evensen @ 2018-08-13 14:25 UTC (permalink / raw)
  To: lorenzo.bianconi
  Cc: arend.vanspriel, kvalo, linux-wireless, Network Development
In-Reply-To: <CAJ0CqmVZNAnHwnU4YqRRv7R8DyHi2MiFJr1bvQDPVszUU5Ez3g@mail.gmail.com>

Hi Lorenzo,

On Mon, Aug 13, 2018 at 12:55 PM Lorenzo Bianconi
<lorenzo.bianconi@redhat.com> wrote:
> According to my understanding (please correct me if I am wrong)
> BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
> by the driver/fw, while if it is not set mac80211 will take care of that
> in ieee80211_handle_beacon_sig routine.
> I am AFK at the moment, I will test that patch when I am back from vacations.

That matches my understanding as well (base on for example the message
for commit ae44b502669d0cd1f167cdb48994292aa20fd3dd).

Great that you are willing to test the patch with one of the mt76
USB-dongles. Unless anyone objects, I will postpone sending a v2 until
you report back with your findings :)

BR,
Kristian

^ permalink raw reply

* Re: pull-request: wireless-drivers-next 2018-08-12
From: David Miller @ 2018-08-13 17:06 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87wosvxwmc.fsf@kamboji.qca.qualcomm.com>

From: Kalle Valo <kvalo@codeaurora.org>
Date: Sun, 12 Aug 2018 21:34:03 +0300

> one more request to net-next for 4.19. I hope I'm not too late with
> this. These have been in linux-next since Friday so I'm hoping there are
> no surprises. Please let me know if you have any problems.

Pulled, thanks Kalle.

^ permalink raw reply

* Re: [PATCH v1 0/3] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-08-13 17:02 UTC (permalink / raw)
  To: James Bottomley; +Cc: linux-kernel, netdev, davem, linux-crypto
In-Reply-To: <1534174811.7872.3.camel@HansenPartnership.com>

Hi James,

On 8/13/18, James Bottomley <James.Bottomley@hansenpartnership.com> wrote:
>> Ample information, including documentation, installation
>> instructions,
>> and project details, is available at:
>>
>>   * https://www.wireguard.com/
>>   * https://www.wireguard.com/papers/wireguard.pdf
>
> In your paper you say this:
>
>> Finally, WireGuard is cryptographically opinionated. It intentionally
>> lacks cipher and protocol agility. If
>> holes are found in the underlying primitives, all endpoints will be
>> required to update.
>
> The only thing that's certain (beyond death and taxes) is that your
> crypto choice will one day need updating; either in response to an
> urgent CVE because an algorithm is compromised or in response to a less
> urgent one because it is deprecated.  Assuming wireguard is reasonably
> successful we'll have a large ecosystem dependent on it.  On this day,
> we're going to have the choice of either breaking the entire ecosystem
> by rolling out a change that can't connect to lower protocol versions
> or trying to wedge version agility into wireguard in a hurry.  The
> former is too awful to contemplate because of the almost universal
> ecosystem breakage it would cause and the latter is going to lead to
> additional bugs because people in a hurry aren't as careful as they
> should be.
>
> Could we please build planning for this crypto failure day into
> wireguard now rather than have to do it later?  It doesn't need to be
> full cipher agility, it just needs to be the ability to handle multiple
> protocol versions ... two should do it because that gives a template to
> follow (and test version to try to find bugs in the implementation).
> It looks like the protocol could simply be updated to put the version
> into one (or more) of the three reserved bytes in the handshake
> headers, so perhaps doing this before they get used for something else
> would be a good first step?
>
> James
>
>

Indeed the answer is in fact along the lines of what you've suggested
in your question: the protocol is very strictly versioned. This means
that while there intentionally isn't negotiation of ciphers --
something historically very bug-prone -- there is ample room for
updating the protocol. This is enabled via 4 aspects of the protocol:

- An explicit "identifier" string is hashed in as part of the first
step of cryptographic operations, containing a "v1" as well as the
protocol designer's email.
- An explicit "construction" string is hashed in as part of the first
step of cryptographic operations, containing the Noise handshake
pattern and a list of the cryptographic primitives used.
- A type field at the beginning of each message. Newer message types
(corresponding with newer versions) can easily be introduced via this
field, and they can even coexist with older ones need be.
- Three unused reserved fields ready to be utilised in the event they're needed.

In other words, there's ample room for such contingency measures
within the protocol.

Jason

^ permalink raw reply

* Re: [PATCH v2 net-next] liquidio: remove set but not used variable 'is25G'
From: David Miller @ 2018-08-13 16:48 UTC (permalink / raw)
  To: yuehaibing
  Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
	linux-kernel, netdev
In-Reply-To: <20180813092936.5200-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 13 Aug 2018 17:29:36 +0800

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c: In function 'lio_set_link_ksettings':
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:392:6: warning:
>  variable 'is25G' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> v2: remove unnecessary braces as Shahed suggested

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] cxgb4: remove set but not used variable 'spd'
From: David Miller @ 2018-08-13 16:48 UTC (permalink / raw)
  To: yuehaibing; +Cc: ganeshgr, linux-kernel, netdev
In-Reply-To: <20180813064851.912-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 13 Aug 2018 14:48:51 +0800

> Fixes gcc '-Wunused-but-set-variable' warning:
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c: In function 'print_port_info':
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:5147:14: warning:
>  variable 'spd' set but not used [-Wunused-but-set-variable]
> 
> variable 'spd' is set but not used since
> commit 547fd27241a8 ("cxgb4: Warn if device doesn't have enough PCI bandwidth")
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: stmmac: Add SMC support for EMAC System Manager register
From: David Miller @ 2018-08-13 16:48 UTC (permalink / raw)
  To: joyce.ooi
  Cc: peppe.cavallaro, alexandre.torgue, joabreu, netdev, linux-kernel,
	hean.loong.ong, yves.vandervennet
In-Reply-To: <1534142494-11541-1-git-send-email-joyce.ooi@intel.com>

From: "Ooi, Joyce" <joyce.ooi@intel.com>
Date: Sun, 12 Aug 2018 23:41:34 -0700

> As there is restriction to access to EMAC System Manager registers in
> the kernel for Intel Stratix10, the use of SMC calls are required and
> added in dwmac-socfpga driver.
> 
> Signed-off-by: Ooi, Joyce <joyce.ooi@intel.com>
> ---
> This patch is dependent on https://lkml.org/lkml/2018/7/26/624

I guess I cannot apply this to my networking tree then.

I would suggest that you make a helper in a header file which dos the
special SMC EMAC accesses, or alternatively the regular regmap access,
based upon the CPP ifdef.

That way you won't have to put all of those CPP tests in the foo.c
code.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next] virtio_net: remove duplicated include from virtio_net.c
From: David Miller @ 2018-08-13 16:46 UTC (permalink / raw)
  To: yuehaibing; +Cc: mst, jasowang, linux-kernel, netdev, virtualization
In-Reply-To: <20180813061315.9084-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 13 Aug 2018 14:13:15 +0800

> Remove duplicated include linux/netdevice.h
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/3] IB/ipoib: Stop using dev_id to expose port numbers
From: Arseny Maslennikov @ 2018-08-13 13:57 UTC (permalink / raw)
  To: Yuval Shaia; +Cc: linux-rdma, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180813124018.GA6122@lap1>

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

On Mon, Aug 13, 2018 at 03:40:20PM +0300, Yuval Shaia wrote:
> On Mon, Aug 13, 2018 at 02:42:23PM +0300, Arseny Maslennikov wrote:
> > Some InfiniBand network devices have multiple ports on the same PCI
> > function. Prior to this the kernel erroneously used the `dev_id' sysfs
> > field of those network interfaces to convey the port number to userspace.
> > 
> > `dev_id' is currently reserved for distinguishing stacked ifaces
> > (e.g: VLANs) with the same hardware address as their parent device.
> > 
> > Similar fixes to net/mlx4_en and many other drivers, which started
> > exporting this information through `dev_id' before 3.15, were accepted
> > into the kernel 4 years ago.
> > See 76a066f2a2a0268b565459c417b59724b5a3197b, commit message:
> > `net/mlx4_en: Expose port number through sysfs'.
> > 
> > I would be OK with this commit not being backported to stable, since
> > it might break admin-supplied udev rules and the likes.
> > 
> > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > ---
> >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > index 6eb0594fffec..f64535038147 100644
> > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > @@ -2252,7 +2252,6 @@ static struct net_device *ipoib_add_port(const char *format,
> >  	}
> >  
> >  	SET_NETDEV_DEV(priv->dev, hca->dev.parent);
> > -	priv->dev->dev_id = port - 1;
> 
> Correct me if i'm wrong here but besides some changes in commit message
> looks like patch 1/3 is the same as 2/3, isn't it?
> 
> Yuval
> 

1/3 has an extra line, 2/3 removes a different line.

(a) If you apply both 1/3 and 2/3, the port number can be seen at
/sys/class/net/*/dev_port and not at .../dev_id.

(b) If you apply only 1/3, the port number can be seen at _both_
.../dev_port and .../dev_id (to preserve backward compatibility with
e.g. existing udev rules that rely on "ATTR{dev_id}")

By splitting those up we have both options (a) and (b) available,
so the maintainers are free to decide which one is wiser.

> >  	priv->dev->dev_port = port - 1;
> >  
> >  	result = ib_query_port(hca, port, &attr);
> > -- 
> > 2.18.0
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply


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