svn commit: r263755 - head/sys/kern

David Xu davidxu at freebsd.org
Fri Mar 28 01:43:38 UTC 2014


On 2014/03/27 22:58, Mateusz Guzik wrote:
> On Thu, Mar 27, 2014 at 04:46:57PM +0800, David Xu wrote:
>> On 2014/03/27 16:37, Mateusz Guzik wrote:
>>> On Thu, Mar 27, 2014 at 03:45:17PM +0800, David Xu wrote:
>>>> I think the async process pointer can be cleared when a process exits
>>>> by registering an event handler. please see attached patch.
>>>>
>>>
>>> Sure, but I'm not very fond of this solution.
>>>
>>> This is a rather obscure bug you wont hit unless you explicitly try,
>>> and even then you need root privs by default.
>>>
>> OK, but I don't like the bug exists in kernel. It is not obscure for me,
>> I can run "shutdown now" command, and insert a device, and then the
>> kernel will write garbage data into freed memory space.
>>
>
> Not sure what you mean. devd does not use this feature, and even if it
> did async_proc is cleared on close, which happens while signal delivery
> is still legal.
>
Thers is no race conidtion when using lock to protect it.

> That said, you are not going to encounter this bug unless you code
> something up to specifically trigger it.
>
> fwiw, I think we could axe this feature if there was no way to fix it
> without introducing a check for every process.
>

If you added a feature, you can not assume people won't use it.

>>> As such writing a callback function which will be executed for all exiting
>>> processes seems unjustified for me.
>>>
>>> Ideally we would get some mechanism which would allow to register
>>> callbacks for events related to given entity. Then it could be used to
>>> provide a "call this function when process p exits", amongst other things.
>>>
>>
>> Yes, but the callback itself is cheap enough and is not worth to be
>> per-entity entry.
>>
>
> There is other code in the kernel which would benefit from such
> functionality - dev/syscons/scmouse, dev/vt/vt_core.c, aio and possibly
> more.
>
> As such I think this is worth pursuing.
>
I can not say more, it seems it is an extension.



More information about the svn-src-head mailing list