Hi Ying,
I tested both your patch plus mine and they together fixed the random
panics I saw.
If you dont have any other comments, I can send both your patch "tipc:
fix a race condition of releasing subscriber " and mine to net.
/Partha
On 05/18/2017 09:54 AM, Ying Xue wrote:
> Hi Partha,
>
> Sorry for late response.
>
> Good catch! Please add my "ack-by".
>
> Thanks,
> Ying
>
> On 05/08/2017 09:46 PM, Parthasarathy Bhuvaragan wrote:
>> In commit, 139bb36f754a ("tipc: advance the time of deleting
>> subscription from subscriber->subscrp_list"), we delete the
>> subscription from the subscribers list and from nametable
>> unconditionally. This leads to the following bug if the timer
>> running tipc_subscrp_timeout() in another CPU accesses the
>> subscription list after the subscription delete request.
>>
>> [39.570] general protection fault: 0000 [#1] SMP
>> ::
>> [39.574] task: ffffffff81c10540 task.stack: ffffffff81c00000
>> [39.575] RIP: 0010:tipc_subscrp_timeout+0x32/0x80 [tipc]
>> [39.576] RSP: 0018:ffff88003ba03e90 EFLAGS: 00010282
>> [39.576] RAX: dead000000000200 RBX: ffff88003f0f3600 RCX: 0000000000000101
>> [39.577] RDX: dead000000000100 RSI: 0000000000000201 RDI: ffff88003f0d7948
>> [39.578] RBP: ffff88003ba03ea0 R08: 0000000000000001 R09: ffff88003ba03ef8
>> [39.579] R10: 000000000000014f R11: 0000000000000000 R12: ffff88003f0d7948
>> [39.580] R13: ffff88003f0f3618 R14: ffffffffa006c250 R15: ffff88003f0f3600
>> [39.581] FS: 0000000000000000(0000) GS:ffff88003ba00000(0000)
>> knlGS:0000000000000000
>> [39.582] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [39.583] CR2: 00007f831c6e0714 CR3: 000000003d3b0000 CR4: 00000000000006f0
>> [39.584] Call Trace:
>> [39.584] <IRQ>
>> [39.585] call_timer_fn+0x3d/0x180
>> [39.585] ? tipc_subscrb_rcv_cb+0x260/0x260 [tipc]
>> [39.586] run_timer_softirq+0x168/0x1f0
>> [39.586] ? sched_clock_cpu+0x16/0xc0
>> [39.587] __do_softirq+0x9b/0x2de
>> [39.587] irq_exit+0x60/0x70
>> [39.588] smp_apic_timer_interrupt+0x3d/0x50
>> [39.588] apic_timer_interrupt+0x86/0x90
>> [39.589] RIP: 0010:default_idle+0x20/0xf0
>> [39.589] RSP: 0018:ffffffff81c03e58 EFLAGS: 00000246 ORIG_RAX:
>> ffffffffffffff10
>> [39.590] RAX: 0000000000000000 RBX: ffffffff81c10540 RCX: 0000000000000000
>> [39.591] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
>> [39.592] RBP: ffffffff81c03e68 R08: 0000000000000000 R09: 0000000000000000
>> [39.593] R10: ffffc90001cbbe00 R11: 0000000000000000 R12: 0000000000000000
>> [39.594] R13: ffffffff81c10540 R14: 0000000000000000 R15: 0000000000000000
>> [39.595] </IRQ>
>> ::
>> [39.603] RIP: tipc_subscrp_timeout+0x32/0x80 [tipc] RSP: ffff88003ba03e90
>> [39.604] ---[ end trace 79ce94b7216cb459 ]---
>>
>> Fixes: 139bb36f754a ("tipc: advance the time of deleting subscription from
>> subscriber->subscrp_list")
>> Signed-off-by: Parthasarathy Bhuvaragan
>> <[email protected]>
>> ---
>> net/tipc/subscr.c | 19 +++++++------------
>> 1 file changed, 7 insertions(+), 12 deletions(-)
>>
>> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
>> index 0bf91cd3733c..f2c81f42dfda 100644
>> --- a/net/tipc/subscr.c
>> +++ b/net/tipc/subscr.c
>> @@ -52,7 +52,6 @@ struct tipc_subscriber {
>> struct list_head subscrp_list;
>> };
>>
>> -static void tipc_subscrp_delete(struct tipc_subscription *sub);
>> static void tipc_subscrb_put(struct tipc_subscriber *subscriber);
>>
>> /**
>> @@ -197,15 +196,19 @@ static void tipc_subscrb_subscrp_delete(struct
>> tipc_subscriber *subscriber,
>> {
>> struct list_head *subscription_list = &subscriber->subscrp_list;
>> struct tipc_subscription *sub, *temp;
>> + u32 timeout;
>>
>> spin_lock_bh(&subscriber->lock);
>> list_for_each_entry_safe(sub, temp, subscription_list, subscrp_list) {
>> if (s && memcmp(s, &sub->evt.s, sizeof(struct tipc_subscr)))
>> continue;
>>
>> - tipc_nametbl_unsubscribe(sub);
>> - list_del(&sub->subscrp_list);
>> - tipc_subscrp_delete(sub);
>> + timeout = htohl(sub->evt.s.timeout, sub->swap);
>> + if (timeout == TIPC_WAIT_FOREVER || del_timer(&sub->timer)) {
>> + tipc_nametbl_unsubscribe(sub);
>> + list_del(&sub->subscrp_list);
>> + tipc_subscrp_put(sub);
>> + }
>>
>> if (s)
>> break;
>> @@ -236,14 +239,6 @@ static void tipc_subscrb_delete(struct tipc_subscriber
>> *subscriber)
>> tipc_subscrb_put(subscriber);
>> }
>>
>> -static void tipc_subscrp_delete(struct tipc_subscription *sub)
>> -{
>> - u32 timeout = htohl(sub->evt.s.timeout, sub->swap);
>> -
>> - if (timeout == TIPC_WAIT_FOREVER || del_timer(&sub->timer))
>> - tipc_subscrp_put(sub);
>> -}
>> -
>> static void tipc_subscrp_cancel(struct tipc_subscr *s,
>> struct tipc_subscriber *subscriber)
>> {
>>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
