* Re: [PATCH 08/16] wireless: Replace spin_is_locked() with lockdep
From: Kalle Valo @ 2018-10-03 9:06 UTC (permalink / raw)
To: Lance Roy
Cc: linux-kernel, Paul E. McKenney, Daniel Drake, Ulrich Kunitz,
David S. Miller, linux-wireless, netdev
In-Reply-To: <20181003053902.6910-9-ldr709@gmail.com>
Lance Roy <ldr709@gmail.com> writes:
> lockdep_assert_held() is better suited to checking locking requirements,
> since it won't get confused when someone else holds the lock. This is
> also a step towards possibly removing spin_is_locked().
>
> Signed-off-by: Lance Roy <ldr709@gmail.com>
> Cc: Daniel Drake <dsd@gentoo.org>
> Cc: Ulrich Kunitz <kune@deine-taler.de>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: <linux-wireless@vger.kernel.org>
> Cc: <netdev@vger.kernel.org>
> ---
> drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Should I take this or is it going through some other tree?
If it goes to via some other tree:
Acked-by: Kalle Valo <kvalo@codeaurora.org>
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next] inet: do not set backlog if listen fails
From: Eric Dumazet @ 2018-10-03 15:41 UTC (permalink / raw)
To: Yafang Shao, edumazet, davem; +Cc: netdev, linux-kernel
In-Reply-To: <1538579279-11614-1-git-send-email-laoar.shao@gmail.com>
On 10/03/2018 08:07 AM, Yafang Shao wrote:
> These two backlog members are not necessary set in inet_csk_listen_start().
> Regarding sk_max_ack_backlog, it will be set in the caller inet_listen
> and dccp_listen_start.
> Regraing sk_ack_backlog, it is better to put it together with
> sk_max_ack_backlog in the same function and only set on success.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> net/dccp/proto.c | 1 +
> net/ipv4/af_inet.c | 1 +
> net/ipv4/inet_connection_sock.c | 3 ---
> 3 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/dccp/proto.c b/net/dccp/proto.c
> index 875858c..34d48cd 100644
> --- a/net/dccp/proto.c
> +++ b/net/dccp/proto.c
> @@ -959,6 +959,7 @@ int inet_dccp_listen(struct socket *sock, int backlog)
> err = dccp_listen_start(sk, backlog);
> if (err)
> goto out;
> + sk->sk_ack_backlog = 0;
This is racy, remember that dccp and tcp have lockless listeners.
Do not change sk_ack_backlog after a TCP/DCCP socket is ready to accept new flows.
^ permalink raw reply
* [PATCH net-next] inet: do not set backlog if listen fails
From: Yafang Shao @ 2018-10-03 15:07 UTC (permalink / raw)
To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao
These two backlog members are not necessary set in inet_csk_listen_start().
Regarding sk_max_ack_backlog, it will be set in the caller inet_listen
and dccp_listen_start.
Regraing sk_ack_backlog, it is better to put it together with
sk_max_ack_backlog in the same function and only set on success.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
net/dccp/proto.c | 1 +
net/ipv4/af_inet.c | 1 +
net/ipv4/inet_connection_sock.c | 3 ---
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 875858c..34d48cd 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -959,6 +959,7 @@ int inet_dccp_listen(struct socket *sock, int backlog)
err = dccp_listen_start(sk, backlog);
if (err)
goto out;
+ sk->sk_ack_backlog = 0;
}
sk->sk_max_ack_backlog = backlog;
err = 0;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1fbe2f8..9690e0d 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -230,6 +230,7 @@ int inet_listen(struct socket *sock, int backlog)
if (err)
goto out;
tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_LISTEN_CB, 0, NULL);
+ sk->sk_ack_backlog = 0;
}
sk->sk_max_ack_backlog = backlog;
err = 0;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index dfd5009..c25d0b3 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -870,9 +870,6 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
int err = -EADDRINUSE;
reqsk_queue_alloc(&icsk->icsk_accept_queue);
-
- sk->sk_max_ack_backlog = backlog;
- sk->sk_ack_backlog = 0;
inet_csk_delack_init(sk);
/* There is race window here: we announce ourselves listening,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] ipv6: revert degradation in IPv6 Ready Logo test results
From: 吉藤英明 @ 2018-10-03 8:16 UTC (permalink / raw)
To: mmanning; +Cc: davem, netdev, fw
In-Reply-To: <c2208861-381c-1f6a-edfe-94d93a80e749@vyatta.att-mail.com>
Hi,
2018年10月3日(水) 16:57 Mike Manning <mmanning@vyatta.att-mail.com>:
>
> On 02/10/2018 19:26, David Miller wrote:
> > From: Mike Manning <mmanning@vyatta.att-mail.com>
> > Date: Tue, 2 Oct 2018 12:40:30 +0100
> >
> >> This reverts commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags
> >> smaller than min mtu"). While one should not get fragments smaller than
> >> the IPv6 minimum MTU, not handling crafted packets in the TAHI IPv6
> >> conformance test suite (v6eval) for IPv6 Ready Logo results in 18
> >> failures representing over 5% of the score.
> >>
> >> Cc: Florian Westphal <fw@strlen.de>
> >> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> > Sorry, I'm not just going to blindly apply a patch because some
> > TAHI tests fail.
> >
> > It's possible the TAHI tests are wrong, or that the specification
> > elements it is testing don't make any sense these days.
> >
> > Allowing all kinds of random junk in the middle of the fragment queue
> > leads to lots of unnecessary cpu overhead and potential bugs, and it
> > triggerable remotely.
>
> Understood, thank you.
>
> It would be great if there is someone on this mailer who has influence
> with ipv6ready.org so as to get the TAHI tests for IPv6 conformance
> updated, as an upgrade to a kernel with the commit mentioned will result
> in a 5% degradation in results for the existing tests.
>
You can ignore some tests especially if you have some related,
updated RFC(s).
--yoshfuji
^ permalink raw reply
* Re: [PATCH net-next v6 11/23] zinc: import Andy Polyakov's Poly1305 ARM and ARM64 implementations
From: Ard Biesheuvel @ 2018-10-03 7:58 UTC (permalink / raw)
To: Eric Biggers
Cc: Andy Polyakov, Jason A. Donenfeld, Linux Kernel Mailing List,
<netdev@vger.kernel.org>,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Greg Kroah-Hartman, Samuel Neves, Andy Lutomirski,
Jean-Philippe Aumasson, Russell King, linux-arm-kernel
In-Reply-To: <20181003061236.GB745@sol.localdomain>
On 3 October 2018 at 08:12, Eric Biggers <ebiggers@kernel.org> wrote:
> On Tue, Sep 25, 2018 at 04:56:10PM +0200, Jason A. Donenfeld wrote:
>> These NEON and non-NEON implementations come from Andy Polyakov's
>> implementation, and are included here in raw form without modification,
>> so that subsequent commits that fix these up for the kernel can see how
>> it has changed. This awkward commit splitting has been requested for the
>> ARM[64] implementations in particular.
>>
"This awkward commit splitting"
Seriously?!?
So you really think it is fine to import huge chunks of code like this
from other projects without keeping track of the local changes?
>> While this is CRYPTOGAMS code, the originating code for this happens to
>> be the same as OpenSSL's commit 5bb1cd2292b388263a0cc05392bb99141212aa53
>
> Sorry to ruin the fun, but actually there are no Poly1305 implementations in
> CRYPTOGAMS (https://github.com/dot-asm/cryptogams). Nor are there any ChaCha20
> implementations.
>
So was this code taken directly from the OpenSSL project then? If so,
why do the commit messages claim otherwise?
> Andy P., can you please add your Poly1305 and ChaCha20 implementations to the
> CRYPTOGAMS repository, so that they have a clear kernel-compatible license?
>
> It would be great if you'd include a kernel-compatible license directly in the
> versions in the OpenSSL tree too...
>
Yes please.
^ permalink raw reply
* Re: [PATCH net-next v6 11/23] zinc: import Andy Polyakov's Poly1305 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-10-03 14:45 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Eric Biggers, Andy Polyakov, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAHmME9oAa+Ezqy639_wSKsUDW4vgSrkbJUVd1O8Jcbo3CN4WKg@mail.gmail.com>
On Wed, Oct 3, 2018 at 4:08 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> for all of the other places, and so I the comment in the dev tree a
The missing word between "I" and "the" is "extirpated".
^ permalink raw reply
* Re: [PATCH net] ipv6: revert degradation in IPv6 Ready Logo test results
From: Mike Manning @ 2018-10-03 7:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fw
In-Reply-To: <20181002.112600.2116388713222602539.davem@davemloft.net>
On 02/10/2018 19:26, David Miller wrote:
> From: Mike Manning <mmanning@vyatta.att-mail.com>
> Date: Tue, 2 Oct 2018 12:40:30 +0100
>
>> This reverts commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags
>> smaller than min mtu"). While one should not get fragments smaller than
>> the IPv6 minimum MTU, not handling crafted packets in the TAHI IPv6
>> conformance test suite (v6eval) for IPv6 Ready Logo results in 18
>> failures representing over 5% of the score.
>>
>> Cc: Florian Westphal <fw@strlen.de>
>> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> Sorry, I'm not just going to blindly apply a patch because some
> TAHI tests fail.
>
> It's possible the TAHI tests are wrong, or that the specification
> elements it is testing don't make any sense these days.
>
> Allowing all kinds of random junk in the middle of the fragment queue
> leads to lots of unnecessary cpu overhead and potential bugs, and it
> triggerable remotely.
Understood, thank you.
It would be great if there is someone on this mailer who has influence
with ipv6ready.org so as to get the TAHI tests for IPv6 conformance
updated, as an upgrade to a kernel with the commit mentioned will result
in a 5% degradation in results for the existing tests.
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Jason A. Donenfeld @ 2018-10-03 14:28 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <CAKv+Gu-d07eqZTzh04dTtCjpnG-0fKsh7=k9BQYW_bx0Ju6gDw@mail.gmail.com>
On Wed, Oct 3, 2018 at 4:25 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> On Wed, Oct 3, 2018 at 1:15 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >>> > +config WIREGUARD
> >>> > + tristate "WireGuard secure network tunnel"
> >>> > + depends on NET && INET
> >>>
> >>> I think you need to add IPV6 here
> >>
> >> Nope. Like much of the net tree, WireGuard can function on ipv6-less
> >> kernels. If you do find something in WireGuard isn't working in a
> >> v6-less configuration, I consider that to be a bug that needs fixing.
> >>
> >
> > OK. I hit a build error yesterday, and setting CONFIG_IPV6 fixed it.
> > Let me see if I can reproduce.
>
> I get
>
> drivers/net/wireguard/socket.o: In function `send6':
> socket.c:(.text+0x56c): undefined reference to `ipv6_chk_addr'
> drivers/net/wireguard/socket.o: In function `wg_socket_send_skb_to_peer':
> socket.c:(.text+0x904): undefined reference to `ipv6_chk_addr'
> drivers/net/wireguard/socket.o: In function `wg_socket_init':
> socket.c:(.text+0x161c): undefined reference to `ipv6_mod_enabled'
>
> if I build my kernel with WireGuard built in but IPv6 support enabled
> as a module.
Nice catch. Looks like the norm is adding "depends on IPV6 || !IPV6"
or the like. I'll play and make sure this works for v7. Thanks for
pointing it out.
Jason
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Ard Biesheuvel @ 2018-10-03 14:25 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <CAKv+Gu9dOLbiBYKp9HG+GSPML0OJ-UuXnGsJO_re2S75ZF_qNA@mail.gmail.com>
On 3 October 2018 at 16:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 3 October 2018 at 16:12, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> Hi Ard,
>>
>> On Wed, Oct 3, 2018 at 1:15 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> > +config WIREGUARD
>>> > + tristate "WireGuard secure network tunnel"
>>> > + depends on NET && INET
>>>
>>> I think you need to add IPV6 here
>>
>> Nope. Like much of the net tree, WireGuard can function on ipv6-less
>> kernels. If you do find something in WireGuard isn't working in a
>> v6-less configuration, I consider that to be a bug that needs fixing.
>>
>
> OK. I hit a build error yesterday, and setting CONFIG_IPV6 fixed it.
> Let me see if I can reproduce.
I get
drivers/net/wireguard/socket.o: In function `send6':
socket.c:(.text+0x56c): undefined reference to `ipv6_chk_addr'
drivers/net/wireguard/socket.o: In function `wg_socket_send_skb_to_peer':
socket.c:(.text+0x904): undefined reference to `ipv6_chk_addr'
drivers/net/wireguard/socket.o: In function `wg_socket_init':
socket.c:(.text+0x161c): undefined reference to `ipv6_mod_enabled'
if I build my kernel with WireGuard built in but IPv6 support enabled
as a module.
^ permalink raw reply
* Re: [PATCH] xfrm: fix gro_cells leak when remove virtual xfrm interfaces
From: Steffen Klassert @ 2018-10-03 7:27 UTC (permalink / raw)
To: Li RongQing; +Cc: netdev
In-Reply-To: <1538291166-14645-1-git-send-email-lirongqing@baidu.com>
On Sun, Sep 30, 2018 at 03:06:06PM +0800, Li RongQing wrote:
> The device gro_cells has been initialized, it should be freed,
> otherwise it will be leaked
>
> Fixes: f203b76d78092faf2 ("xfrm: Add virtual xfrm interfaces")
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Applied, thanks a lot!
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Ard Biesheuvel @ 2018-10-03 14:13 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <CAHmME9rWkLT=92QVhbYZd1oee5wYtVRnYu0BGHkGn3bXjN-OxQ@mail.gmail.com>
On 3 October 2018 at 16:12, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hi Ard,
>
> On Wed, Oct 3, 2018 at 1:15 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > +config WIREGUARD
>> > + tristate "WireGuard secure network tunnel"
>> > + depends on NET && INET
>>
>> I think you need to add IPV6 here
>
> Nope. Like much of the net tree, WireGuard can function on ipv6-less
> kernels. If you do find something in WireGuard isn't working in a
> v6-less configuration, I consider that to be a bug that needs fixing.
>
OK. I hit a build error yesterday, and setting CONFIG_IPV6 fixed it.
Let me see if I can reproduce.
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Jason A. Donenfeld @ 2018-10-03 14:12 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <CAKv+Gu-oCH4D_otvMk+R_6z_p73Le90PkdQUUPnT6q0kWVL4Jw@mail.gmail.com>
Hi Ard,
On Wed, Oct 3, 2018 at 1:15 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > +config WIREGUARD
> > + tristate "WireGuard secure network tunnel"
> > + depends on NET && INET
>
> I think you need to add IPV6 here
Nope. Like much of the net tree, WireGuard can function on ipv6-less
kernels. If you do find something in WireGuard isn't working in a
v6-less configuration, I consider that to be a bug that needs fixing.
Jason
^ permalink raw reply
* Re: [PATCH net-next v6 11/23] zinc: import Andy Polyakov's Poly1305 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-10-03 14:08 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Eric Biggers, Andy Polyakov, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAKv+Gu8-HoAEyYiPYqMOMy9MGduFLQik2YxdJziqxN_cUs0nCA@mail.gmail.com>
Hi Ard,
On Wed, Oct 3, 2018 at 9:58 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> it has changed. This awkward commit splitting has been requested for the
> >> ARM[64] implementations in particular.
> >>
>
> "This awkward commit splitting"
Awkward in the sense that only those two commits are doing it, whereas
the rest of the series is not. Not awkward in any other sense that you
seemed to have divined based on your oversized punctuation below.
Fortunately, at your suggestion for v7, I've now done the splitting
for all of the other places, and so I the comment in the dev tree a
few days ago, since it's now done consistently across the patchset.
>
> Seriously?!?
>
> So you really think it is fine to import huge chunks of code like this
> from other projects without keeping track of the local changes?
As explained above, that's not what I meant at all.
Jason
^ permalink raw reply
* [PATCH] net: wireless: iwlegacy: Fix possible data races in il4965_send_rxon_assoc()
From: Jia-Ju Bai @ 2018-10-03 14:07 UTC (permalink / raw)
To: sgruszka, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, Jia-Ju Bai
CPU0:
il4965_configure_filter
mutex_lock()
line 6183: il->staging.filter_flags &= ... [WRITE]
line 6184: il->staging.filter_flags |= ... [WRITE]
CPU1:
il4965_send_rxon_assoc
line 1301: rxon1->filter_flags, rxon1->filter_flags [READ]
line 1314: il->staging.filter_flags [READ]
The WRITE operations in CPU0 are performed with holding a mutex lock,
but the READ operations in CPU1 are performed without holding this lock,
so there may exist data races.
These possible races are detected by a runtime testing.
To fix these races, the mutex lock is used in il4965_send_rxon_assoc()
to protect the data.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/wireless/intel/iwlegacy/4965.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/intel/iwlegacy/4965.c b/drivers/net/wireless/intel/iwlegacy/4965.c
index c3c638ed0ed7..45342777a5f1 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965.c
@@ -1297,6 +1297,7 @@ il4965_send_rxon_assoc(struct il_priv *il)
const struct il_rxon_cmd *rxon1 = &il->staging;
const struct il_rxon_cmd *rxon2 = &il->active;
+ mutex_lock(&il->mutex);
if (rxon1->flags == rxon2->flags &&
rxon1->filter_flags == rxon2->filter_flags &&
rxon1->cck_basic_rates == rxon2->cck_basic_rates &&
@@ -1307,6 +1308,7 @@ il4965_send_rxon_assoc(struct il_priv *il)
rxon1->rx_chain == rxon2->rx_chain &&
rxon1->ofdm_basic_rates == rxon2->ofdm_basic_rates) {
D_INFO("Using current RXON_ASSOC. Not resending.\n");
+ mutex_unlock(&il->mutex);
return 0;
}
@@ -1321,6 +1323,8 @@ il4965_send_rxon_assoc(struct il_priv *il)
il->staging.ofdm_ht_dual_stream_basic_rates;
rxon_assoc.rx_chain_select_flags = il->staging.rx_chain;
+ mutex_unlock(&il->mutex);
+
ret =
il_send_cmd_pdu_async(il, C_RXON_ASSOC, sizeof(rxon_assoc),
&rxon_assoc, NULL);
--
2.17.0
^ permalink raw reply related
* Re: [PATCH net-next v6 21/23] crypto: port ChaCha20 to Zinc
From: Jason A. Donenfeld @ 2018-10-03 14:01 UTC (permalink / raw)
To: Eric Biggers
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <20181003055559.GA745@sol.localdomain>
Hi Eric,
On Wed, Oct 3, 2018 at 7:56 AM Eric Biggers <ebiggers@kernel.org> wrote:
> Multiple threads may use the same tfm concurrently, so the tfm context must not
> be used to store per-request information such as the IV.
Thanks, fixed for v7.
Jason
^ permalink raw reply
* [REPORT] net: 3com: 3c59x: Possible data races
From: Jia-Ju Bai @ 2018-10-03 13:52 UTC (permalink / raw)
To: klassert, davem, anna-maria, bigeasy, nhorman, keescook
Cc: netdev, Linux Kernel Mailing List
****** Possible race0 ******
CPU0:
vortex_boomerang_interrupt
line 2510: spin_lock_irqsave()
_boomerang_interrupt
line 2432: vp->tx_skbuff[entry] [READ]
line 2433: vp->tx_skbuff[entry] [READ]
line 2453: vp->tx_skbuff[entry] = NULL [WRITE]
CPU1:
boomerang_start_xmit
line 2145: vp->tx_skbuff[entry] = skb [WRITE]
As for vp->tx_skbuff[entry], the WRITE and READ operations in CPU0
are performed with holding a spinlock, but the WRITE operation in CPU1
is performed without holding this spinlock, so there may exist data races.
****** Possible race1 ******
CPU0:
vortex_boomerang_interrupt
line 2510: spin_lock_irqsave()
_boomerang_interrupt
line 2421: vp->dirty_tx = dirty_tx [WRITE]
CPU1:
boomerang_start_xmit
line 2137: vp->dirty_tx [READ]
As for vp->dirty_tx, the WRITE operation in CPU0 is performed
with holding a spinlock, but the READ operation in CPU1 is performed
without holding this spinlock, so there may exist a data race.
****** Possible race2 ******
CPU0:
vortex_boomerang_interrupt
line 2510: spin_lock_irqsave()
_boomerang_interrupt
line 2381: vp->handling_irq = 1 [WRITE]
line 2498: vp->handling_irq = 0 [WRITE]
CPU1:
boomerang_start_xmit
line 2134: vp->handling_irq [READ]
As for vp->handling_irq, the WRITE operations in CPU0 are performed
with holding a spinlock, but the READ operation in CPU1 is performed
without holding this spinlock, so there may exist data races.
****** Possible race3 ******
CPU0:
vortex_boomerang_interrupt
line 2510: spin_lock_irqsave()
_boomerang_interrupt
boomerang_rx
line 2669: skb->ip_summed = ... [WRITE]
CPU1:
boomerang_start_xmit
line 2149: skb->ip_summed [READ]
As for skb->ip_summed, the WRITE operation in CPU0 is performed
with holding a spinlock, but the READ operation in CPU1 is performed
without holding this spinlock, so there may exist data races.
These possible races are detected by a runtime testing.
A possible fix of these races is protecting the code in
boomerang_start_xmit()
using the spinlock in vortex_boomerang_interrupt().
But I am not sure whether this fix is correct, so I only report these races.
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* RE: [PATCH] usbnet: smsc95xx: simplify tx_fixup code
From: David Laight @ 2018-10-03 13:36 UTC (permalink / raw)
To: 'Ben Dooks', netdev@vger.kernel.org
Cc: oneukum@suse.com, davem@davemloft.net, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kernel@lists.codethink.co.uk
In-Reply-To: <20181002165602.21033-1-ben.dooks@codethink.co.uk>
From: Ben Dooks
> Sent: 02 October 2018 17:56
>
> The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
> put an 8-byte command header onto the packet. It would be easier
> to do one skb_push() and then copy the data in once the push is
> done.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> drivers/net/usb/smsc95xx.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index cb19aea139d3..813ab93ee2c3 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
> bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
> int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
> u32 tx_cmd_a, tx_cmd_b;
> + void *ptr;
It might be useful to define a structure for the header.
You might need to find the 'store unaligned 32bit word' macro though.
(Actually that will probably be better than the memcpy() which might
end up doing memory-memory copies rather than storing the register.)
Although if/when you add the tx alignment that won't be needed because the
header will be aligned.
> /* We do not advertise SG, so skbs should be already linearized */
> BUG_ON(skb_shinfo(skb)->nr_frags);
> @@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
> return NULL;
> }
>
> + tx_cmd_b = (u32)skb->len;
> + tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
> +
> if (csum) {
> if (skb->len <= 45) {
> /* workaround - hardware tx checksum does not work
> @@ -2035,21 +2039,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
> skb_push(skb, 4);
> cpu_to_le32s(&csum_preamble);
Not related, but csum_preamble = cpu_to_le32(csum_preamble) is likely to
generate better code (at least for some architectures).
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] net: arp, ipv6: handle special case of tap device
From: Vladis Dronov @ 2018-10-03 13:25 UTC (permalink / raw)
To: David Miller; +Cc: kuznet, yoshfuji, netdev, syzkaller, linux-kernel
In-Reply-To: <20181002.222540.1948082296309410659.davem@davemloft.net>
> From: "David Miller" <davem@davemloft.net>
> Sent: Wednesday, October 3, 2018 7:25:40 AM
>
> > Probably, this functionality is already used by some userspace, so I
> > believe, I cannot just remove TUNSETLINK ioctl.
>
> Who uses it, how do they use it, and what do they sanely expect about
> the link layer header properties of a tap device which has had ->type
> changed?
Honestly, I have no idea. I was working on a syzkaller bug report and
found that the reason for that bug was TUNSETLINK ioctl.
This ioctl was introduced 13 years ago with ff4cc3ac93e1d and it says:
> For use with
> wireless and other networking types it should be possible to set the
> ARP type via an ioctl.
, that's all. Let me try to prepare a patch that sets dev->addr_len and
dev->broadcast according to dev->type. I would probably need an advise
what other fields must be adjusted too.
Otherwise, we can just to remove the TUNSETLINK ioctl.
Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security Engineer
^ permalink raw reply
* [REPORT] net: e1000e: possible data races
From: Jia-Ju Bai @ 2018-10-03 13:18 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem
Cc: intel-wired-lan, netdev, Linux Kernel Mailing List
CPU0:
e1000e_get_stats64
line 5944: spin_lock()
e1000e_update_stats
line 4949: hw->mac.tx_packet_delta = er32(TPT); [WRITE]
CPU1:
e1000_watchdog_task
e1000e_update_adaptive
line 1762: mac->tx_packet_delta [READ]
line 1763: mac->tx_packet_delta [READ]
line 1776: mac->tx_packet_delta [READ]
As for mac->tx_packet_delta, the WRITE operation in CPU0 is performed
with holding a spinlock, but the READ operations in CPU1 are performed
without holding this spinlock, so there may exist data races.
These possible races are detected by a runtime testing.
I do not find a good way to fix these races, so I only report them.
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* [REPORT] net: e100: possible data races
From: Jia-Ju Bai @ 2018-10-03 13:03 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem
Cc: intel-wired-lan, netdev, Linux Kernel Mailing List
CPU0:
e100_set_multicast_list
e100_exec_cb
line 854: spin_lock_irqsave()
line 866: e100_configure (function pointer)
line 1114: nic->flags [READ]
line 1123: nic->flags [READ]
CPU1:
e100_watchdog
line 1731: nic->flags [WRITE]
line 1733: nic->flags [WRITE]
As for nic->flags, the WRITE operations in CPU0 are performed
with holding a spinlock, but the READ operations in CPU1 are performed
without holding this spinlock, so there may exist data races.
These possible races are detected by a runtime testing.
I do not find a good way to fix these races, so I only report them.
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH net-next] tls: Add support for inplace records encryption
From: David Miller @ 2018-10-03 6:04 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
In-Reply-To: <20180930023435.22864-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Sun, 30 Sep 2018 08:04:35 +0530
> Presently, for non-zero copy case, separate pages are allocated for
> storing plaintext and encrypted text of records. These pages are stored
> in sg_plaintext_data and sg_encrypted_data scatterlists inside record
> structure. Further, sg_plaintext_data & sg_encrypted_data are passed
> to cryptoapis for record encryption. Allocating separate pages for
> plaintext and encrypted text is inefficient from both required memory
> and performance point of view.
>
> This patch adds support of inplace encryption of records. For non-zero
> copy case, we reuse the pages from sg_encrypted_data scatterlist to
> copy the application's plaintext data. For the movement of pages from
> sg_encrypted_data to sg_plaintext_data scatterlists, we introduce a new
> function move_to_plaintext_sg(). This function add pages into
> sg_plaintext_data from sg_encrypted_data scatterlists.
>
> tls_do_encryption() is modified to pass the same scatterlist as both
> source and destination into aead_request_set_crypt() if inplace crypto
> has been enabled. A new ariable 'inplace_crypto' has been introduced in
> record structure to signify whether the same scatterlist can be used.
> By default, the inplace_crypto is enabled in get_rec(). If zero-copy is
> used (i.e. plaintext data is not copied), inplace_crypto is set to '0'.
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: dsa: lantiq: Fix path in MAINTAINERS file
From: David Miller @ 2018-10-03 5:58 UTC (permalink / raw)
To: hauke; +Cc: netdev
In-Reply-To: <20180929112630.9872-1-hauke@hauke-m.de>
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 29 Sep 2018 13:26:30 +0200
> The MAINTAINERS file contained the wrong file name of the driver.
>
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Reported-by: Joe Perches <joe@perches.com>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied.
^ permalink raw reply
* Re: [PATCH] hv_netvsc: remove ndo_poll_controller
From: David Miller @ 2018-10-03 5:57 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin, haiyangz, edumazet
In-Reply-To: <20180929125256.25994-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sat, 29 Sep 2018 14:52:56 +0200
> Similar to other patches from ERic.
>
> As diagnosed by Song Liu, ndo_poll_controller() can
> be very dangerous on loaded hosts, since the cpu
> calling ndo_poll_controller() might steal all NAPI
> contexts (for all RX/TX queues of the NIC). This capture
> can last for unlimited amount of time, since one
> cpu is generally not able to drain all the queues under load.
>
> In netvsc driver it uses NAPI for TX completions. The default
> poll_napi will do this for us now and avoid the capture.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net-next] MAINTAINERS: Fix wrong include file path
From: David Miller @ 2018-10-03 5:56 UTC (permalink / raw)
To: andrew; +Cc: netdev
In-Reply-To: <1538251022-13348-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 29 Sep 2018 21:57:02 +0200
> Fix the patch for the mv88e6xxx.h header file in MAINTAINERS
>
> Reported-by: Joe Perches <joe@perches.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Applied.
^ permalink raw reply
* Re: [PATCH v2 08/22] soc/fsl/qbman_portals: add APIs to retrieve the probing status
From: Robin Murphy @ 2018-10-03 12:27 UTC (permalink / raw)
To: Laurentiu Tudor, Leo Li
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Netdev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Roy Pledge, Madalin-cristian Bucur, David Miller, Shawn Guo,
Bharat Bhushan
In-Reply-To: <c4ff9977-c730-be48-8046-97da663c5a23@nxp.com>
On 2018-10-03 11:50 AM, Laurentiu Tudor wrote:
> Hi Leo,
>
> On 27.09.2018 23:03, Li Yang wrote:
>> On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>>>
>>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>>
>>> Add a couple of new APIs to check the probing status of the required
>>> cpu bound qman and bman portals:
>>> 'int bman_portals_probed()' and 'int qman_portals_probed()'.
>>> They return the following values.
>>> * 1 if qman/bman portals were all probed correctly
>>> * 0 if qman/bman portals were not yet probed
>>> * -1 if probing of qman/bman portals failed
>>> Drivers that use qman/bman portal driver services are required to use
>>> these APIs before calling any functions exported by these drivers or
>>> otherwise they will crash the kernel.
>>> First user will be the dpaa1 ethernet driver, coming in a subsequent
>>> patch.
>>>
>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>> ---
>>> drivers/soc/fsl/qbman/bman_portal.c | 10 ++++++++++
>>> drivers/soc/fsl/qbman/qman_portal.c | 10 ++++++++++
>>> include/soc/fsl/bman.h | 8 ++++++++
>>> include/soc/fsl/qman.h | 9 +++++++++
>>> 4 files changed, 37 insertions(+)
>>>
>>> diff --git a/drivers/soc/fsl/qbman/bman_portal.c b/drivers/soc/fsl/qbman/bman_portal.c
>>> index f9edd28894fd..8048d35de8a2 100644
>>> --- a/drivers/soc/fsl/qbman/bman_portal.c
>>> +++ b/drivers/soc/fsl/qbman/bman_portal.c
>>> @@ -32,6 +32,7 @@
>>>
>>> static struct bman_portal *affine_bportals[NR_CPUS];
>>> static struct cpumask portal_cpus;
>>> +static int __bman_portals_probed;
>>> /* protect bman global registers and global data shared among portals */
>>> static DEFINE_SPINLOCK(bman_lock);
>>>
>>> @@ -85,6 +86,12 @@ static int bman_online_cpu(unsigned int cpu)
>>> return 0;
>>> }
>>>
>>> +int bman_portals_probed(void)
>>> +{
>>> + return __bman_portals_probed;
>>> +}
>>> +EXPORT_SYMBOL_GPL(bman_portals_probed);
>>> +
>>> static int bman_portal_probe(struct platform_device *pdev)
>>> {
>>> struct device *dev = &pdev->dev;
>>> @@ -148,6 +155,7 @@ static int bman_portal_probe(struct platform_device *pdev)
>>> spin_lock(&bman_lock);
>>> cpu = cpumask_next_zero(-1, &portal_cpus);
>>> if (cpu >= nr_cpu_ids) {
>>> + __bman_portals_probed = 1;
>>
>> What if the last CPU is not used for portals? Is there a hard
>> requirement that all CPUs need to be used for portal?
>
> As far as I know, in the current driver design a portal is required for
> each CPU.
>
>> What happens if the last CPU is offline?
>
> Can this happen at probe time?
As I understand things, yes - if you boot with "maxcpus=1", only the
boot CPU will be onlined by the kernel, but the others can still be
brought up manually by userspace later.
Robin.
> Anyway, I'm not sure that the driver is even aware of cpu hotplug but
> I'll let Roy comment on this one.
>
>>
>>> /* unassigned portal, skip init */
>>> spin_unlock(&bman_lock);
>>> return 0;
>>> @@ -173,6 +181,8 @@ static int bman_portal_probe(struct platform_device *pdev)
>>> err_ioremap2:
>>> memunmap(pcfg->addr_virt_ce);
>>> err_ioremap1:
>>> + __bman_portals_probed = 1;
>>> +
>>
>> There are other error paths that not covered.
>
> Right, thanks for pointing. On top of that, the assigned value here
> should be -1 to signal error (instead of 1 which signals success).
>
> ---
> Best Regards, Laurentiu
>
^ 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