Messages in this thread | | | Subject | Re: [PATCH v2] Input: uinput: Avoid Object-Already-Free with a global lock | From | Mukesh Ojha <> | Date | Wed, 24 Apr 2019 17:40:40 +0530 |
| |
On 4/23/2019 4:36 PM, Al Viro wrote: > On Tue, Apr 23, 2019 at 08:49:44AM +0000, dmitry.torokhov@gmail.com wrote: >> On Tue, Apr 23, 2019 at 12:51:13PM +0530, Mukesh Ojha wrote: >>> I have taken care this case from ioctl and release point of view. >>> >>> Even if the release gets called first it will make the >>> file->private_data=NULL. >>> and further call to ioctl will not be a problem as the check is already >>> there. >> Al, do we have any protections in VFS layer from userspace hanging onto >> a file descriptor and calling ioctl() on it even as another thread >> calls close() on the same fd? >> >> Should the issue be solved by individual drivers, or more careful >> accounting for currently running operations is needed at VFS layer? > Neither. An overlap of ->release() and ->ioctl() is possible only > if you've got memory corruption somewhere. > > close() overlapping ioctl() is certainly possible, and won't trigger > that at all - sys_ioctl() holds onto reference to struct file, so > its refcount won't reach zero until we are done with it.
Al,
i tried to put traceprintk inside ioctl after fdget and fdput on a simple call of open => ioctl => close on /dev/uinput.
uinput-532 [002] .... 45.312044: SYSC_ioctl: 2 <= f_count > <After fdget() uinput-532 [002] .... 45.312055: SYSC_ioctl: 2 <After fdput() uinput-532 [004] .... 45.313766: uinput_open: uinput: 1 uinput-532 [004] .... 45.313783: SYSC_ioctl: 1 uinput-532 [004] .... 45.313788: uinput_ioctl_handler: uinput: uinput_ioctl_handler, 1 uinput-532 [004] .... 45.313835: SYSC_ioctl: 1 uinput-532 [004] .... 45.313843: uinput_release: uinput: 0
So while a ioctl is running the f_count is 1, so a fput could be run and do atomic_long_dec_and_test this could call release right ?
-Mukesh
| |