linux-kernel - Re: [PATCH v2] staging: pi433: add mutex fixing concurrency issues.

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180607124216.GA2329@hle-laptop.local>
Date:   Thu, 7 Jun 2018 08:45:03 -0400
From:   Hugo Lefeuvre <hle@....eu.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        kernelnewbies@...nelnewbies.org
Subject: Re: [PATCH v2] staging: pi433: add mutex fixing concurrency issues.

> Add a mutex fixing a potential NULL pointer dereference in the pi433
> driver.
> 
> If pi433_release and pi433_ioctl are concurrently called,
> pi433_release might set filp->private_data to NULL while pi433_ioctl
> is still accessing it, leading to NULL pointer dereference. This issue
> might also affect pi433_write and pi433_read.
> 
> The newly introduced mutex makes sure that instance data
> will not be modified simultaneously by pi433_release, pi433_write,
> pi433_read or pi433_ioctl.
> 
> The mutex is stored in a newly introduced struct pi433_data, which
> wraps struct pi433_instance and its mutex.
> 
> Make filp->private_data point to a struct pi433_data, allowing to
> acquire the lock before accessing the struct pi433_instance.
> 
> Signed-off-by: Hugo Lefeuvre <hle@....eu.com>
> ---
> Changes in v2:
>  	- Use mutex instead of rw semaphore. 
>  	- Introduce struct pi433_data in order to allow functions to lock
>  	  before dereferencing instance pointer.
>  	- Make filp->private_data point to a struct pi433_data.
>  	- Add missing braces.

After discussing this issue on the kernel newbies mailing list[0] we
came to the conclusion that it is very unlikely that pi433_release and
pi433_ioctl would ever run concurrently in this case. This is also
true for read/write. Unless one can find a situation where this might
happen, I think we should not add this potentially unnecessary lock.

Regards,
 Hugo

[0] http://lists.kernelnewbies.org/pipermail/kernelnewbies/2018-June/019131.html 

-- 
             Hugo Lefeuvre (hle)    |    www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA