On Thu, Mar 27, 2014 at 04:05:12PM +0100, Mateusz Guzik wrote: > On Thu, Mar 27, 2014 at 03:58:19PM +0100, 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. > > > > 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. > > > > > >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. > > > > We can hack around this one the way the other code is doing - apart from > from proc pointer you store pid and then compare result of pfind(pid). > > This is still buggy as both proc and pid pointer can be recycled and end > up being the same (but you have an entrirely new process). > > However, then in absolutely worst cae you send SIGIO to incorrect > process, always an existing process so no more corruption. > > Would you be ok with such hack for the time being?
Isn't p_sigiolist and fsetown(9) already provide the neccessary registration and cleanup on the process exit ? The KPI might require some generalization, but I think that the mechanism itself is enough.
pgp8QUuUidTo7.pgp
Description: PGP signature
