RTDM module ownership

RTDM module ownership

[Richard Weinberger]

Hi!

I'm working on a kernel module which is used on plain Linux and
Xenomai (RTDM), while reviewing RTDM to understand refcounting I found
something that is not entirely clear to me.

Why does __rtdm_dev_open() not grab a reference on the RTDM module owner?
This leads to the case where one can rmmod the module while it is in use.
Unloading will block uninterruptible in rtdm_dev_unregister() in the
module's cleanup funktion.

In contrast, on regular Linux rmmod will refuse to unload the module
if the device node is still open because fops_get() or other helpers
gained a reference on the owner.

-- 
Thanks,
//richard

Re: RTDM module ownership

[Jan Kiszka]

On 02.07.20 10:32, Richard Weinberger via Xenomai wrote:
> Hi!
> 
> I'm working on a kernel module which is used on plain Linux and
> Xenomai (RTDM), while reviewing RTDM to understand refcounting I found
> something that is not entirely clear to me.
> 
> Why does __rtdm_dev_open() not grab a reference on the RTDM module owner?
> This leads to the case where one can rmmod the module while it is in use.
> Unloading will block uninterruptible in rtdm_dev_unregister() in the
> module's cleanup funktion.
> 
> In contrast, on regular Linux rmmod will refuse to unload the module
> if the device node is still open because fops_get() or other helpers
> gained a reference on the owner.
> 

First of all, your driver is apparently not reacting on the close 
request that it receives in that case. This leads the the stall you see.

Still, we could indeed run some module_put/get on open/socket/close. I 
thought we did, but that was once RTnet [1].

Jan

[1] 
https://gitlab.denx.de/Xenomai/xenomai/-/commit/11bbdef0e9486d8f952ea29023882449331af1aa

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: RTDM module ownership

[Richard Weinberger]

----- Ursprüngliche Mail -----
>> In contrast, on regular Linux rmmod will refuse to unload the module
>> if the device node is still open because fops_get() or other helpers
>> gained a reference on the owner.
>> 
> 
> First of all, your driver is apparently not reacting on the close
> request that it receives in that case. This leads the the stall you see.

Huh? rmmmod triggers close of what?
*confused*
 
> Still, we could indeed run some module_put/get on open/socket/close. I
> thought we did, but that was once RTnet [1].

Yes. Would be nice. :-)

Thanks,
//richard

Re: RTDM module ownership

[Jan Kiszka]

On 03.07.20 08:22, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>>> In contrast, on regular Linux rmmod will refuse to unload the module
>>> if the device node is still open because fops_get() or other helpers
>>> gained a reference on the owner.
>>>
>>
>> First of all, your driver is apparently not reacting on the close
>> request that it receives in that case. This leads the the stall you see.
> 
> Huh? rmmmod triggers close of what?
> *confused*
>   

rmmod -> module cleanup -> rtdm_dev_unregister -> rtdm_device_flush_fds

>> Still, we could indeed run some module_put/get on open/socket/close. I
>> thought we did, but that was once RTnet [1].
> 
> Yes. Would be nice. :-)
> 

Just like a patch would be...

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: RTDM module ownership

[Richard Weinberger]

----- Ursprüngliche Mail -----
>>> First of all, your driver is apparently not reacting on the close
>>> request that it receives in that case. This leads the the stall you see.
>> 
>> Huh? rmmmod triggers close of what?
>> *confused*
>>   
> 
> rmmod -> module cleanup -> rtdm_dev_unregister -> rtdm_device_flush_fds

Sure. The driver has a close method where it cleans up everything, of course.
But it can't force userspace to close the fd, Linux has no fd revocation
mechanism.
 
>>> Still, we could indeed run some module_put/get on open/socket/close. I
>>> thought we did, but that was once RTnet [1].
>> 
>> Yes. Would be nice. :-)
>> 
> 
> Just like a patch would be...

Pushed to my TODO...

Thanks,
//richard