Re: [PATCH v2] Input: uinput: Avoid Object-Already-Free with a global lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/23/2019 4:36 PM, Al Viro wrote:
On Tue, Apr 23, 2019 at 08:49:44AM +0000, dmitry.torokhov@xxxxxxxxx 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




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux