Jeff Moyer wrote:
==> Regarding Re: [autofs] Race in expire code; [EMAIL PROTECTED] adds:

raven> On Sat, 29 Jan 2005, Steinar H. Gunderson wrote:

On Sat, Jan 29, 2005 at 09:36:31PM +0800, [EMAIL PROTECTED] wrote:

Can everyone who is maintaining downstream packages please chaeck that
the code in their package ends up looking like what the patch below
acheives.

[...]

case EXP_STARTED: - sigprocmask(SIG_SETMASK, &ready_sigs, NULL);
ap.state = ST_EXPIRE; + sigprocmask(SIG_SETMASK, &ready_sigs, NULL);
return 0; } return 1;

The Debian packages are close, but not quite:

case EXP_STARTED: sigprocmask(SIG_SETMASK, &lock_sigs, NULL); ap.state =
ST_EXPIRE; sigprocmask(SIG_SETMASK, &ready_sigs, NULL); return 0;

I have no idea what lock_sigs does, but this should be OK, no?


raven> It's just the list of signals that are blocked while we do "stuff"
raven> that we don`t want interrupted by something else.

raven> Similarly ready_sigs contains the signals we expect to be received
raven> in normal operation.

raven> The first sigprocmask call above does nothing as signals are already
raven> blocked at that point.

The Red Hat package also has the two calls to sigprocmask.  When "fixing"
signal races, I definitely managed to introduce a similar bug.  Chris Feist
fixed it.  Chris, do you agree with Ian on this matter, that the first
sigprocmask is superfluous?

Yes, I think I just included it to be safe, but it should be able to be safely removed.


The reason that we needed to make the 'ap.state = ST_EXPIRE' run before enabling ready_sigs, is as soon as we enable sigprocmask the kernel sends down a queued up SIGCHLD which then runs the sigchld event handler which doesn't realize that we're in ST_EXPIRE (because it hasn't been set yet).

Thanks,
Chris

_______________________________________________
autofs mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/autofs

Reply via email to