netdev - RE: [Intel-wired-lan] [PATCH net v2] ice: make writes to /dev/gnssX synchronous

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]

Date: Tue, 6 Jun 2023 07:38:27 +0000
From: "Mekala, SunithaX D" <sunithax.d.mekala@...el.com>
To: Simon Horman <simon.horman@...igine.com>, mschmidt <mschmidt@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Kolacinski, Karol"
	<karol.kolacinski@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, Johan Hovold <johan@...nel.org>
Subject: RE: [Intel-wired-lan] [PATCH net v2] ice: make writes to /dev/gnssX
 synchronous

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of Simon Horman
> Sent: Friday, May 26, 2023 2:13 AM
> To: mschmidt <mschmidt@...hat.com>
> Cc: netdev@...r.kernel.org; Kolacinski, Karol <karol.kolacinski@...el.com>; Nguyen, Anthony L <anthony.l.nguyen@...el.com>; intel-wired-lan@...ts.osuosl.org; Johan Hovold <johan@...nel.org>
> Subject: Re: [Intel-wired-lan] [PATCH net v2] ice: make writes to /dev/gnssX synchronous
>
> On Thu, May 25, 2023 at 12:04:46PM +0200, Michal Schmidt wrote:
>>  The current ice driver's GNSS write implementation buffers writes and 
>>  works through them asynchronously in a kthread. That's bad because:
>>   - The GNSS write_raw operation is supposed to be synchronous[1][2].
>>   - There is no upper bound on the number of pending writes.
>>     Userspace can submit writes much faster than the driver can process,
> >    consuming unlimited amounts of kernel memory.
>>  
>>  A patch that's currently on review[3] ("[v3,net] ice: Write all GNSS 
>>  buffers instead of first one") would add one more problem:
>>  - The possibility of waiting for a very long time to flush the write
>>     work when doing rmmod, softlockups.
>>  
>>  To fix these issues, simplify the implementation: Drop the buffering, 
>>  the write_work, and make the writes synchronous.
>>  
>>  I tested this with gpsd and ubxtool.
>>  
>>  [1] https://events19.linuxfoundation.org/wp-content/uploads/2017/12/The-GNSS-Subsystem-Johan-Hovold-Hovold-Consulting-AB.pdf
>>      "User interface" slide.
>>  [2] A comment in drivers/gnss/core.c:gnss_write():
>>          /* Ignoring O_NONBLOCK, write_raw() is synchronous. */ [3] 
>>  https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20230217120
>>  541.16745-1-karol.kolacinski@...el.com/
>>  
>>  Fixes: d6b98c8d242a ("ice: add write functionality for GNSS TTY")
>>  Signed-off-by: Michal Schmidt <mschmidt@...hat.com>
>>  ---
>>  v2: No real change. Just rebased, per Michal Michalik's request.
>
> Reviewed-by: Simon Horman <simon.horman@...igine.com>
>
> Added Johan Hovold to CC list in case he has any feedback.
>
Tested-by: Sunitha Mekala <sunithax.d.mekala@...el.com> (A Contingent worker at Intel)