lkml.org 
[lkml]   [2019]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] Input: uinput: Avoid Object-Already-Free with a global lock
On Wed, Apr 24, 2019 at 07:39:03PM +0530, Mukesh Ojha wrote:

> This was my simple program no multithreading just to understand f_counting
>
> int main()
> {
>         int fd = open("/dev/uinput", O_WRONLY | O_NONBLOCK);
>         ioctl(fd, UI_SET_EVBIT, EV_KEY);
>         close(fd);
>         return 0;
> }
>
>            uinput-532   [002] ....    45.312044: SYSC_ioctl: 2   <= f_count

Er... So how does it manage to hit ioctl(2) before open(2)? Confused...

> >    <After fdget()
>           uinput-532   [002] ....    45.312055: SYSC_ioctl: 2           
> <After fdput()
>           uinput-532   [004] ....    45.313766: uinput_open: uinput: 1   /*
> This is from the uinput driver uinput_open()*/
>
>   =>>>>                         /* All the above calls happened for the
> open() in userspace*/
>
>           uinput-532   [004] ....    45.313783: SYSC_ioctl: 1 /* This print
> is for the trace, i put after fdget */
>           uinput-532   [004] ....    45.313788: uinput_ioctl_handler:
> uinput: uinput_ioctl_handler, 1 /* This print is from the uinput_ioctl
> driver */
>
>           uinput-532   [004] ....    45.313835: SYSC_ioctl: 1 /* This print
> is for the trace, i put after fdput*/
>           uinput-532   [004] ....    45.313843: uinput_release: uinput:  0
> /* And this is from the close()  */
>
>
> Should fdget not suppose to increment the f_count here, as it is coming 1 ?
> This f_count to one is done at the open, but i have no idea how this  below
> f_count 2 came before open() for
> this simple program.

If descriptor table is not shared, fdget() will simply return you the reference
from there, without bothering to bump the refcount. _And_ having it marked
"don't drop refcount" in struct fd.

Rationale: since it's not shared, nobody other than our process can modify
it. So unless we remove (and drop) the reference from it ourselves (which
we certainly have no business doing in ->ioctl() and do not do anywhere
in drivers/input), it will remain there until we return from syscall.

Nobody is allowed to modify descriptor table other than their own.
And if it's not shared, no other owners can appear while the only
existing one is in the middle of syscall other than clone() (with
CLONE_FILES in flags, at that).

For shared descriptor table fdget() bumps file refcount, since there
the reference in descriptor table itself could be removed and dropped
by another thread.

And fdget() marks whether fput() is needed in fd.flags, so that
fdput() does the right thing.

\
 
 \ /
  Last update: 2019-04-25 00:57    [W:2.242 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site