netdev - Re: [PATCH net 3/7] net/mlx5e: Fix soft lockup when HW Timestamping is enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20160225.163106.2304939289705309406.davem@davemloft.net>
Date:	Thu, 25 Feb 2016 16:31:06 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	saeedm@...lanox.com
Cc:	netdev@...r.kernel.org, ogerlitz@...lanox.com, talal@...lanox.com,
	eranbe@...lanox.com, tariqt@...lanox.com, galp@...lanox.com
Subject: Re: [PATCH net 3/7] net/mlx5e: Fix soft lockup when HW
 Timestamping is enabled

From: Saeed Mahameed <saeedm@...lanox.com>
Date: Wed, 24 Feb 2016 13:18:49 +0200

> From: Eran Ben Elisha <eranbe@...lanox.com>
> 
> Readers/Writers lock for SW timecounter was acquired without disabling
> interrupts on local CPU.
> 
> The problematic scenario:
> * HW timestamping is enabled
> * Timestamp overflow periodic service task is running on local CPU and
>   holding write_lock for SW timecounter
> * Completion arrives, triggers interrupt for local CPU.
>   Interrupt routine calls napi_schedule(), which triggers rx/tx
>   skb process.
>   An attempt to read SW timecounter using read_lock is done, which is
>   already locked by a writer on the same CPU and cause soft lockup.
> 
> Add irqsave/irqrestore for when using the readers/writers lock.
> 
> Fixes: ef9814deafd0 ('net/mlx5e: Add HW timestamping (TS) support')
> Signed-off-by: Eran Ben Elisha <eranbe@...lanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>

I'd like to suggest a slight modification to this fix.

The canonical way to deal with this issue is to use IRQ disabling
for the write lock side, but not for the read lock side.

You only need to prevent a read lock via interrupt from nesting on
the same cpu where you took the write lock.

So you don't need the expensive IRQ disabling for the read lock side,
which is the more important path performance wise.

Thanks.