* Re: Get rid of RCU callbacks in TC filters?
From: Cong Wang @ 2017-10-20 3:26 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Jamal Hadi Salim, Chris Mi, Linux Kernel Network Developers,
Daniel Borkmann, Eric Dumazet, David Miller, Jiri Pirko
In-Reply-To: <20171018193548.GM3521@linux.vnet.ibm.com>
On Wed, Oct 18, 2017 at 12:35 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Oct 18, 2017 at 10:36:28AM -0700, Cong Wang wrote:
>> Hi, all
>>
>> Recently, the RCU callbacks used in TC filters and TC actions keep
>> drawing my attention, they introduce at least 4 race condition bugs:
>>
>> 1. A simple one fixed by Daniel:
>>
>> commit c78e1746d3ad7d548bdf3fe491898cc453911a49
>> Author: Daniel Borkmann <daniel@iogearbox.net>
>> Date: Wed May 20 17:13:33 2015 +0200
>>
>> net: sched: fix call_rcu() race on classifier module unloads
>>
>> 2. A very nasty one fixed by me:
>>
>> commit 1697c4bb5245649a23f06a144cc38c06715e1b65
>> Author: Cong Wang <xiyou.wangcong@gmail.com>
>> Date: Mon Sep 11 16:33:32 2017 -0700
>>
>> net_sched: carefully handle tcf_block_put()
>>
>> 3. Two more bugs found by Chris:
>> https://patchwork.ozlabs.org/patch/826696/
>> https://patchwork.ozlabs.org/patch/826695/
>>
>>
>> Usually RCU callbacks are simple, however for TC filters and actions,
>> they are complex because at least TC actions could be destroyed
>> together with the TC filter in one callback. And RCU callbacks are
>> invoked in BH context, without locking they are parallel too. All of
>> these contribute to the cause of these nasty bugs. It looks like they
>> bring us more problems than benefits.
>>
>> Therefore, I have been thinking about getting rid of these callbacks,
>> because they are not strictly necessary, callers of these call_rcu()
>> are all on slow path and have RTNL lock, so blocking is permitted in
>> their contexts, and _I think_ it does not harm to use
>> synchronize_rcu() on slow paths, at least I can argue RTNL lock is
>> already there and is a bottleneck if we really care. :)
>>
>> There are 3 solutions here:
>>
>> 1) Get rid of these RCU callbacks and use synchronize_rcu(). The
>> downside is this could hurt the performance of deleting TC filters,
>> but again it is slow path comparing to skb classification path. Note,
>> it is _not_ merely replacing call_rcu() with synchronize_rcu(),
>> because many call_rcu()'s are actually in list iterations, we have to
>> use a local list and call list_del_rcu()+list_add() before
>> synchronize_rcu() (Or is there any other API I am not aware of?). If
>> people really hate synchronize_rcu() because of performance, we could
>> also defer the work to a workqueue and callers could keep their
>> performance as they are.
>>
>> 2) Introduce a spinlock to serialize these RCU callbacks. But as I
>> said in commit 1697c4bb5245 ("net_sched: carefully handle
>> tcf_block_put()"), it is very hard to do because of tcf_chain_dump().
>> Potentially we need to do a lot of work to make it possible, if not
>> impossible.
>>
>> 3) Keep these RCU callbacks and fix all race conditions. Like what
>> Chris tries to do in his patchset, but my argument is that we can not
>> prove we are really race-free even with Chris' patches and his patches
>> are already large enough.
>>
>>
>> What do you think? Any other ideas?
>
> 4) Move from call_rcu() to synchronize_rcu(), but if feasible use one
> synchronize_rcu() for multiple deletions/iterations.
This is what I meant by using a local list, perhaps I didn't make it clear.
>
> 5) Keep call_rcu(), but have the RCU callback schedule a workqueue.
> The workqueue could then use blocking primitives, for example, acquiring
> RTNL.
Yeah, this could work too but we would get one more async...
filter delete -> call_rcu() -> schedule_work() -> action destroy
>
> 6) As with #5, have the RCU callback schedule a workqueue, but aggregate
> workqueue scheduling using a timer. This would reduce the number of
> RTNL acquisitions.
Ouch, sounds like even one more async:
filter delete -> call_rcu() -> schedule_work() -> timer -> flush_work()
-> action destroy
:-(
>
> 7) As with #5, have the RCU callback schedule a workqueue, but have each
> iterator accumulate a list of things removed and do call_rcu() on the
> list. This is an alternative way of aggregating to reduce the number
> of RTNL acquisitions.
Yeah, this seems working too.
>
> There are many other ways to skin this cat.
>
We still have to pick one. :) Any preference? I want to keep it as simple
as possible, otherwise some day I would not understand it either.
Thanks for all the ideas!
^ permalink raw reply
* Re: [RFC PATCH 1/5] security: Add support for SCTP security hooks
From: James Morris @ 2017-10-20 4:53 UTC (permalink / raw)
To: Richard Haines
Cc: selinux, netdev, linux-sctp, linux-security-module, paul,
vyasevich, nhorman, sds, eparis, marcelo.leitner
In-Reply-To: <20171017140247.4604-1-richard_c_haines@btinternet.com>
On Tue, 17 Oct 2017, Richard Haines wrote:
> The SCTP security hooks are explained in:
> Documentation/security/LSM-sctp.txt
>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> ---
> Documentation/security/LSM-sctp.txt | 212 ++++++++++++++++++++++++++++++++++++
> include/linux/lsm_hooks.h | 37 +++++++
> include/linux/security.h | 27 +++++
> security/security.c | 23 ++++
> 4 files changed, 299 insertions(+)
> create mode 100644 Documentation/security/LSM-sctp.txt
This looks ok from an LSM API pov, but note that I'm not an expert on
SCTP. It would be good to see more review from networking folk.
Reviewed-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply
* net-next: del_timer_sync(): possible circular locking dependency detected
From: Andrei Vagin @ 2017-10-20 5:06 UTC (permalink / raw)
To: Linux Kernel Network Developers; +Cc: Kees Cook
Hi,
We run criu tests on net-next/master regularly, and today tests
triggered this warning:
v4.14-rc4-1168-g7a0947e
[ 23.922640] ======================================================
[ 23.922735] WARNING: possible circular locking dependency detected
[ 23.922823] 4.14.0-rc4+ #1 Not tainted
[ 23.922910] ------------------------------------------------------
[ 23.922995] criu/1679 is trying to acquire lock:
[ 23.923081] ((timer)){+.-.}, at: [<ffffffff8e12b5d5>]
del_timer_sync+0x5/0xc0
[ 23.923186]
[ 23.923186] but task is already holding lock:
[ 23.923280] (slock-AF_INET){+.-.}, at: [<ffffffff8e93131f>]
sk_clone_lock+0x1af/0x580
[ 23.923380]
[ 23.923380] which lock already depends on the new lock.
[ 23.923380]
[ 23.923482]
[ 23.923482] the existing dependency chain (in reverse order) is:
[ 23.923576]
[ 23.923576] -> #1 (slock-AF_INET){+.-.}:
[ 23.923678] __lock_acquire+0x10fc/0x11a0
[ 23.923775] lock_acquire+0xed/0x1e0
[ 23.923865] _raw_spin_lock+0x2f/0x40
[ 23.923955] tcp_write_timer+0x29/0xd0
[ 23.924042] call_timer_fn+0x9b/0x330
[ 23.924131] run_timer_softirq+0x235/0x5f0
[ 23.924217] __do_softirq+0xd1/0x4a8
[ 23.924311] irq_exit+0xd4/0xe0
[ 23.924400] smp_apic_timer_interrupt+0xa1/0x2c0
[ 23.924491] apic_timer_interrupt+0x9d/0xb0
[ 23.924582] native_safe_halt+0x6/0x10
[ 23.924672] default_idle+0x23/0x1b0
[ 23.924768] arch_cpu_idle+0xf/0x20
[ 23.924858] default_idle_call+0x23/0x40
[ 23.924950] do_idle+0x177/0x200
[ 23.925040] cpu_startup_entry+0x1d/0x20
[ 23.925130] rest_init+0xc3/0xd0
[ 23.925222] start_kernel+0x43b/0x448
[ 23.925312] x86_64_start_reservations+0x24/0x26
[ 23.925403] x86_64_start_kernel+0x6f/0x72
[ 23.925493] verify_cpu+0x0/0xfb
[ 23.925582]
[ 23.925582] -> #0 ((timer)){+.-.}:
[ 23.925687] check_prev_add+0x401/0x800
[ 23.925782] __lock_acquire+0x10fc/0x11a0
[ 23.925874] lock_acquire+0xed/0x1e0
[ 23.925965] del_timer_sync+0x47/0xc0
[ 23.926055] inet_csk_reqsk_queue_drop+0xcc/0x1e0
[ 23.926146] inet_csk_complete_hashdance+0x23/0x80
[ 23.926237] tcp_check_req+0x3ec/0x510
[ 23.926326] tcp_v4_rcv+0x8ec/0xc20
[ 23.926425] ip_local_deliver_finish+0xdc/0x380
[ 23.926517] ip_local_deliver+0x66/0x200
[ 23.926605] ip_rcv_finish+0x1b7/0x530
[ 23.926688] ip_rcv+0x26c/0x4c0
[ 23.926779] __netif_receive_skb_core+0x74d/0xcc0
[ 23.926874] __netif_receive_skb+0x18/0x60
[ 23.926962] process_backlog+0x72/0x240
[ 23.927045] net_rx_action+0x1cb/0x3e0
[ 23.927125] __do_softirq+0xd1/0x4a8
[ 23.927207] do_softirq_own_stack+0x2a/0x40
[ 23.927288] do_softirq.part.16+0x46/0x70
[ 23.927370] __local_bh_enable_ip+0x9a/0xa0
[ 23.927452] ip_finish_output2+0x263/0x630
[ 23.927534] ip_finish_output+0x1ba/0x2e0
[ 23.927615] ip_output+0x73/0x240
[ 23.927705] ip_local_out+0x39/0x60
[ 23.927795] ip_queue_xmit+0x1ea/0x5c0
[ 23.927887] tcp_transmit_skb+0x551/0xaa0
[ 23.927979] tcp_send_ack+0xc8/0x130
[ 23.928071] tcp_rcv_state_process+0xe3d/0xe90
[ 23.928162] tcp_v4_do_rcv+0xbd/0x1d0
[ 23.928254] __release_sock+0x6d/0x110
[ 23.928346] release_sock+0x30/0xb0
[ 23.928438] __inet_stream_connect+0x187/0x320
[ 23.928531] inet_stream_connect+0x3b/0x60
[ 23.928620] SYSC_connect+0xbe/0xf0
[ 23.928712] SyS_connect+0xe/0x10
[ 23.928799] entry_SYSCALL_64_fastpath+0x23/0xc2
[ 23.928886]
[ 23.928886] other info that might help us debug this:
[ 23.928886]
[ 23.928988] Possible unsafe locking scenario:
[ 23.928988]
[ 23.929080] CPU0 CPU1
[ 23.929166] ---- ----
[ 23.929252] lock(slock-AF_INET);
[ 23.929342] lock((timer));
[ 23.929430] lock(slock-AF_INET);
[ 23.929517] lock((timer));
[ 23.929604]
[ 23.929604] *** DEADLOCK ***
[ 23.929604]
[ 23.929711] 5 locks held by criu/1679:
[ 23.929796] #0: (sk_lock-AF_INET){+.+.}, at: [<ffffffff8ea04627>]
inet_stream_connect+0x27/0x60
[ 23.929900] #1: (rcu_read_lock){....}, at: [<ffffffff8e9c4065>]
ip_queue_xmit+0x5/0x5c0
[ 23.930000] #2: (rcu_read_lock){....}, at: [<ffffffff8e94e464>]
process_backlog+0xd4/0x240
[ 23.930098] #3: (rcu_read_lock){....}, at: [<ffffffff8e9bd4ff>]
ip_local_deliver_finish+0x2f/0x380
[ 23.930200] #4: (slock-AF_INET){+.-.}, at: [<ffffffff8e93131f>]
sk_clone_lock+0x1af/0x580
[ 23.930297]
[ 23.930297] stack backtrace:
[ 23.930396] CPU: 1 PID: 1679 Comm: criu Not tainted 4.14.0-rc4+ #1
[ 23.930483] Hardware name: Google Google Compute Engine/Google
Compute Engine, BIOS Google 01/01/2011
[ 23.930581] Call Trace:
[ 23.930667] <IRQ>
[ 23.930766] dump_stack+0x85/0xc7
[ 23.930855] print_circular_bug+0x226/0x330
[ 23.930943] ? copy_trace+0xb0/0xb0
[ 23.931033] check_prev_add+0x401/0x800
[ 23.931122] ? tcp_v4_rcv+0x8ec/0xc20
[ 23.931212] __lock_acquire+0x10fc/0x11a0
[ 23.931294] ? __lock_acquire+0x10fc/0x11a0
[ 23.931381] ? copy_trace+0xb0/0xb0
[ 23.931468] ? inet_ehash_insert+0x14f/0x1d0
[ 23.931558] lock_acquire+0xed/0x1e0
[ 23.931648] ? del_timer_sync+0x5/0xc0
[ 23.931743] del_timer_sync+0x47/0xc0
[ 23.931829] ? del_timer_sync+0x5/0xc0
[ 23.931917] inet_csk_reqsk_queue_drop+0xcc/0x1e0
[ 23.932005] inet_csk_complete_hashdance+0x23/0x80
[ 23.932095] tcp_check_req+0x3ec/0x510
[ 23.932185] tcp_v4_rcv+0x8ec/0xc20
[ 23.932275] ip_local_deliver_finish+0xdc/0x380
[ 23.932366] ip_local_deliver+0x66/0x200
[ 23.932457] ? inet_del_offload+0x40/0x40
[ 23.932546] ip_rcv_finish+0x1b7/0x530
[ 23.932633] ip_rcv+0x26c/0x4c0
[ 23.932726] ? ip_local_deliver_finish+0x380/0x380
[ 23.932813] __netif_receive_skb_core+0x74d/0xcc0
[ 23.932899] __netif_receive_skb+0x18/0x60
[ 23.932985] ? __netif_receive_skb+0x18/0x60
[ 23.933070] process_backlog+0x72/0x240
[ 23.933157] net_rx_action+0x1cb/0x3e0
[ 23.933244] ? mark_held_locks+0x72/0xa0
[ 23.933333] __do_softirq+0xd1/0x4a8
[ 23.933418] ? ip_finish_output2+0x23a/0x630
[ 23.933503] do_softirq_own_stack+0x2a/0x40
[ 23.933585] </IRQ>
[ 23.933670] do_softirq.part.16+0x46/0x70
[ 23.933757] __local_bh_enable_ip+0x9a/0xa0
[ 23.933839] ip_finish_output2+0x263/0x630
[ 23.933926] ip_finish_output+0x1ba/0x2e0
[ 23.934014] ? ip_finish_output+0x1ba/0x2e0
[ 23.934101] ip_output+0x73/0x240
[ 23.934192] ? ip_fragment.constprop.54+0x80/0x80
[ 23.934284] ip_local_out+0x39/0x60
[ 23.934373] ip_queue_xmit+0x1ea/0x5c0
[ 23.934461] ? tcp_v4_md5_lookup+0x13/0x20
[ 23.934550] tcp_transmit_skb+0x551/0xaa0
[ 23.934639] tcp_send_ack+0xc8/0x130
[ 23.934735] tcp_rcv_state_process+0xe3d/0xe90
[ 23.934826] ? __local_bh_enable_ip+0x70/0xa0
[ 23.934911] tcp_v4_do_rcv+0xbd/0x1d0
[ 23.935000] ? tcp_v4_do_rcv+0xbd/0x1d0
[ 23.935089] __release_sock+0x6d/0x110
[ 23.935200] release_sock+0x30/0xb0
[ 23.935288] __inet_stream_connect+0x187/0x320
[ 23.935377] ? do_wait_intr_irq+0xc0/0xc0
[ 23.935464] inet_stream_connect+0x3b/0x60
[ 23.935557] SYSC_connect+0xbe/0xf0
[ 23.935642] ? sock_alloc_file+0x97/0x110
[ 23.935733] ? trace_hardirqs_on_caller+0x11f/0x190
[ 23.935821] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 23.935911] SyS_connect+0xe/0x10
[ 23.935999] entry_SYSCALL_64_fastpath+0x23/0xc2
[ 23.936086] RIP: 0033:0x7fb80b838720
[ 23.936170] RSP: 002b:00007ffc277dac88 EFLAGS: 00000246 ORIG_RAX:
000000000000002a
[ 23.936266] RAX: ffffffffffffffda RBX: 00007ffc277dac18 RCX: 00007fb80b838720
[ 23.936353] RDX: 0000000000000010 RSI: 00007ffc277daca0 RDI: 0000000000000005
[ 23.936442] RBP: 00007ffc277dabf0 R08: 00007ffc277dac43 R09: 0000000000000004
[ 23.936528] R10: 00007ffc277daa50 R11: 0000000000000246 R12: 00007fb80bc52bc0
[ 23.936613] R13: 0000000000000001 R14: 0000000000000000 R15: 00007fb80bc6b1c8
Here are all logs https://travis-ci.org/avagin/linux/jobs/290283691
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Eric Dumazet @ 2017-10-20 5:34 UTC (permalink / raw)
To: Wei Wei
Cc: linux-arm-kernel, LKML, netdev, David Miller, Willem de Bruijn,
syzkaller
In-Reply-To: <FF5A6DF1-0E5F-4C7E-A31B-06ED33366039@gmail.com>
On Thu, Oct 19, 2017 at 8:13 PM, Wei Wei <dotweiba@gmail.com> wrote:
> Sry. Here it is.
>
> Unable to handle kernel paging request at virtual address ffff80005bfb81ed
> Mem abort info:
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000033
> CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000b366000
> [ffff80005bfb81ed] *pgd=00000000beff7003, *pud=00e8000080000711
> Internal error: Oops: 96000021 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 3 PID: 4725 Comm: syz-executor0 Not tainted 4.14.0-rc3 #3
> Hardware name: linux,dummy-virt (DT)
> task: ffff800074409e00 task.stack: ffff800033db0000
> PC is at __skb_clone (/./arch/arm64/include/asm/atomic_ll_sc.h:113 (discriminator 4) /net/core/skbuff.c:873 (discriminator 4))
> LR is at __skb_clone (/net/core/skbuff.c:861 (discriminator 4))
> pc : lr : pstate: 10000145
>
> sp : ffff800033db33d0
> x29: ffff800033db33d0 x28: ffff2000098ac378
> x27: ffff100006a860e1 x26: 1ffff000067b66b6
> x25: ffff8000743340a0 x24: ffff800035430708
> x23: ffff80005bfb80c9 x22: ffff800035430710
> x21: 0000000000000380 x20: ffff800035430640
> x19: ffff8000354312c0 x18: 0000000000000000
> x17: 00000000004af000 x16: ffff20000845e8c8
> x15: 000000001e518060 x14: 0000ffffd8316070
> x13: 0000ffffd8316090 x12: ffffffffffffffff
> x11: 1ffff00006a8626f x10: ffff100006a8626f
> x9 : dfff200000000000 x8 : 0082009000900608
> x7 : 0000000000000000 x6 : ffff800035431380
> x5 : ffff100006a86270 x4 : 0000000000000000
> x3 : 1ffff00006a86273 x2 : 0000000000000000
> x1 : 0000000000000100 x0 : ffff80005bfb81ed
> Process syz-executor0 (pid: 4725, stack limit = 0xffff800033db0000)
> Call trace:
> Exception stack(0xffff800033db3290 to 0xffff800033db33d0)
> 3280: ffff80005bfb81ed 0000000000000100
> 32a0: 0000000000000000 1ffff00006a86273 0000000000000000 ffff100006a86270
> 32c0: ffff800035431380 0000000000000000 0082009000900608 dfff200000000000
> 32e0: ffff100006a8626f 1ffff00006a8626f ffffffffffffffff 0000ffffd8316090
> 3300: 0000ffffd8316070 000000001e518060 ffff20000845e8c8 00000000004af000
> 3320: 0000000000000000 ffff8000354312c0 ffff800035430640 0000000000000380
> 3340: ffff800035430710 ffff80005bfb80c9 ffff800035430708 ffff8000743340a0
> 3360: 1ffff000067b66b6 ffff100006a860e1 ffff2000098ac378 ffff800033db33d0
> 3380: ffff200009705cfc ffff800033db33d0 ffff200009705f50 0000000010000145
> 33a0: ffff8000354312c0 ffff800035430640 0001000000000000 ffff800074334000
> 33c0: ffff800033db33d0 ffff200009705f50
> __skb_clone (/./arch/arm64/include/asm/atomic_ll_sc.h:113 (discriminator 4) /net/core/skbuff.c:873 (discriminator 4))
> skb_clone (/net/core/skbuff.c:1286)
> arp_rcv (/./include/linux/skbuff.h:1518 /net/ipv4/arp.c:946)
> __netif_receive_skb_core (/net/core/dev.c:1859 /net/core/dev.c:1874 /net/core/dev.c:4416)
> __netif_receive_skb (/net/core/dev.c:4466)
> netif_receive_skb_internal (/net/core/dev.c:4539)
> netif_receive_skb (/net/core/dev.c:4564)
> tun_get_user (/./include/linux/bottom_half.h:31 /drivers/net/tun.c:1219 /drivers/net/tun.c:1553)
> tun_chr_write_iter (/drivers/net/tun.c:1579)
> do_iter_readv_writev (/./include/linux/fs.h:1770 /fs/read_write.c:673)
> do_iter_write (/fs/read_write.c:952)
> vfs_writev (/fs/read_write.c:997)
> do_writev (/fs/read_write.c:1032)
> SyS_writev (/fs/read_write.c:1102)
> Exception stack(0xffff800033db3ec0 to 0xffff800033db4000)
> 3ec0: 0000000000000015 0000ffff829985e0 0000000000000001 0000ffff8299851c
> 3ee0: 0000ffff82999068 0000ffff82998f60 0000ffff82999650 0000000000000000
> 3f00: 0000000000000042 0000000000000036 0000000000406608 0000ffff82998400
> 3f20: 0000ffff82998f60 0000ffffd8316090 0000ffffd8316070 000000001e518060
> 3f40: 0000000000000000 00000000004af000 0000000000000000 0000000000000036
> 3f60: 0000000020004fca 0000000020000000 000000000046ccf0 0000000000000530
> 3f80: 000000000046cce8 00000000004ade98 0000000000000000 00000000395fa6f0
> 3fa0: 0000ffff82998f60 0000ffff82998560 0000000000431448 0000ffff82998520
> 3fc0: 000000000043145c 0000000080000000 0000000000000015 0000000000000042
> 3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> el0_svc_naked (/arch/arm64/kernel/entry.S:853)
> Code: f9406680 8b010000 91009000 f9800011 (885f7c01)
> All code
> ========
> 0: 80 66 40 f9 andb $0xf9,0x40(%rsi)
> 4: 00 00 add %al,(%rax)
> 6: 01 8b 00 90 00 91 add %ecx,-0x6eff7000(%rbx)
> c: 11 00 adc %eax,(%rax)
> e: 80 f9 01 cmp $0x1,%cl
> 11: 7c 5f jl 0x72
> 13:* 88 00 mov %al,(%rax) <-- trapping instruction
> 15: 00 00 add %al,(%rax)
> ...
>
> Code starting with the faulting instruction
> ===========================================
> 0: 01 7c 5f 88 add %edi,-0x78(%rdi,%rbx,2)
> 4: 00 00 add %al,(%rax)
> ...
> —[ end trace 261e7ac1458ccc0a ]---
>
I thought it was happening on arm64 ?
This is x86_64 disassembly :/
Thanks.
> Thanks,
> Wei
>
>> On 19 Oct 2017, at 10:53 PM, Eric Dumazet <edumazet@google.com> wrote:
>>
>> On Thu, Oct 19, 2017 at 7:16 PM, Wei Wei <dotweiba@gmail.com> wrote:
>>> Hi all,
>>>
>>> I have fuzzed v4.14-rc3 using syzkaller and found a bug similar to that one [1].
>>> But the call trace isn’t the same. The atomic_inc() might handle a corrupted
>>> skb_buff.
>>>
>>> The logs and config have been uploaded to my github repo [2].
>>>
>>> [1] https://lkml.org/lkml/2017/10/2/216
>>> [2] https://github.com/dotweiba/skb_clone_atomic_inc_bug
>>>
>>> Thanks,
>>> Wei
>>>
>>> Unable to handle kernel paging request at virtual address ffff80005bfb81ed
>>> Mem abort info:
>>> Exception class = DABT (current EL), IL = 32 bits
>>> SET = 0, FnV = 0
>>> EA = 0, S1PTW = 0
>>> Data abort info:
>>> ISV = 0, ISS = 0x00000033
>>> CM = 0, WnR = 0
>>> swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000b366000
>>> [ffff80005bfb81ed] *pgd=00000000beff7003, *pud=00e8000080000711
>>> Internal error: Oops: 96000021 [#1] PREEMPT SMP
>>> Modules linked in:
>>> CPU: 3 PID: 4725 Comm: syz-executor0 Not tainted 4.14.0-rc3 #3
>>> Hardware name: linux,dummy-virt (DT)
>>> task: ffff800074409e00 task.stack: ffff800033db0000
>>> PC is at __skb_clone+0x430/0x5b0
>>> LR is at __skb_clone+0x1dc/0x5b0
>>> pc : [<ffff200009705f50>] lr : [<ffff200009705cfc>] pstate: 10000145
>>> sp : ffff800033db33d0
>>> x29: ffff800033db33d0 x28: ffff2000098ac378
>>> x27: ffff100006a860e1 x26: 1ffff000067b66b6
>>> x25: ffff8000743340a0 x24: ffff800035430708
>>> x23: ffff80005bfb80c9 x22: ffff800035430710
>>> x21: 0000000000000380 x20: ffff800035430640
>>> x19: ffff8000354312c0 x18: 0000000000000000
>>> x17: 00000000004af000 x16: ffff20000845e8c8
>>> x15: 000000001e518060 x14: 0000ffffd8316070
>>> x13: 0000ffffd8316090 x12: ffffffffffffffff
>>> x11: 1ffff00006a8626f x10: ffff100006a8626f
>>> x9 : dfff200000000000 x8 : 0082009000900608
>>> x7 : 0000000000000000 x6 : ffff800035431380
>>> x5 : ffff100006a86270 x4 : 0000000000000000
>>> x3 : 1ffff00006a86273 x2 : 0000000000000000
>>> x1 : 0000000000000100 x0 : ffff80005bfb81ed
>>> Process syz-executor0 (pid: 4725, stack limit = 0xffff800033db0000)
>>> Call trace:
>>> Exception stack(0xffff800033db3290 to 0xffff800033db33d0)
>>> 3280: ffff80005bfb81ed 0000000000000100
>>> 32a0: 0000000000000000 1ffff00006a86273 0000000000000000 ffff100006a86270
>>> 32c0: ffff800035431380 0000000000000000 0082009000900608 dfff200000000000
>>> 32e0: ffff100006a8626f 1ffff00006a8626f ffffffffffffffff 0000ffffd8316090
>>> 3300: 0000ffffd8316070 000000001e518060 ffff20000845e8c8 00000000004af000
>>> 3320: 0000000000000000 ffff8000354312c0 ffff800035430640 0000000000000380
>>> 3340: ffff800035430710 ffff80005bfb80c9 ffff800035430708 ffff8000743340a0
>>> 3360: 1ffff000067b66b6 ffff100006a860e1 ffff2000098ac378 ffff800033db33d0
>>> 3380: ffff200009705cfc ffff800033db33d0 ffff200009705f50 0000000010000145
>>> 33a0: ffff8000354312c0 ffff800035430640 0001000000000000 ffff800074334000
>>> 33c0: ffff800033db33d0 ffff200009705f50
>>> [<ffff200009705f50>] __skb_clone+0x430/0x5b0
>>> [<ffff20000971520c>] skb_clone+0x164/0x2c8
>>> [<ffff2000098ac498>] arp_rcv+0x120/0x488
>>> [<ffff200009741878>] __netif_receive_skb_core+0x11e8/0x18c8
>>> [<ffff2000097479b0>] __netif_receive_skb+0x30/0x198
>>> [<ffff200009751fd8>] netif_receive_skb_internal+0x98/0x370
>>> [<ffff2000097522cc>] netif_receive_skb+0x1c/0x28
>>> [<ffff2000090730e0>] tun_get_user+0x12f0/0x2e40
>>> [<ffff200009074ddc>] tun_chr_write_iter+0xbc/0x140
>>> [<ffff200008457284>] do_iter_readv_writev+0x2d4/0x468
>>> [<ffff20000845a5a0>] do_iter_write+0x148/0x498
>>> [<ffff20000845aac0>] vfs_writev+0x118/0x250
>>> [<ffff20000845acbc>] do_writev+0xc4/0x1e8
>>> [<ffff20000845e8fc>] SyS_writev+0x34/0x48
>>> Exception stack(0xffff800033db3ec0 to 0xffff800033db4000)
>>> 3ec0: 0000000000000015 0000ffff829985e0 0000000000000001 0000ffff8299851c
>>> 3ee0: 0000ffff82999068 0000ffff82998f60 0000ffff82999650 0000000000000000
>>> 3f00: 0000000000000042 0000000000000036 0000000000406608 0000ffff82998400
>>> 3f20: 0000ffff82998f60 0000ffffd8316090 0000ffffd8316070 000000001e518060
>>> 3f40: 0000000000000000 00000000004af000 0000000000000000 0000000000000036
>>> 3f60: 0000000020004fca 0000000020000000 000000000046ccf0 0000000000000530
>>> 3f80: 000000000046cce8 00000000004ade98 0000000000000000 00000000395fa6f0
>>> 3fa0: 0000ffff82998f60 0000ffff82998560 0000000000431448 0000ffff82998520
>>> 3fc0: 000000000043145c 0000000080000000 0000000000000015 0000000000000042
>>> 3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>> Code: f9406680 8b010000 91009000 f9800011 (885f7c01)
>>> ---[ end trace 261e7ac1458ccc0a ]---
>>
>> Please provide proper file:line information in this trace.
>>
>> You can use scripts/decode_stacktrace.sh
>>
>> Thanks.
>
^ permalink raw reply
* Re: net-next: del_timer_sync(): possible circular locking dependency detected
From: Eric Dumazet @ 2017-10-20 6:07 UTC (permalink / raw)
To: Andrei Vagin; +Cc: Linux Kernel Network Developers, Kees Cook
In-Reply-To: <CANaxB-zFCh8wKtZF+xtP=hf+Ze3UXfAkqOmFxX2_h=-QpZc9CQ@mail.gmail.com>
On Thu, 2017-10-19 at 22:06 -0700, Andrei Vagin wrote:
> Hi,
>
> We run criu tests on net-next/master regularly, and today tests
> triggered this warning:
>
> v4.14-rc4-1168-g7a0947e
>
> [ 23.922640] ======================================================
> [ 23.922735] WARNING: possible circular locking dependency detected
> [ 23.922823] 4.14.0-rc4+ #1 Not tainted
> [ 23.922910] ------------------------------------------------------
> [ 23.922995] criu/1679 is trying to acquire lock:
> [ 23.923081] ((timer)){+.-.}, at: [<ffffffff8e12b5d5>]
> del_timer_sync+0x5/0xc0
> [ 23.923186]
> [ 23.923186] but task is already holding lock:
> [ 23.923280] (slock-AF_INET){+.-.}, at: [<ffffffff8e93131f>]
> sk_clone_lock+0x1af/0x580
> [ 23.923380]
> [ 23.923380] which lock already depends on the new lock.
> [ 23.923380]
> [ 23.923482]
> [ 23.923482] the existing dependency chain (in reverse order) is:
> [ 23.923576]
> [ 23.923576] -> #1 (slock-AF_INET){+.-.}:
> [ 23.923678] __lock_acquire+0x10fc/0x11a0
> [ 23.923775] lock_acquire+0xed/0x1e0
> [ 23.923865] _raw_spin_lock+0x2f/0x40
> [ 23.923955] tcp_write_timer+0x29/0xd0
> [ 23.924042] call_timer_fn+0x9b/0x330
> [ 23.924131] run_timer_softirq+0x235/0x5f0
> [ 23.924217] __do_softirq+0xd1/0x4a8
> [ 23.924311] irq_exit+0xd4/0xe0
> [ 23.924400] smp_apic_timer_interrupt+0xa1/0x2c0
> [ 23.924491] apic_timer_interrupt+0x9d/0xb0
> [ 23.924582] native_safe_halt+0x6/0x10
> [ 23.924672] default_idle+0x23/0x1b0
> [ 23.924768] arch_cpu_idle+0xf/0x20
> [ 23.924858] default_idle_call+0x23/0x40
> [ 23.924950] do_idle+0x177/0x200
> [ 23.925040] cpu_startup_entry+0x1d/0x20
> [ 23.925130] rest_init+0xc3/0xd0
> [ 23.925222] start_kernel+0x43b/0x448
> [ 23.925312] x86_64_start_reservations+0x24/0x26
> [ 23.925403] x86_64_start_kernel+0x6f/0x72
> [ 23.925493] verify_cpu+0x0/0xfb
> [ 23.925582]
> [ 23.925582] -> #0 ((timer)){+.-.}:
> [ 23.925687] check_prev_add+0x401/0x800
> [ 23.925782] __lock_acquire+0x10fc/0x11a0
> [ 23.925874] lock_acquire+0xed/0x1e0
> [ 23.925965] del_timer_sync+0x47/0xc0
> [ 23.926055] inet_csk_reqsk_queue_drop+0xcc/0x1e0
> [ 23.926146] inet_csk_complete_hashdance+0x23/0x80
> [ 23.926237] tcp_check_req+0x3ec/0x510
> [ 23.926326] tcp_v4_rcv+0x8ec/0xc20
> [ 23.926425] ip_local_deliver_finish+0xdc/0x380
> [ 23.926517] ip_local_deliver+0x66/0x200
> [ 23.926605] ip_rcv_finish+0x1b7/0x530
> [ 23.926688] ip_rcv+0x26c/0x4c0
> [ 23.926779] __netif_receive_skb_core+0x74d/0xcc0
> [ 23.926874] __netif_receive_skb+0x18/0x60
> [ 23.926962] process_backlog+0x72/0x240
> [ 23.927045] net_rx_action+0x1cb/0x3e0
> [ 23.927125] __do_softirq+0xd1/0x4a8
> [ 23.927207] do_softirq_own_stack+0x2a/0x40
> [ 23.927288] do_softirq.part.16+0x46/0x70
> [ 23.927370] __local_bh_enable_ip+0x9a/0xa0
> [ 23.927452] ip_finish_output2+0x263/0x630
> [ 23.927534] ip_finish_output+0x1ba/0x2e0
> [ 23.927615] ip_output+0x73/0x240
> [ 23.927705] ip_local_out+0x39/0x60
> [ 23.927795] ip_queue_xmit+0x1ea/0x5c0
> [ 23.927887] tcp_transmit_skb+0x551/0xaa0
> [ 23.927979] tcp_send_ack+0xc8/0x130
> [ 23.928071] tcp_rcv_state_process+0xe3d/0xe90
> [ 23.928162] tcp_v4_do_rcv+0xbd/0x1d0
> [ 23.928254] __release_sock+0x6d/0x110
> [ 23.928346] release_sock+0x30/0xb0
> [ 23.928438] __inet_stream_connect+0x187/0x320
> [ 23.928531] inet_stream_connect+0x3b/0x60
> [ 23.928620] SYSC_connect+0xbe/0xf0
> [ 23.928712] SyS_connect+0xe/0x10
> [ 23.928799] entry_SYSCALL_64_fastpath+0x23/0xc2
> [ 23.928886]
> [ 23.928886] other info that might help us debug this:
> [ 23.928886]
> [ 23.928988] Possible unsafe locking scenario:
> [ 23.928988]
> [ 23.929080] CPU0 CPU1
> [ 23.929166] ---- ----
> [ 23.929252] lock(slock-AF_INET);
> [ 23.929342] lock((timer));
> [ 23.929430] lock(slock-AF_INET);
> [ 23.929517] lock((timer));
> [ 23.929604]
> [ 23.929604] *** DEADLOCK ***
> [ 23.929604]
> [ 23.929711] 5 locks held by criu/1679:
> [ 23.929796] #0: (sk_lock-AF_INET){+.+.}, at: [<ffffffff8ea04627>]
> inet_stream_connect+0x27/0x60
> [ 23.929900] #1: (rcu_read_lock){....}, at: [<ffffffff8e9c4065>]
> ip_queue_xmit+0x5/0x5c0
> [ 23.930000] #2: (rcu_read_lock){....}, at: [<ffffffff8e94e464>]
> process_backlog+0xd4/0x240
> [ 23.930098] #3: (rcu_read_lock){....}, at: [<ffffffff8e9bd4ff>]
> ip_local_deliver_finish+0x2f/0x380
> [ 23.930200] #4: (slock-AF_INET){+.-.}, at: [<ffffffff8e93131f>]
> sk_clone_lock+0x1af/0x580
> [ 23.930297]
> [ 23.930297] stack backtrace:
> [ 23.930396] CPU: 1 PID: 1679 Comm: criu Not tainted 4.14.0-rc4+ #1
> [ 23.930483] Hardware name: Google Google Compute Engine/Google
> Compute Engine, BIOS Google 01/01/2011
> [ 23.930581] Call Trace:
> [ 23.930667] <IRQ>
> [ 23.930766] dump_stack+0x85/0xc7
> [ 23.930855] print_circular_bug+0x226/0x330
> [ 23.930943] ? copy_trace+0xb0/0xb0
> [ 23.931033] check_prev_add+0x401/0x800
> [ 23.931122] ? tcp_v4_rcv+0x8ec/0xc20
> [ 23.931212] __lock_acquire+0x10fc/0x11a0
> [ 23.931294] ? __lock_acquire+0x10fc/0x11a0
> [ 23.931381] ? copy_trace+0xb0/0xb0
> [ 23.931468] ? inet_ehash_insert+0x14f/0x1d0
> [ 23.931558] lock_acquire+0xed/0x1e0
> [ 23.931648] ? del_timer_sync+0x5/0xc0
> [ 23.931743] del_timer_sync+0x47/0xc0
> [ 23.931829] ? del_timer_sync+0x5/0xc0
> [ 23.931917] inet_csk_reqsk_queue_drop+0xcc/0x1e0
> [ 23.932005] inet_csk_complete_hashdance+0x23/0x80
> [ 23.932095] tcp_check_req+0x3ec/0x510
> [ 23.932185] tcp_v4_rcv+0x8ec/0xc20
> [ 23.932275] ip_local_deliver_finish+0xdc/0x380
> [ 23.932366] ip_local_deliver+0x66/0x200
> [ 23.932457] ? inet_del_offload+0x40/0x40
> [ 23.932546] ip_rcv_finish+0x1b7/0x530
> [ 23.932633] ip_rcv+0x26c/0x4c0
> [ 23.932726] ? ip_local_deliver_finish+0x380/0x380
> [ 23.932813] __netif_receive_skb_core+0x74d/0xcc0
> [ 23.932899] __netif_receive_skb+0x18/0x60
> [ 23.932985] ? __netif_receive_skb+0x18/0x60
> [ 23.933070] process_backlog+0x72/0x240
> [ 23.933157] net_rx_action+0x1cb/0x3e0
> [ 23.933244] ? mark_held_locks+0x72/0xa0
> [ 23.933333] __do_softirq+0xd1/0x4a8
> [ 23.933418] ? ip_finish_output2+0x23a/0x630
> [ 23.933503] do_softirq_own_stack+0x2a/0x40
> [ 23.933585] </IRQ>
> [ 23.933670] do_softirq.part.16+0x46/0x70
> [ 23.933757] __local_bh_enable_ip+0x9a/0xa0
> [ 23.933839] ip_finish_output2+0x263/0x630
> [ 23.933926] ip_finish_output+0x1ba/0x2e0
> [ 23.934014] ? ip_finish_output+0x1ba/0x2e0
> [ 23.934101] ip_output+0x73/0x240
> [ 23.934192] ? ip_fragment.constprop.54+0x80/0x80
> [ 23.934284] ip_local_out+0x39/0x60
> [ 23.934373] ip_queue_xmit+0x1ea/0x5c0
> [ 23.934461] ? tcp_v4_md5_lookup+0x13/0x20
> [ 23.934550] tcp_transmit_skb+0x551/0xaa0
> [ 23.934639] tcp_send_ack+0xc8/0x130
> [ 23.934735] tcp_rcv_state_process+0xe3d/0xe90
> [ 23.934826] ? __local_bh_enable_ip+0x70/0xa0
> [ 23.934911] tcp_v4_do_rcv+0xbd/0x1d0
> [ 23.935000] ? tcp_v4_do_rcv+0xbd/0x1d0
> [ 23.935089] __release_sock+0x6d/0x110
> [ 23.935200] release_sock+0x30/0xb0
> [ 23.935288] __inet_stream_connect+0x187/0x320
> [ 23.935377] ? do_wait_intr_irq+0xc0/0xc0
> [ 23.935464] inet_stream_connect+0x3b/0x60
> [ 23.935557] SYSC_connect+0xbe/0xf0
> [ 23.935642] ? sock_alloc_file+0x97/0x110
> [ 23.935733] ? trace_hardirqs_on_caller+0x11f/0x190
> [ 23.935821] ? trace_hardirqs_on_thunk+0x1a/0x1c
> [ 23.935911] SyS_connect+0xe/0x10
> [ 23.935999] entry_SYSCALL_64_fastpath+0x23/0xc2
> [ 23.936086] RIP: 0033:0x7fb80b838720
> [ 23.936170] RSP: 002b:00007ffc277dac88 EFLAGS: 00000246 ORIG_RAX:
> 000000000000002a
> [ 23.936266] RAX: ffffffffffffffda RBX: 00007ffc277dac18 RCX: 00007fb80b838720
> [ 23.936353] RDX: 0000000000000010 RSI: 00007ffc277daca0 RDI: 0000000000000005
> [ 23.936442] RBP: 00007ffc277dabf0 R08: 00007ffc277dac43 R09: 0000000000000004
> [ 23.936528] R10: 00007ffc277daa50 R11: 0000000000000246 R12: 00007fb80bc52bc0
> [ 23.936613] R13: 0000000000000001 R14: 0000000000000000 R15: 00007fb80bc6b1c8
>
> Here are all logs https://travis-ci.org/avagin/linux/jobs/290283691
Hi Andrey
Kees posted a patch for this issue, thanks for the report.
https://patchwork.ozlabs.org/patch/828333/
^ permalink raw reply
* Re: [PATCH v3 net] dccp/tcp: fix ireq->opt races
From: David Miller @ 2017-10-20 6:04 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1508451889.30291.4.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 19 Oct 2017 15:24:49 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller found another bug in DCCP/TCP stacks [1]
>
> For the reasons explained in commit ce1050089c96 ("tcp/dccp: fix
> ireq->pktopts race"), we need to make sure we do not access
> ireq->opt unless we own the request sock.
...
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> v3: fixed the whit space mangling
> v2: removed some lines from KASAN report that confuse patchwork.
Much better, applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] vmxnet3: Use correct minimum MTU value
From: David Miller @ 2017-10-20 6:15 UTC (permalink / raw)
To: mgamal; +Cc: netdev, skhare, pv-drivers, linux-kernel, cavery, otubo, vkuznets
In-Reply-To: <1508250823-1954-1-git-send-email-mgamal@redhat.com>
From: Mohammed Gamal <mgamal@redhat.com>
Date: Tue, 17 Oct 2017 16:33:43 +0200
> Currently the vmxnet3 driver has a minimum MTU value of 60. Which
> goes against the RFC791 spec which specifies it at 68.
>
> Setting MTU to values between 60 <= MTU <= 67 causes the network
> interface to lose its IP, and it fails to restart.
>
> This sets the minimum value to ETH_MIN_MTU (68) which is compatible
> with is according to spec.
>
> Reported-by: Bo Yang <boyang@redhat.com>
> Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
There are protocols other than IPv4, and as Andrew mentioned
the ipv4 stack does the right thing by disabling ipv4 on
an interface when the MTU is too small.
^ permalink raw reply
* Re: [PATCH v3 net] dccp/tcp: fix ireq->opt races
From: David Miller @ 2017-10-20 6:18 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <20171020.070458.2242397039979446788.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Fri, 20 Oct 2017 07:04:58 +0100 (WEST)
> Much better, applied and queued up for -stable.
I take that back.
Please build test your changes more thoroughly.
In file included from ./arch/x86/include/asm/atomic.h:7:0,
from ./include/linux/atomic.h:4,
from ./include/linux/rcupdate.h:38,
from net/ipv4/cipso_ipv4.c:40:
net/ipv4/cipso_ipv4.c: In function ‘cipso_v4_req_setattr’:
net/ipv4/cipso_ipv4.c:1954:22: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = xchg(&req_inet->opt, opt);
^
./arch/x86/include/asm/cmpxchg.h:43:24: note: in definition of macro ‘__xchg_op’
__typeof__ (*(ptr)) __ret = (arg); \
^~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
./arch/x86/include/asm/cmpxchg.h:43:38: warning: initialization makes integer from pointer without a cast [-Wint-conversion]
__typeof__ (*(ptr)) __ret = (arg); \
^
./arch/x86/include/asm/cmpxchg.h:77:22: note: in expansion of macro ‘__xchg_op’
#define xchg(ptr, v) __xchg_op((ptr), (v), xchg, "")
^~~~~~~~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
net/ipv4/cipso_ipv4.c:1954:22: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = xchg(&req_inet->opt, opt);
^
./arch/x86/include/asm/cmpxchg.h:44:20: note: in definition of macro ‘__xchg_op’
switch (sizeof(*(ptr))) { \
^~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
net/ipv4/cipso_ipv4.c:1954:22: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = xchg(&req_inet->opt, opt);
^
./arch/x86/include/asm/cmpxchg.h:47:35: note: in definition of macro ‘__xchg_op’
: "+q" (__ret), "+m" (*(ptr)) \
^~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
net/ipv4/cipso_ipv4.c:1954:22: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = xchg(&req_inet->opt, opt);
^
./arch/x86/include/asm/cmpxchg.h:52:35: note: in definition of macro ‘__xchg_op’
: "+r" (__ret), "+m" (*(ptr)) \
^~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
net/ipv4/cipso_ipv4.c:1954:22: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = xchg(&req_inet->opt, opt);
^
./arch/x86/include/asm/cmpxchg.h:57:35: note: in definition of macro ‘__xchg_op’
: "+r" (__ret), "+m" (*(ptr)) \
^~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
net/ipv4/cipso_ipv4.c:1954:22: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = xchg(&req_inet->opt, opt);
^
./arch/x86/include/asm/cmpxchg.h:62:35: note: in definition of macro ‘__xchg_op’
: "+r" (__ret), "+m" (*(ptr)) \
^~~
net/ipv4/cipso_ipv4.c:1954:8: note: in expansion of macro ‘xchg’
opt = xchg(&req_inet->opt, opt);
^~~~
net/ipv4/cipso_ipv4.c:1954:6: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
opt = xchg(&req_inet->opt, opt);
^
net/ipv4/cipso_ipv4.c: In function ‘cipso_v4_req_delattr’:
net/ipv4/cipso_ipv4.c:2073:16: error: ‘struct inet_request_sock’ has no member named ‘opt’
opt = req_inet->opt;
^~
net/ipv4/cipso_ipv4.c:2077:27: error: ‘struct inet_request_sock’ has no member named ‘opt’
cipso_v4_delopt(&req_inet->opt);
^~
scripts/Makefile.build:313: recipe for target 'net/ipv4/cipso_ipv4.o' failed
make[2]: *** [net/ipv4/cipso_ipv4.o] Error 1
make[2]: *** Waiting for unfinished jobs....
scripts/Makefile.build:572: recipe for target 'net/ipv4' failed
make[1]: *** [net/ipv4] Error 2
Makefile:1019: recipe for target 'net' failed
make: *** [net] Error 2
^ permalink raw reply
* Re: [PATCH net-next v2 0/3] net: sh_eth: add R-Car Gen[12] fallback compatibility strings
From: David Miller @ 2017-10-20 6:36 UTC (permalink / raw)
To: horms+renesas; +Cc: sergei.shtylyov, magnus.damm, netdev, linux-renesas-soc
In-Reply-To: <20171018072128.11857-1-horms+renesas@verge.net.au>
From: Simon Horman <horms+renesas@verge.net.au>
Date: Wed, 18 Oct 2017 09:21:25 +0200
> Add fallback compatibility strings for R-Car Gen 1 and 2.
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Jiri Pirko @ 2017-10-20 6:44 UTC (permalink / raw)
To: Florian Fainelli
Cc: Yuval Mintz, Steve Lin, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com, andrew,
vivien.didelot
In-Reply-To: <d9968b08-eeb7-267a-cb55-48e5feeee922@gmail.com>
Fri, Oct 20, 2017 at 03:01:35AM CEST, f.fainelli@gmail.com wrote:
>On 10/19/2017 02:43 PM, Jiri Pirko wrote:
>> Thu, Oct 19, 2017 at 11:39:55PM CEST, jiri@resnulli.us wrote:
>>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>>>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>>>>> config
>>>>> parameter. Defines number of MSI-X vectors allocated per VF.
>>>>> Value is permanent (stored in NVRAM), so becomes the new default
>>>>> value for this device.
>>>>
>>>> Sounds like you're having this enforce the same configuration for all child VFs.
>>>
>>> Yeah, this sounds like per-port config.
>>
>> This opens old but lately silent discussion about introducing new port
>> types for different things. Like VF, dsa CPU port or dsa inter-chip
>> ports.
>
>FWIW, the "issue" with representing VF, DSA CPU port or DSA inter-chip
>port is that you would be representing a pipe, so there is obviously a
>question of whether your represent one end or both ends of that pipe,
>and how do you make sure both stay in sync if you represent those.
>
>For instance, for an inter-switch connection, I could decide to
>configure VLANs 1-3 tagged on one end of the connection, and forget to
>that on the other end of the connection, and that's just one example
>where things can go seriously wrong.
Certainly you have to represent both ends. So in your dsa inter-port
example, there should be onle devlink port instance for both dsa chips.
>
>>
>>>
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>>>> Acked-by: Andy Gospodarek <gospo@broadcom.com>
>>>>> ---
>>>>> include/uapi/linux/devlink.h | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>>>> index 8ad6c63..ef163b6 100644
>>>>> --- a/include/uapi/linux/devlink.h
>>>>> +++ b/include/uapi/linux/devlink.h
>>>>> @@ -260,6 +260,7 @@ enum devlink_perm_config_param {
>>>>> DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>>>>> DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
>>>>> DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
>>>>> + DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
>>>>> };
>>>>>
>>>>> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>>>>> --
>>>>> 2.7.4
>>>>
>
>
>--
>Florian
^ permalink raw reply
* Re: Problems with mvneta
From: Thomas Petazzoni @ 2017-10-20 7:09 UTC (permalink / raw)
To: Sven Müller
Cc: Grégory Clement, Antoine Ténart, netdev, Marcin Wojtas
In-Reply-To: <20171020002524.4b4cf122@gmx.de>
Hello,
On Fri, 20 Oct 2017 00:25:24 +0200, Sven Müller wrote:
> First of all I'm not familiar with kernel programming at all, so
> please excuse me, if I don't understand everything at the first
> glance.
No problem. Your bug report and effort to nail down the issue are much
appreciated!
> It compiles and runs fine. After a couple of hours and testing no
> issues were found.
OK, so this really hints at a regression in the mvneta driver itself.
Could you try to revert just:
6ad20165d376fa07919a70e4f43dfae564601829
a29b6235560a1ed10c8e1a73bfc616a66b802b90
2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a
first all of them, and then each one by one, so that we can pin-point
the commit that causes the breakage ?
I looked at all the other changes in mvneta between 4.10 and 4.12, and
I don't see how any of the other changes can cause a functional
difference. So let's focus on those 3 commits for the moment.
Assuming you're using Git, to revert a commit just do: "git revert
<commitid>".
Thanks again!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] net: ethtool: remove error check for legacy setting transceiver type
From: Geert Uytterhoeven @ 2017-10-20 7:15 UTC (permalink / raw)
To: Niklas Söderlund
Cc: David S. Miller, Florian Fainelli, netdev@vger.kernel.org,
Linux-Renesas, Renjith R V
In-Reply-To: <20171019233208.15444-1-niklas.soderlund+renesas@ragnatech.se>
Hi Niklas,
On Fri, Oct 20, 2017 at 1:32 AM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Commit 9cab88726929605 ("net: ethtool: Add back transceiver type")
> restores the transceiver type to struct ethtool_link_settings and
> convert_link_ksettings_to_legacy_settings() but forgets to remove the
> error check for the same in convert_legacy_settings_to_link_ksettings().
> This prevents older versions of ethtool to change link settings.
>
> # ethtool --version
> ethtool version 3.16
>
> # ethtool -s eth0 autoneg on speed 100 duplex full
> Cannot set new settings: Invalid argument
> not setting speed
> not setting duplex
> not setting autoneg
>
> While newer versions of ethtool works.
>
> # ethtool --version
> ethtool version 4.10
>
> # ethtool -s eth0 autoneg on speed 100 duplex full
> [ 57.703268] sh-eth ee700000.ethernet eth0: Link is Down
> [ 59.618227] sh-eth ee700000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>
> Fixes: 19cab88726929605 ("net: ethtool: Add back transceiver type")
Thanks for your patch!
Reported-by: Renjith R V <renjith.rv@quest-global.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
While this fixed ethtool, unfortunately this revealed another issue: several
drivers call phy_ethtool_ksettings_set() while holding a driver-specific
spinlock, which leads to phy_start_aneg_priv() calling mutex_lock() with
interrupts disabled.
Backtrace for sh_eth:
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238
[ 161.636382] in_atomic(): 1, irqs_disabled(): 128, pid: 1684, name: ethtool
[ 161.643260] CPU: 1 PID: 1684 Comm: ethtool Not tainted
4.14.0-rc5-koelsch-00441-g8545ae86337930b0 #3649
[ 161.652653] Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[ 161.659109] [<c020efe4>] (unwind_backtrace) from [<c020aa5c>]
(show_stack+0x10/0x14)
[ 161.666859] [<c020aa5c>] (show_stack) from [<c0723444>]
(dump_stack+0x7c/0x9c)
[ 161.674090] [<c0723444>] (dump_stack) from [<c023f760>]
(___might_sleep+0x128/0x164)
[ 161.681841] [<c023f760>] (___might_sleep) from [<c073894c>]
(mutex_lock+0x18/0x60)
[ 161.689418] [<c073894c>] (mutex_lock) from [<c0537c60>]
(phy_start_aneg_priv+0x28/0x120)
[ 161.697513] [<c0537c60>] (phy_start_aneg_priv) from [<c0537ee4>]
(phy_ethtool_ksettings_set+0xc0/0xcc)
[ 161.706824] [<c0537ee4>] (phy_ethtool_ksettings_set) from
[<c053fe58>] (sh_eth_set_link_ksettings+0x3c/0xa8)
[ 161.716657] [<c053fe58>] (sh_eth_set_link_ksettings) from
[<c0676f88>] (ethtool_set_settings+0x100/0x114)
[ 161.726229] [<c0676f88>] (ethtool_set_settings) from [<c0679ea0>]
(dev_ethtool+0x400/0x2248)
[ 161.734672] [<c0679ea0>] (dev_ethtool) from [<c068f850>]
(dev_ioctl+0x424/0x774)
[ 161.742074] [<c068f850>] (dev_ioctl) from [<c02f9a24>] (vfs_ioctl+0x20/0x34)
[ 161.749128] [<c02f9a24>] (vfs_ioctl) from [<c02fa1f0>]
(do_vfs_ioctl+0x6b4/0x7b8)
[ 161.756616] [<c02fa1f0>] (do_vfs_ioctl) from [<c02fa328>]
(SyS_ioctl+0x34/0x5c)
[ 161.763930] [<c02fa328>] (SyS_ioctl) from [<c0206e60>]
(ret_fast_syscall+0x0/0x40)
There's a similar dump for ravb.
I quick grep shows that at least the Broadcom B44 driver is also affected.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [patch net 0/2] mlxsw: spectrum: Configure TTL of "inherit" for offloaded tunnels
From: Jiri Pirko @ 2017-10-20 7:16 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Petr says:
Currently mlxsw only offloads tunnels that are configured with TTL of "inherit"
(which is the default). However, Spectrum defaults to 255 and the driver
neglects to change the configuration. Thus the tunnel packets from offloaded
tunnels always have TTL of 255, even though tunnels with explicit TTL of 255 are
never actually offloaded.
To fix this, introduce support for TIGCR, the register that keeps the related
bits of global tunnel configuration, and use it on first offload to properly
configure inheritance of TTL of tunnel packets from overlay packets.
Petr Machata (2):
mlxsw: reg: Add Tunneling IPinIP General Configuration Register
mlxsw: spectrum_router: Configure TIGCR on init
drivers/net/ethernet/mellanox/mlxsw/reg.h | 31 ++++++++++++++++++++++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 11 +++++++-
2 files changed, 41 insertions(+), 1 deletion(-)
--
2.9.5
^ permalink raw reply
* [patch net 1/2] mlxsw: reg: Add Tunneling IPinIP General Configuration Register
From: Jiri Pirko @ 2017-10-20 7:16 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, idosch, mlxsw
In-Reply-To: <20171020071616.1498-1-jiri@resnulli.us>
From: Petr Machata <petrm@mellanox.com>
The TIGCR register is used for setting up the IPinIP Tunnel
configuration.
Fixes: ee954d1a91b2 ("mlxsw: spectrum_router: Support GRE tunnels")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index cc27c5d..4afc848 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -6401,6 +6401,36 @@ static inline void mlxsw_reg_mgpc_pack(char *payload, u32 counter_index,
mlxsw_reg_mgpc_opcode_set(payload, opcode);
}
+/* TIGCR - Tunneling IPinIP General Configuration Register
+ * -------------------------------------------------------
+ * The TIGCR register is used for setting up the IPinIP Tunnel configuration.
+ */
+#define MLXSW_REG_TIGCR_ID 0xA801
+#define MLXSW_REG_TIGCR_LEN 0x10
+
+MLXSW_REG_DEFINE(tigcr, MLXSW_REG_TIGCR_ID, MLXSW_REG_TIGCR_LEN);
+
+/* reg_tigcr_ipip_ttlc
+ * For IPinIP Tunnel encapsulation: whether to copy the ttl from the packet
+ * header.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, tigcr, ttlc, 0x04, 8, 1);
+
+/* reg_tigcr_ipip_ttl_uc
+ * The TTL for IPinIP Tunnel encapsulation of unicast packets if
+ * reg_tigcr_ipip_ttlc is unset.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, tigcr, ttl_uc, 0x04, 0, 8);
+
+static inline void mlxsw_reg_tigcr_pack(char *payload, bool ttlc, u8 ttl_uc)
+{
+ MLXSW_REG_ZERO(tigcr, payload);
+ mlxsw_reg_tigcr_ttlc_set(payload, ttlc);
+ mlxsw_reg_tigcr_ttl_uc_set(payload, ttl_uc);
+}
+
/* SBPR - Shared Buffer Pools Register
* -----------------------------------
* The SBPR configures and retrieves the shared buffer pools and configuration.
@@ -6881,6 +6911,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
MLXSW_REG(mcc),
MLXSW_REG(mcda),
MLXSW_REG(mgpc),
+ MLXSW_REG(tigcr),
MLXSW_REG(sbpr),
MLXSW_REG(sbcm),
MLXSW_REG(sbpm),
--
2.9.5
^ permalink raw reply related
* [patch net 2/2] mlxsw: spectrum_router: Configure TIGCR on init
From: Jiri Pirko @ 2017-10-20 7:16 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, idosch, mlxsw
In-Reply-To: <20171020071616.1498-1-jiri@resnulli.us>
From: Petr Machata <petrm@mellanox.com>
Spectrum tunnels do not default to ttl of "inherit" like the Linux ones
do. Configure TIGCR on router init so that the TTL of tunnel packets is
copied from the overlay packets.
Fixes: ee954d1a91b2 ("mlxsw: spectrum_router: Support GRE tunnels")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index c16718d..5189022a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5896,11 +5896,20 @@ static void mlxsw_sp_rifs_fini(struct mlxsw_sp *mlxsw_sp)
kfree(mlxsw_sp->router->rifs);
}
+static int
+mlxsw_sp_ipip_config_tigcr(struct mlxsw_sp *mlxsw_sp)
+{
+ char tigcr_pl[MLXSW_REG_TIGCR_LEN];
+
+ mlxsw_reg_tigcr_pack(tigcr_pl, true, 0);
+ return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(tigcr), tigcr_pl);
+}
+
static int mlxsw_sp_ipips_init(struct mlxsw_sp *mlxsw_sp)
{
mlxsw_sp->router->ipip_ops_arr = mlxsw_sp_ipip_ops_arr;
INIT_LIST_HEAD(&mlxsw_sp->router->ipip_list);
- return 0;
+ return mlxsw_sp_ipip_config_tigcr(mlxsw_sp);
}
static void mlxsw_sp_ipips_fini(struct mlxsw_sp *mlxsw_sp)
--
2.9.5
^ permalink raw reply related
* [PATCH 00/15] networking drivers refcount_t conversions
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
Note: these are the last patches related to networking that perform
conversion of refcounters from atomic_t to refcount_t.
In contrast to the core network refcounter conversions that
were merged earlier, these are much more straightforward ones.
This series, for various networking drivers, replaces atomic_t reference
counters with the new refcount_t type and API (see include/linux/refcount.h).
By doing this we prevent intentional or accidental
underflows or overflows that can led to use-after-free vulnerabilities.
The patches are fully independent and can be cherry-picked separately.
Patches are based on top of net-next.
If there are no objections to the patches, please merge them via respective trees
Elena Reshetova (15):
drivers, net, ethernet: convert clip_entry.refcnt from atomic_t to
refcount_t
drivers, net, ethernet: convert mtk_eth.dma_refcnt from atomic_t to
refcount_t
drivers, net, mlx4: convert mlx4_cq.refcount from atomic_t to
refcount_t
drivers, net, mlx4: convert mlx4_qp.refcount from atomic_t to
refcount_t
drivers, net, mlx4: convert mlx4_srq.refcount from atomic_t to
refcount_t
drivers, net, mlx5: convert mlx5_cq.refcount from atomic_t to
refcount_t
drivers, net, mlx5: convert fs_node.refcount from atomic_t to
refcount_t
drivers, net, hamradio: convert sixpack.refcnt from atomic_t to
refcount_t
drivers, net: convert masces_rx_sa.refcnt from atomic_t to refcount_t
drivers, net: convert masces_rx_sc.refcnt from atomic_t to refcount_t
drivers, net: convert masces_tx_sa.refcnt from atomic_t to refcount_t
drivers, net, ppp: convert asyncppp.refcnt from atomic_t to refcount_t
drivers, net, ppp: convert ppp_file.refcnt from atomic_t to refcount_t
drivers, net, ppp: convert syncppp.refcnt from atomic_t to refcount_t
drivers, connector: convert cn_callback_entry.refcnt from atomic_t to
refcount_t
drivers/connector/cn_queue.c | 4 ++--
drivers/connector/connector.c | 2 +-
drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c | 13 +++++------
drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h | 4 +++-
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 ++++---
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 +++-
drivers/net/ethernet/mellanox/mlx4/cq.c | 8 +++----
drivers/net/ethernet/mellanox/mlx4/qp.c | 8 +++----
drivers/net/ethernet/mellanox/mlx4/srq.c | 8 +++----
drivers/net/ethernet/mellanox/mlx5/core/cq.c | 16 ++++++-------
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 28 +++++++++++------------
drivers/net/ethernet/mellanox/mlx5/core/fs_core.h | 3 ++-
drivers/net/hamradio/6pack.c | 12 +++++-----
drivers/net/macsec.c | 25 ++++++++++----------
drivers/net/ppp/ppp_async.c | 10 ++++----
drivers/net/ppp/ppp_generic.c | 21 +++++++++--------
drivers/net/ppp/ppp_synctty.c | 11 +++++----
include/linux/connector.h | 4 ++--
include/linux/mlx4/device.h | 8 +++----
include/linux/mlx5/cq.h | 4 ++--
20 files changed, 105 insertions(+), 96 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 01/15] drivers, net, ethernet: convert clip_entry.refcnt from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable clip_entry.refcnt is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c | 13 ++++++-------
drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h | 4 +++-
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
index 3103ef9..2900390 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
@@ -96,7 +96,8 @@ int cxgb4_clip_get(const struct net_device *dev, const u32 *lip, u8 v6)
if (!ret) {
ce = cte;
read_unlock_bh(&ctbl->lock);
- goto found;
+ refcount_inc(&ce->refcnt);
+ return 0;
}
}
read_unlock_bh(&ctbl->lock);
@@ -108,7 +109,7 @@ int cxgb4_clip_get(const struct net_device *dev, const u32 *lip, u8 v6)
list_del(&ce->list);
INIT_LIST_HEAD(&ce->list);
spin_lock_init(&ce->lock);
- atomic_set(&ce->refcnt, 0);
+ refcount_set(&ce->refcnt, 0);
atomic_dec(&ctbl->nfree);
list_add_tail(&ce->list, &ctbl->hash_list[hash]);
if (v6) {
@@ -138,9 +139,7 @@ int cxgb4_clip_get(const struct net_device *dev, const u32 *lip, u8 v6)
return -ENOMEM;
}
write_unlock_bh(&ctbl->lock);
-found:
- atomic_inc(&ce->refcnt);
-
+ refcount_set(&ce->refcnt, 1);
return 0;
}
EXPORT_SYMBOL(cxgb4_clip_get);
@@ -179,7 +178,7 @@ void cxgb4_clip_release(const struct net_device *dev, const u32 *lip, u8 v6)
found:
write_lock_bh(&ctbl->lock);
spin_lock_bh(&ce->lock);
- if (atomic_dec_and_test(&ce->refcnt)) {
+ if (refcount_dec_and_test(&ce->refcnt)) {
list_del(&ce->list);
INIT_LIST_HEAD(&ce->list);
list_add_tail(&ce->list, &ctbl->ce_free_head);
@@ -266,7 +265,7 @@ int clip_tbl_show(struct seq_file *seq, void *v)
ip[0] = '\0';
sprintf(ip, "%pISc", &ce->addr);
seq_printf(seq, "%-25s %u\n", ip,
- atomic_read(&ce->refcnt));
+ refcount_read(&ce->refcnt));
}
}
seq_printf(seq, "Free clip entries : %d\n", atomic_read(&ctbl->nfree));
diff --git a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h
index 35eb43c..a0e0ae1 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h
@@ -10,9 +10,11 @@
* release for licensing terms and conditions.
*/
+#include <linux/refcount.h>
+
struct clip_entry {
spinlock_t lock; /* Hold while modifying clip reference */
- atomic_t refcnt;
+ refcount_t refcnt;
struct list_head list;
union {
struct sockaddr_in addr;
--
2.7.4
^ permalink raw reply related
* [PATCH 02/15] drivers, net, ethernet: convert mtk_eth.dma_refcnt from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable mtk_eth.dma_refcnt is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +++++---
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 +++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 5e81a72..54adfd9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1817,7 +1817,7 @@ static int mtk_open(struct net_device *dev)
struct mtk_eth *eth = mac->hw;
/* we run 2 netdevs on the same dma ring so we only bring it up once */
- if (!atomic_read(ð->dma_refcnt)) {
+ if (!refcount_read(ð->dma_refcnt)) {
int err = mtk_start_dma(eth);
if (err)
@@ -1827,8 +1827,10 @@ static int mtk_open(struct net_device *dev)
napi_enable(ð->rx_napi);
mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
mtk_rx_irq_enable(eth, MTK_RX_DONE_INT);
+ refcount_set(ð->dma_refcnt, 1);
}
- atomic_inc(ð->dma_refcnt);
+ else
+ refcount_inc(ð->dma_refcnt);
phy_start(dev->phydev);
netif_start_queue(dev);
@@ -1868,7 +1870,7 @@ static int mtk_stop(struct net_device *dev)
phy_stop(dev->phydev);
/* only shutdown DMA if this is the last user */
- if (!atomic_dec_and_test(ð->dma_refcnt))
+ if (!refcount_dec_and_test(ð->dma_refcnt))
return 0;
mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 3d3c24a..a3af466 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -15,6 +15,8 @@
#ifndef MTK_ETH_H
#define MTK_ETH_H
+#include <linux/refcount.h>
+
#define MTK_QDMA_PAGE_SIZE 2048
#define MTK_MAX_RX_LENGTH 1536
#define MTK_TX_DMA_BUF_LEN 0x3fff
@@ -632,7 +634,7 @@ struct mtk_eth {
struct regmap *pctl;
u32 chip_id;
bool hwlro;
- atomic_t dma_refcnt;
+ refcount_t dma_refcnt;
struct mtk_tx_ring tx_ring;
struct mtk_rx_ring rx_ring[MTK_MAX_RX_RING_NUM];
struct mtk_rx_ring rx_ring_qdma;
--
2.7.4
^ permalink raw reply related
* [PATCH 04/15] drivers, net, mlx4: convert mlx4_qp.refcount from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: peterz-wEGCiKHe2LqWVfeAwA7xHQ, matanb-VPRAkNaXOzVWk0Htik3J/w,
paulus-eUNUBHrolfbYtjvyW6yDsg, zbr-i6C2adt8DTjR7s880joybQ,
Elena Reshetova, nbd-p3rKhJxN3npAfugRpC6u6w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, saeedm-VPRAkNaXOzVWk0Htik3J/w,
ganeshgr-ut6Up61K2wZBDgjK7y7TUQ,
ajk-iz34hMvxm2Hmj42eshorlhS11BummzK+,
keescook-F7+t8E8rja9g9hUCZPvPmw, sean.wang-NuS5LvNUpcJWk0Htik3J/w,
leonro-VPRAkNaXOzVWk0Htik3J/w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
john-Pj+rj9U5foFAfugRpC6u6w, matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-hams-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-ppp-u79uwXL29TY76Z2rM5mHXA, tariqt-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable mlx4_qp.refcount is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: David Windsor <dwindsor-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Hans Liljestrand <ishkamiel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Elena Reshetova <elena.reshetova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/net/ethernet/mellanox/mlx4/qp.c | 8 ++++----
include/linux/mlx4/device.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
index 2033209..769598f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
@@ -55,7 +55,7 @@ void mlx4_qp_event(struct mlx4_dev *dev, u32 qpn, int event_type)
qp = __mlx4_qp_lookup(dev, qpn);
if (qp)
- atomic_inc(&qp->refcount);
+ refcount_inc(&qp->refcount);
spin_unlock(&qp_table->lock);
@@ -66,7 +66,7 @@ void mlx4_qp_event(struct mlx4_dev *dev, u32 qpn, int event_type)
qp->event(qp, event_type);
- if (atomic_dec_and_test(&qp->refcount))
+ if (refcount_dec_and_test(&qp->refcount))
complete(&qp->free);
}
@@ -420,7 +420,7 @@ int mlx4_qp_alloc(struct mlx4_dev *dev, int qpn, struct mlx4_qp *qp)
if (err)
goto err_icm;
- atomic_set(&qp->refcount, 1);
+ refcount_set(&qp->refcount, 1);
init_completion(&qp->free);
return 0;
@@ -520,7 +520,7 @@ EXPORT_SYMBOL_GPL(mlx4_qp_remove);
void mlx4_qp_free(struct mlx4_dev *dev, struct mlx4_qp *qp)
{
- if (atomic_dec_and_test(&qp->refcount))
+ if (refcount_dec_and_test(&qp->refcount))
complete(&qp->free);
wait_for_completion(&qp->free);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index daac2e3..b8e19c4 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -768,7 +768,7 @@ struct mlx4_qp {
int qpn;
- atomic_t refcount;
+ refcount_t refcount;
struct completion free;
u8 usage;
};
--
2.7.4
^ permalink raw reply related
* [PATCH 05/15] drivers, net, mlx4: convert mlx4_srq.refcount from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable mlx4_srq.refcount is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/ethernet/mellanox/mlx4/srq.c | 8 ++++----
include/linux/mlx4/device.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/srq.c b/drivers/net/ethernet/mellanox/mlx4/srq.c
index bedf521..cbe4d97 100644
--- a/drivers/net/ethernet/mellanox/mlx4/srq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/srq.c
@@ -49,7 +49,7 @@ void mlx4_srq_event(struct mlx4_dev *dev, u32 srqn, int event_type)
srq = radix_tree_lookup(&srq_table->tree, srqn & (dev->caps.num_srqs - 1));
rcu_read_unlock();
if (srq)
- atomic_inc(&srq->refcount);
+ refcount_inc(&srq->refcount);
else {
mlx4_warn(dev, "Async event for bogus SRQ %08x\n", srqn);
return;
@@ -57,7 +57,7 @@ void mlx4_srq_event(struct mlx4_dev *dev, u32 srqn, int event_type)
srq->event(srq, event_type);
- if (atomic_dec_and_test(&srq->refcount))
+ if (refcount_dec_and_test(&srq->refcount))
complete(&srq->free);
}
@@ -203,7 +203,7 @@ int mlx4_srq_alloc(struct mlx4_dev *dev, u32 pdn, u32 cqn, u16 xrcd,
if (err)
goto err_radix;
- atomic_set(&srq->refcount, 1);
+ refcount_set(&srq->refcount, 1);
init_completion(&srq->free);
return 0;
@@ -232,7 +232,7 @@ void mlx4_srq_free(struct mlx4_dev *dev, struct mlx4_srq *srq)
radix_tree_delete(&srq_table->tree, srq->srqn);
spin_unlock_irq(&srq_table->lock);
- if (atomic_dec_and_test(&srq->refcount))
+ if (refcount_dec_and_test(&srq->refcount))
complete(&srq->free);
wait_for_completion(&srq->free);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index b8e19c4..a9b5fed 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -781,7 +781,7 @@ struct mlx4_srq {
int max_gs;
int wqe_shift;
- atomic_t refcount;
+ refcount_t refcount;
struct completion free;
};
--
2.7.4
^ permalink raw reply related
* [PATCH 06/15] drivers, net, mlx5: convert mlx5_cq.refcount from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable mlx5_cq.refcount is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/ethernet/mellanox/mlx5/core/cq.c | 16 ++++++++--------
include/linux/mlx5/cq.h | 4 ++--
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cq.c b/drivers/net/ethernet/mellanox/mlx5/core/cq.c
index 336d473..1016e05 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cq.c
@@ -58,7 +58,7 @@ void mlx5_cq_tasklet_cb(unsigned long data)
tasklet_ctx.list) {
list_del_init(&mcq->tasklet_ctx.list);
mcq->tasklet_ctx.comp(mcq);
- if (atomic_dec_and_test(&mcq->refcount))
+ if (refcount_dec_and_test(&mcq->refcount))
complete(&mcq->free);
if (time_after(jiffies, end))
break;
@@ -80,7 +80,7 @@ static void mlx5_add_cq_to_tasklet(struct mlx5_core_cq *cq)
* still arrive.
*/
if (list_empty_careful(&cq->tasklet_ctx.list)) {
- atomic_inc(&cq->refcount);
+ refcount_inc(&cq->refcount);
list_add_tail(&cq->tasklet_ctx.list, &tasklet_ctx->list);
}
spin_unlock_irqrestore(&tasklet_ctx->lock, flags);
@@ -94,7 +94,7 @@ void mlx5_cq_completion(struct mlx5_core_dev *dev, u32 cqn)
spin_lock(&table->lock);
cq = radix_tree_lookup(&table->tree, cqn);
if (likely(cq))
- atomic_inc(&cq->refcount);
+ refcount_inc(&cq->refcount);
spin_unlock(&table->lock);
if (!cq) {
@@ -106,7 +106,7 @@ void mlx5_cq_completion(struct mlx5_core_dev *dev, u32 cqn)
cq->comp(cq);
- if (atomic_dec_and_test(&cq->refcount))
+ if (refcount_dec_and_test(&cq->refcount))
complete(&cq->free);
}
@@ -119,7 +119,7 @@ void mlx5_cq_event(struct mlx5_core_dev *dev, u32 cqn, int event_type)
cq = radix_tree_lookup(&table->tree, cqn);
if (cq)
- atomic_inc(&cq->refcount);
+ refcount_inc(&cq->refcount);
spin_unlock(&table->lock);
@@ -130,7 +130,7 @@ void mlx5_cq_event(struct mlx5_core_dev *dev, u32 cqn, int event_type)
cq->event(cq, event_type);
- if (atomic_dec_and_test(&cq->refcount))
+ if (refcount_dec_and_test(&cq->refcount))
complete(&cq->free);
}
@@ -159,7 +159,7 @@ int mlx5_core_create_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
cq->cqn = MLX5_GET(create_cq_out, out, cqn);
cq->cons_index = 0;
cq->arm_sn = 0;
- atomic_set(&cq->refcount, 1);
+ refcount_set(&cq->refcount, 1);
init_completion(&cq->free);
if (!cq->comp)
cq->comp = mlx5_add_cq_to_tasklet;
@@ -222,7 +222,7 @@ int mlx5_core_destroy_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq)
synchronize_irq(cq->irqn);
mlx5_debug_cq_remove(dev, cq);
- if (atomic_dec_and_test(&cq->refcount))
+ if (refcount_dec_and_test(&cq->refcount))
complete(&cq->free);
wait_for_completion(&cq->free);
diff --git a/include/linux/mlx5/cq.h b/include/linux/mlx5/cq.h
index 9589884..6a57ec2 100644
--- a/include/linux/mlx5/cq.h
+++ b/include/linux/mlx5/cq.h
@@ -35,7 +35,7 @@
#include <rdma/ib_verbs.h>
#include <linux/mlx5/driver.h>
-
+#include <linux/refcount.h>
struct mlx5_core_cq {
u32 cqn;
@@ -43,7 +43,7 @@ struct mlx5_core_cq {
__be32 *set_ci_db;
__be32 *arm_db;
struct mlx5_uars_page *uar;
- atomic_t refcount;
+ refcount_t refcount;
struct completion free;
unsigned vector;
unsigned int irqn;
--
2.7.4
^ permalink raw reply related
* [PATCH 07/15] drivers, net, mlx5: convert fs_node.refcount from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable fs_node.refcount is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 28 +++++++++++------------
drivers/net/ethernet/mellanox/mlx5/core/fs_core.h | 3 ++-
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index f77e496..c7fa1389 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -188,7 +188,7 @@ static void tree_init_node(struct fs_node *node,
void (*del_hw_func)(struct fs_node *),
void (*del_sw_func)(struct fs_node *))
{
- atomic_set(&node->refcount, 1);
+ refcount_set(&node->refcount, 1);
INIT_LIST_HEAD(&node->list);
INIT_LIST_HEAD(&node->children);
init_rwsem(&node->lock);
@@ -200,7 +200,7 @@ static void tree_init_node(struct fs_node *node,
static void tree_add_node(struct fs_node *node, struct fs_node *parent)
{
if (parent)
- atomic_inc(&parent->refcount);
+ refcount_inc(&parent->refcount);
node->parent = parent;
/* Parent is the root */
@@ -212,7 +212,7 @@ static void tree_add_node(struct fs_node *node, struct fs_node *parent)
static int tree_get_node(struct fs_node *node)
{
- return atomic_add_unless(&node->refcount, 1, 0);
+ return refcount_inc_not_zero(&node->refcount);
}
static void nested_down_read_ref_node(struct fs_node *node,
@@ -220,7 +220,7 @@ static void nested_down_read_ref_node(struct fs_node *node,
{
if (node) {
down_read_nested(&node->lock, class);
- atomic_inc(&node->refcount);
+ refcount_inc(&node->refcount);
}
}
@@ -229,7 +229,7 @@ static void nested_down_write_ref_node(struct fs_node *node,
{
if (node) {
down_write_nested(&node->lock, class);
- atomic_inc(&node->refcount);
+ refcount_inc(&node->refcount);
}
}
@@ -237,19 +237,19 @@ static void down_write_ref_node(struct fs_node *node)
{
if (node) {
down_write(&node->lock);
- atomic_inc(&node->refcount);
+ refcount_inc(&node->refcount);
}
}
static void up_read_ref_node(struct fs_node *node)
{
- atomic_dec(&node->refcount);
+ refcount_dec(&node->refcount);
up_read(&node->lock);
}
static void up_write_ref_node(struct fs_node *node)
{
- atomic_dec(&node->refcount);
+ refcount_dec(&node->refcount);
up_write(&node->lock);
}
@@ -257,7 +257,7 @@ static void tree_put_node(struct fs_node *node)
{
struct fs_node *parent_node = node->parent;
- if (atomic_dec_and_test(&node->refcount)) {
+ if (refcount_dec_and_test(&node->refcount)) {
if (node->del_hw_func)
node->del_hw_func(node);
if (parent_node) {
@@ -280,8 +280,8 @@ static void tree_put_node(struct fs_node *node)
static int tree_remove_node(struct fs_node *node)
{
- if (atomic_read(&node->refcount) > 1) {
- atomic_dec(&node->refcount);
+ if (refcount_read(&node->refcount) > 1) {
+ refcount_dec(&node->refcount);
return -EEXIST;
}
tree_put_node(node);
@@ -1184,7 +1184,7 @@ static void destroy_flow_handle(struct fs_fte *fte,
int i)
{
for (; --i >= 0;) {
- if (atomic_dec_and_test(&handle->rule[i]->node.refcount)) {
+ if (refcount_dec_and_test(&handle->rule[i]->node.refcount)) {
fte->dests_size--;
list_del(&handle->rule[i]->node.list);
kfree(handle->rule[i]);
@@ -1215,7 +1215,7 @@ create_flow_handle(struct fs_fte *fte,
if (dest) {
rule = find_flow_rule(fte, dest + i);
if (rule) {
- atomic_inc(&rule->node.refcount);
+ refcount_inc(&rule->node.refcount);
goto rule_found;
}
}
@@ -1466,7 +1466,7 @@ static struct mlx5_flow_handle *add_rule_fg(struct mlx5_flow_group *fg,
trace_mlx5_fs_set_fte(fte, false);
for (i = 0; i < handle->num_rules; i++) {
- if (atomic_read(&handle->rule[i]->node.refcount) == 1) {
+ if (refcount_read(&handle->rule[i]->node.refcount) == 1) {
tree_add_node(&handle->rule[i]->node, &fte->node);
trace_mlx5_fs_add_rule(handle->rule[i]);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h
index 80f6f3c7..397d24a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h
@@ -33,6 +33,7 @@
#ifndef _MLX5_FS_CORE_
#define _MLX5_FS_CORE_
+#include <linux/refcount.h>
#include <linux/mlx5/fs.h>
#include <linux/rhashtable.h>
@@ -84,7 +85,7 @@ struct fs_node {
struct fs_node *root;
/* lock the node for writing and traversing */
struct rw_semaphore lock;
- atomic_t refcount;
+ refcount_t refcount;
bool active;
void (*del_hw_func)(struct fs_node *);
void (*del_sw_func)(struct fs_node *);
--
2.7.4
^ permalink raw reply related
* [PATCH 08/15] drivers, net, hamradio: convert sixpack.refcnt from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable sixpack.refcnt is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/hamradio/6pack.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index bbc7b78..32f49c4 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -35,7 +35,7 @@
#include <linux/tcp.h>
#include <linux/semaphore.h>
#include <linux/compat.h>
-#include <linux/atomic.h>
+#include <linux/refcount.h>
#define SIXPACK_VERSION "Revision: 0.3.0"
@@ -120,7 +120,7 @@ struct sixpack {
struct timer_list tx_t;
struct timer_list resync_t;
- atomic_t refcnt;
+ refcount_t refcnt;
struct semaphore dead_sem;
spinlock_t lock;
};
@@ -381,7 +381,7 @@ static struct sixpack *sp_get(struct tty_struct *tty)
read_lock(&disc_data_lock);
sp = tty->disc_data;
if (sp)
- atomic_inc(&sp->refcnt);
+ refcount_inc(&sp->refcnt);
read_unlock(&disc_data_lock);
return sp;
@@ -389,7 +389,7 @@ static struct sixpack *sp_get(struct tty_struct *tty)
static void sp_put(struct sixpack *sp)
{
- if (atomic_dec_and_test(&sp->refcnt))
+ if (refcount_dec_and_test(&sp->refcnt))
up(&sp->dead_sem);
}
@@ -576,7 +576,7 @@ static int sixpack_open(struct tty_struct *tty)
sp->dev = dev;
spin_lock_init(&sp->lock);
- atomic_set(&sp->refcnt, 1);
+ refcount_set(&sp->refcnt, 1);
sema_init(&sp->dead_sem, 0);
/* !!! length of the buffers. MTU is IP MTU, not PACLEN! */
@@ -670,7 +670,7 @@ static void sixpack_close(struct tty_struct *tty)
* We have now ensured that nobody can start using ap from now on, but
* we have to wait for all existing users to finish.
*/
- if (!atomic_dec_and_test(&sp->refcnt))
+ if (!refcount_dec_and_test(&sp->refcnt))
down(&sp->dead_sem);
/* We must stop the queue to avoid potentially scribbling
--
2.7.4
^ permalink raw reply related
* [PATCH 09/15] drivers, net: convert masces_rx_sa.refcnt from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable masces_rx_sa.refcnt is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/macsec.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index ccbe4eaf..733e1c2 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -16,6 +16,7 @@
#include <crypto/aead.h>
#include <linux/etherdevice.h>
#include <linux/rtnetlink.h>
+#include <linux/refcount.h>
#include <net/genetlink.h>
#include <net/sock.h>
#include <net/gro_cells.h>
@@ -146,7 +147,7 @@ struct macsec_rx_sa {
struct macsec_key key;
spinlock_t lock;
u32 next_pn;
- atomic_t refcnt;
+ refcount_t refcnt;
bool active;
struct macsec_rx_sa_stats __percpu *stats;
struct macsec_rx_sc *sc;
@@ -314,7 +315,7 @@ static struct macsec_rx_sa *macsec_rxsa_get(struct macsec_rx_sa __rcu *ptr)
if (!sa || !sa->active)
return NULL;
- if (!atomic_inc_not_zero(&sa->refcnt))
+ if (!refcount_inc_not_zero(&sa->refcnt))
return NULL;
return sa;
@@ -350,7 +351,7 @@ static void free_rxsa(struct rcu_head *head)
static void macsec_rxsa_put(struct macsec_rx_sa *sa)
{
- if (atomic_dec_and_test(&sa->refcnt))
+ if (refcount_dec_and_test(&sa->refcnt))
call_rcu(&sa->rcu, free_rxsa);
}
@@ -1339,7 +1340,7 @@ static int init_rx_sa(struct macsec_rx_sa *rx_sa, char *sak, int key_len,
rx_sa->active = false;
rx_sa->next_pn = 1;
- atomic_set(&rx_sa->refcnt, 1);
+ refcount_set(&rx_sa->refcnt, 1);
spin_lock_init(&rx_sa->lock);
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH 11/15] drivers, net: convert masces_tx_sa.refcnt from atomic_t to refcount_t
From: Elena Reshetova @ 2017-10-20 7:23 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-rdma, linux-hams, linux-ppp, ganeshgr, nbd, john, sean.wang,
matthias.bgg, yishaih, saeedm, matanb, tariqt, leonro, ajk,
paulus, zbr, peterz, keescook, Elena Reshetova
In-Reply-To: <1508484229-19706-1-git-send-email-elena.reshetova@intel.com>
atomic_t variables are currently used to implement reference
counters with the following properties:
- counter is initialized to 1 using atomic_set()
- a resource is freed upon counter reaching zero
- once counter reaches zero, its further
increments aren't allowed
- counter schema uses basic atomic operations
(set, inc, inc_not_zero, dec_and_test, etc.)
Such atomic variables should be converted to a newly provided
refcount_t type and API that prevents accidental counter overflows
and underflows. This is important since overflows and underflows
can lead to use-after-free situation and be exploitable.
The variable masces_tx_sa.refcnt is used as pure reference counter.
Convert it to refcount_t and fix up the operations.
Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
drivers/net/macsec.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index e0aeb51..8948b6a 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -188,7 +188,7 @@ struct macsec_tx_sa {
struct macsec_key key;
spinlock_t lock;
u32 next_pn;
- atomic_t refcnt;
+ refcount_t refcnt;
bool active;
struct macsec_tx_sa_stats __percpu *stats;
struct rcu_head rcu;
@@ -362,7 +362,7 @@ static struct macsec_tx_sa *macsec_txsa_get(struct macsec_tx_sa __rcu *ptr)
if (!sa || !sa->active)
return NULL;
- if (!atomic_inc_not_zero(&sa->refcnt))
+ if (!refcount_inc_not_zero(&sa->refcnt))
return NULL;
return sa;
@@ -379,7 +379,7 @@ static void free_txsa(struct rcu_head *head)
static void macsec_txsa_put(struct macsec_tx_sa *sa)
{
- if (atomic_dec_and_test(&sa->refcnt))
+ if (refcount_dec_and_test(&sa->refcnt))
call_rcu(&sa->rcu, free_txsa);
}
@@ -1437,7 +1437,7 @@ static int init_tx_sa(struct macsec_tx_sa *tx_sa, char *sak, int key_len,
}
tx_sa->active = false;
- atomic_set(&tx_sa->refcnt, 1);
+ refcount_set(&tx_sa->refcnt, 1);
spin_lock_init(&tx_sa->lock);
return 0;
--
2.7.4
^ permalink raw reply related
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