> Date: Thu, 20 Jun 2013 11:30:11 +0100 > From: Stuart Henderson <[email protected]> > > On 2013/06/20 09:38, Otto Moerbeek wrote: > > On Wed, Jun 19, 2013 at 07:39:15PM +0000, Miod Vallat wrote: > > > > > > > > - p->p_sigmask = mask &~ sigcantmask; > > > > > > + p->p_sigmask = mask; > > > > > > > > On the right architecture where a word store isn't atomic enough and > > > > with the right compiler that decides to put p_sigmask on an address > > > > ending with 0xFFF with 4k-sized pages, we have two problems already. > > > > > > Holy Pumpkin forbid, struct proc layout is sane enough for fields to be > > > aligned to their natural alignment, and the allocator will return > > > properly aligned structs as well. > > > > > > > I'm only asking if such a situation can happen, or if there is some > > > > ensure_this_assignment_is_always_atomic(&p->p_sigmask, mask); function > > > > that I missed. > > > > > > If such a function exists, it should be shot, if only for having a too > > > long name. > > > > > > Miod > > > > But watch out, as kettenis@ already mentioned elsewhere, this can only > > be safe if *all* reads and modifications to the mask are done in an > > atomic way. > > > > Straight read and assignment of int values is safe, afaik, but e.g. > > struct assignment and memcpy are *not* atomic.
And p_sigmask is copied during fork, so that needs a bit of thought. I guess it doesn't matter for the new child, as it isn't running yet and therefore can't invoke sigprocmask(2). And the parent should be safe as well, as it is in fork(2) and therefore can't call sigprocmask(2) either. > I spotted a few places where p_sigmask is &= or |='d, for example these > from linux compat, might these be an issue? > > p->p_sigmask &= ~bs; > p->p_sigmask |= bs & ~sigcantmask; There's also a couple of additional p->p_sigmask |= ... in kern_sig.c. All of these need to become atomic_setbits_int/atomic_clearbits_int calls.
