* RE: [PATCH] usbnet: smsc95xx: simplify tx_fixup code
From: Ben Dooks @ 2018-10-03 16:25 UTC (permalink / raw)
To: David Laight
Cc: netdev, oneukum, davem, linux-usb, linux-kernel, linux-kernel
In-Reply-To: <55e4b55bae6148b18a27a23aab7d7501@AcuMS.aculab.com>
On 2018-10-03 14:36, David Laight wrote:
> 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.
Ok, might be worth doing.
I did try to do a "u32 tx_cmd[2]" but the code generated ended up
storing
stuff onto the stack before copying into the packet. I agree that
possibly
going to the "put_unaligned" function might be nicer too.
If we did enable tx-align all the time then we'd not have to care about
the
alignment, but I didn't want to do that if possible as that would end up
sending up to 3 bytes extra per packet.
I am trying not too do too many changes at one time to allow roll back.
>> /* 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
* Kernel oops with mlx5 and dual XDP redirect programs
From: Toke Høiland-Jørgensen @ 2018-10-03 9:30 UTC (permalink / raw)
To: Saeed Mahameed, netdev@vger.kernel.org
Cc: brouer, Tariq Toukan, Eran Ben Elisha
Hi Saeed
I can reliably oops the kernel with the mlx5 driver, by installing
XDP_REDIRECT programs on two devices so they redirect to each other,
and then remove them while there is traffic on the interface.
Steps to reproduce:
# cd ~/build/linux/samples/bpf
# ./xdp_redirect_map $(</sys/class/net/ens1f1/ifindex) $(</sys/class/net/ens1f0/ifindex)
# ./xdp_redirect_map $(</sys/class/net/ens1f0/ifindex) $(</sys/class/net/ens1f1/ifindex)
Now, run some traffic (e.g., using pktgen) across the interfaces, and
while the traffic is running, interrupt one of the xdp_redirect_map
commands (thus unloading the eBPF program). This results in a kernel
oops with the backtrace below. I get no crash if there's only a single
XDP program.
Is this something you could look into, please? :)
-Toke
[ 1400.937870] BUG: unable to handle kernel paging request at 0000000000003fa8
[ 1400.944826] PGD 800000072cc7b067 P4D 800000072cc7b067 PUD 72cc7a067 PMD 0
[ 1400.951693] Oops: 0000 [#1] SMP PTI
[ 1400.955184] CPU: 5 PID: 10392 Comm: xdp_redirect_ma Not tainted 4.19.0-rc5-xdptest-g5be3ebf+ #17
[ 1400.965344] Hardware name: LENOVO 30B3005DMT/102F, BIOS S00KT56A 01/15/2018
[ 1400.972318] RIP: 0010:mlx5e_xdp_xmit+0x7b/0x2a0 [mlx5_core]
[ 1400.977889] Code: 8b 0d 29 d9 4f 3f 39 8f 48 39 00 00 b8 fa ff ff ff 0f 86 45 01 00 00 48 8b 87 40 39 00 00 48 63 c9 4c 8b 24 c8 b8 9c ff ff ff <49> 8b 8c 24 a8 3f 00 00 4d 8d bc 24 c0 3c 00 00 83 e1 01 0f 84 19
[ 1400.996624] RSP: 0018:ffff90209fb43bb0 EFLAGS: 00010202
[ 1401.002001] RAX: 00000000ffffff9c RBX: 0000000000000000 RCX: 0000000000000005
[ 1401.009122] RDX: ffffc7627fd75190 RSI: 0000000000000010 RDI: ffff902084580000
[ 1401.016250] RBP: ffffc7627fd75190 R08: ffff901f9821c100 R09: ffffc7627fd75210
[ 1401.023379] R10: 00000000000005dc R11: 0000000000000000 R12: 0000000000000000
[ 1401.030500] R13: ffff902081580000 R14: 0000000000000001 R15: ffffc7627fd75190
[ 1401.037645] FS: 00007f460fa96700(0000) GS:ffff90209fb40000(0000) knlGS:0000000000000000
[ 1401.045718] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1401.051452] CR2: 0000000000003fa8 CR3: 000000076c3b6006 CR4: 00000000003606e0
[ 1401.058573] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1401.065823] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1401.072943] Call Trace:
[ 1401.075390] <IRQ>
[ 1401.077409] bq_xmit_all+0x5e/0x160
[ 1401.080897] dev_map_enqueue+0x12e/0x140
[ 1401.084823] xdp_do_redirect+0x1a9/0x2a0
[ 1401.088756] mlx5e_xdp_handle+0x24f/0x2b0 [mlx5_core]
[ 1401.093821] ? resched_cpu+0x5f/0x70
[ 1401.097399] ? __xdp_return+0x189/0x400
[ 1401.101242] mlx5e_skb_from_cqe_linear+0xdd/0x180 [mlx5_core]
[ 1401.106987] mlx5e_handle_rx_cqe+0x43/0xe0 [mlx5_core]
[ 1401.112130] mlx5e_poll_rx_cq+0xcb/0x940 [mlx5_core]
[ 1401.117094] mlx5e_napi_poll+0xa6/0xc90 [mlx5_core]
[ 1401.121966] ? smp_reschedule_interrupt+0x16/0xd0
[ 1401.126789] ? reschedule_interrupt+0xf/0x20
[ 1401.131057] ? reschedule_interrupt+0xa/0x20
[ 1401.135321] net_rx_action+0x279/0x3d0
[ 1401.139071] __do_softirq+0xf2/0x28e
[ 1401.142651] irq_exit+0xb6/0xc0
[ 1401.145792] do_IRQ+0x52/0xd0
[ 1401.148785] common_interrupt+0xf/0xf
[ 1401.152445] </IRQ>
[ 1401.154559] RIP: 0010:mlx5e_open_channels+0x65e/0x1390 [mlx5_core]
[ 1401.160734] Code: 8b 00 48 05 a8 00 00 00 48 89 85 78 3c 00 00 48 8b 83 f8 8d 01 00 48 89 85 80 3c 00 00 48 8b 83 f0 8d 01 00 8b 80 a8 fb 03 00 <0f> c8 89 85 88 3c 00 00 41 0f b6 45 16 88 85 8c 3c 00 00 49 83 bd
[ 1401.179463] RSP: 0018:ffffa7628dd43808 EFLAGS: 00000282 ORIG_RAX: ffffffffffffffd4
[ 1401.187024] RAX: 0000000000080000 RBX: ffff9020845808c0 RCX: 0000000000000000
[ 1401.194325] RDX: ffffa7628dd43894 RSI: 0000000000000000 RDI: ffff901f8a0e0000
[ 1401.201463] RBP: ffff901f8a0d8000 R08: ffffe1799d283800 R09: 0000000000000008
[ 1401.208582] R10: 0000000000000000 R11: 0000000000000002 R12: 0000000000000000
[ 1401.215702] R13: ffff902084583940 R14: 0000000000000000 R15: 0000000000000000
[ 1401.222834] ? mlx5e_open_channels+0x5e1/0x1390 [mlx5_core]
[ 1401.228404] ? rcu_exp_wait_wake+0x550/0x550
[ 1401.232674] ? free_one_page+0x68/0x370
[ 1401.236519] mlx5e_open_locked+0x28/0xa0 [mlx5_core]
[ 1401.241491] mlx5e_xdp+0x2b2/0x300 [mlx5_core]
[ 1401.245936] dev_xdp_install+0x4c/0x70
[ 1401.249686] do_setlink+0xcdb/0xd10
[ 1401.253300] ? flat_send_IPI_allbutself+0x6c/0xa0
[ 1401.258003] ? __update_load_avg_se+0x20c/0x290
[ 1401.262530] rtnl_setlink+0x104/0x140
[ 1401.266189] rtnetlink_rcv_msg+0x269/0x310
[ 1401.270283] ? _cond_resched+0x16/0x40
[ 1401.274029] ? __kmalloc_node_track_caller+0x1dd/0x2a0
[ 1401.279162] ? rtnl_calcit.isra.32+0x110/0x110
[ 1401.283601] netlink_rcv_skb+0xdb/0x110
[ 1401.287437] netlink_unicast+0x18b/0x250
[ 1401.291359] netlink_sendmsg+0x2c7/0x3b0
[ 1401.295287] sock_sendmsg+0x30/0x40
[ 1401.298776] __sys_sendto+0xd8/0x150
[ 1401.302351] ? __sys_getsockname+0xac/0xc0
[ 1401.306448] ? netlink_setsockopt+0x2e/0x2b0
[ 1401.310718] ? __sys_setsockopt+0x7c/0xe0
[ 1401.314867] __x64_sys_sendto+0x24/0x30
[ 1401.318709] do_syscall_64+0x4f/0x100
[ 1401.322372] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1401.327420] RIP: 0033:0x7f460f3a83dd
[ 1401.330997] Code: 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 8b 05 7a 13 2c 00 85 c0 75 3e 45 31 c9 45 31 c0 4c 63 d1 48 63 ff b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 0b c3 66 2e 0f 1f 84 00 00 00 00 00 48 8b 15
[ 1401.349733] RSP: 002b:00007ffd28d23138 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[ 1401.357293] RAX: ffffffffffffffda RBX: ffffffffffffff90 RCX: 00007f460f3a83dd
[ 1401.364413] RDX: 000000000000002c RSI: 00007ffd28d23170 RDI: 0000000000000003
[ 1401.371533] RBP: 00007ffd28d231e0 R08: 0000000000000000 R09: 0000000000000000
[ 1401.378767] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000006
[ 1401.385895] R13: 00007ffd28d237f0 R14: 00007ffd28d23830 R15: 00007ffd28d2388c
[ 1401.393016] Modules linked in: rpcrdma ib_umad sunrpc ib_ipoib rdma_ucm mlx5_ib binfmt_misc ib_uverbs snd_hda_codec_hdmi intel_rapl sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp mlx5_core kvm_intel snd_hda_codec_realtek snd_hda_codec_generic kvm snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm e1000e uas irqbypass snd_timer crct10dif_pclmul snd mei_me usb_storage crc32_pclmul ghash_clmulni_intel wmi_bmof mei lpc_ich soundcore mlxfw pata_acpi mac_hid ib_iser rdma_cm iw_cm ib_cm ib_core configfs iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 raid10 raid456 libcrc32c async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 multipath linear nouveau video i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops
drm mxm_wmi
[ 1401.463638] aesni_intel aes_x86_64 crypto_simd cryptd glue_helper ahci libahci wmi
[ 1401.471289] CR2: 0000000000003fa8
[ 1401.474617] ---[ end trace 1a0d8962c7db30ed ]---
[ 1401.528487] RIP: 0010:mlx5e_xdp_xmit+0x7b/0x2a0 [mlx5_core]
[ 1401.534058] Code: 8b 0d 29 d9 4f 3f 39 8f 48 39 00 00 b8 fa ff ff ff 0f 86 45 01 00 00 48 8b 87 40 39 00 00 48 63 c9 4c 8b 24 c8 b8 9c ff ff ff <49> 8b 8c 24 a8 3f 00 00 4d 8d bc 24 c0 3c 00 00 83 e1 01 0f 84 19
[ 1401.552789] RSP: 0018:ffff90209fb43bb0 EFLAGS: 00010202
[ 1401.558012] RAX: 00000000ffffff9c RBX: 0000000000000000 RCX: 0000000000000005
[ 1401.565132] RDX: ffffc7627fd75190 RSI: 0000000000000010 RDI: ffff902084580000
[ 1401.572252] RBP: ffffc7627fd75190 R08: ffff901f9821c100 R09: ffffc7627fd75210
[ 1401.579371] R10: 00000000000005dc R11: 0000000000000000 R12: 0000000000000000
[ 1401.586493] R13: ffff902081580000 R14: 0000000000000001 R15: ffffc7627fd75190
[ 1401.593726] FS: 00007f460fa96700(0000) GS:ffff90209fb40000(0000) knlGS:0000000000000000
[ 1401.601797] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1401.607533] CR2: 0000000000003fa8 CR3: 000000076c3b6006 CR4: 00000000003606e0
[ 1401.614653] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1401.621772] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1401.628895] Kernel panic - not syncing: Fatal exception in interrupt
[ 1401.635280] Kernel Offset: 0x5000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[ 1401.694263] Rebooting in 5 seconds..
^ permalink raw reply
* Re: [PATCH net-next] netfilter: xt_quota: fix the behavior of xt_quota module
From: Pablo Neira Ayuso @ 2018-10-03 9:28 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: Chenbo Feng, Linux NetDev, netfilter-devel, kernel-team,
Lorenzo Colitti, Chenbo Feng
In-Reply-To: <CANP3RGdRD5cQGRthBSx0HDVesf7aSBj0gxBwGxvuXrhZfKfYmw@mail.gmail.com>
On Wed, Oct 03, 2018 at 02:26:05AM -0700, Maciej Żenczykowski wrote:
> While on a 32-bit arch the tail pointer is 32 bits, the struct itself
> contains (and has always contained) a 64-bit value 'quota' with a
> forced 64-bit alignment which forces 64-bit alignment of the entire
> struct and thus also forces tail padding to make it a multiple of
> 64-bits. ie. size doesn't change - even on 32 bits.
>
> Chenbo was testing compatibility on both 32 and 64 bit x86 VMs.
Great! Hopefully this is my last question. Thanks for addressing them all.
^ permalink raw reply
* Re: [PATCH net-next] netfilter: xt_quota: fix the behavior of xt_quota module
From: Maciej Żenczykowski @ 2018-10-03 9:26 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Chenbo Feng, Linux NetDev, netfilter-devel, kernel-team,
Lorenzo Colitti, Chenbo Feng
In-Reply-To: <20181003091941.cbaceffk3uwrqwjo@salvia>
While on a 32-bit arch the tail pointer is 32 bits, the struct itself
contains (and has always contained) a 64-bit value 'quota' with a
forced 64-bit alignment which forces 64-bit alignment of the entire
struct and thus also forces tail padding to make it a multiple of
64-bits. ie. size doesn't change - even on 32 bits.
Chenbo was testing compatibility on both 32 and 64 bit x86 VMs.
^ permalink raw reply
* Re: [PATCH net-next] netfilter: xt_quota: fix the behavior of xt_quota module
From: Pablo Neira Ayuso @ 2018-10-03 9:19 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, netfilter-devel, kernel-team, Lorenzo Colitti, maze,
Chenbo Feng
In-Reply-To: <1538443388-6881-3-git-send-email-chenbofeng.kernel@gmail.com>
On Mon, Oct 01, 2018 at 06:23:08PM -0700, Chenbo Feng wrote:
[...]
> diff --git a/include/uapi/linux/netfilter/xt_quota.h b/include/uapi/linux/netfilter/xt_quota.h
> index f3ba5d9..d72fd52 100644
> --- a/include/uapi/linux/netfilter/xt_quota.h
> +++ b/include/uapi/linux/netfilter/xt_quota.h
> @@ -15,9 +15,11 @@ struct xt_quota_info {
> __u32 flags;
> __u32 pad;
> __aligned_u64 quota;
> -
> - /* Used internally by the kernel */
> - struct xt_quota_priv *master;
> +#ifdef __KERNEL__
> + atomic64_t counter;
> +#else
> + __aligned_u64 remain;
> +#endif
> };
Sorry, just noticed, one more question though:
Would this break backward compatibility with existing iptables 32-bits
binaries?
New kernel will hit size mismatch given that master pointer area used
to be 32-bits but now it is 64-bits long?
Let me know, thanks!
^ permalink raw reply
* 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
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