Re: [PATCH v11 0/2 hurd] Add irqhelp library and clean up ddekit
Hi, On 24 Mar 2024, 11:05 pm, Samuel Thibault < samuel.thiba...@gnu.org> wrote: > Ok, but there's not even a comment about it. If somebody else comes and > changes the code a bit, they'll very easily get to the unsafe side again. And > really, accessing local variables from nested functions is something we want > to kill because it ends up making the stack executable. OK I will attempt to remove the local access from nested function in v12. Damien
Re: [PATCH v11 0/2 hurd] Add irqhelp library and clean up ddekit
Damien Zammit, le dim. 24 mars 2024 11:40:27 +, a ecrit: > \ Original Message > On 24 Mar 2024, 9:32 pm, Samuel Thibault > >... Your version 11 is however still accessing the \`irq\` local parameter, > >so it's still only by luck that it's working. You need to restore allocating > >the params structure to store irq and priv, which was precisely meant to > >avoid this kind issue. > > Actually, the ddekit semaphore protects the irq variable from disappearing > because the function is guaranteed to not return until after the semaphore > has signalled it has run the thread\_init and used the irq local. Ok, but there's not even a comment about it. If somebody else comes and changes the code a bit, they'll very easily get to the unsafe side again. And really, accessing local variables from nested functions is something we want to kill because it ends up making the stack executable. Samuel
Re: [PATCH v11 0/2 hurd] Add irqhelp library and clean up ddekit
Hi Samuel, Original Message On 24 Mar 2024, 9:32 pm, Samuel Thibault >... Your version 11 is however still accessing the `irq` local parameter, so >it's still only by luck that it's working. You need to restore allocating the >params structure to store irq and priv, which was precisely meant to avoid >this kind issue. Actually, the ddekit semaphore protects the irq variable from disappearing because the function is guaranteed to not return until after the semaphore has signalled it has run the thread_init and used the irq local. Damien
Re: [PATCH v11 0/2 hurd] Add irqhelp library and clean up ddekit
Damien Zammit, le dim. 24 mars 2024 10:08:30 +, a ecrit: > The previous problem was that calling a function with a global param > from inside the wrapped_server_loop was crashing netdde > due to a bogus stack offset. This is fixed. Ah, it's not a global param, but a local param. Passing a nested function that access local parameters to a thread is indeed a way to mayhem since as soon as the caller function exits after creating the thread, the local parameters don't exist any more. Also note that such local access from nested function is exactly what makes the stack executable because gcc has to put some trampoline there. Your version 11 is however still accessing the `irq` local parameter, so it's still only by luck that it's working. You need to restore allocating the params structure to store irq and priv, which was precisely meant to avoid this kind issue. Samuel
