Nicolas Williams wrote: > On Mon, Jun 15, 2009 at 09:24:26PM +0200, Darren Reed wrote: >> http://cr.opensolaris.org/~darrenr/6271923-20090615/ >> >> It includes a fix for the bug identified by Nicolas earlier, 6851264. > > Hmmm, I think you could still loop: > > + int which = RCTL_FIRST; > > - while (pr_getrctl(Pr, ctl_name, NULL, blk, RCTL_FIRST) != -1 && > + while (pr_getrctl(Pr, ctl_name, NULL, blk, which) != -1 && > rctlblk_get_privilege(blk) != RCPRIV_SYSTEM) { > - if (pr_setrctl(Pr, ctl_name, NULL, blk, RCTL_DELETE) != 0) > + if (pr_setrctl(Pr, ctl_name, NULL, blk, RCTL_DELETE) != 0) { > failed++; > + which = RCTL_NEXT; > + } else { > + which = RCTL_FIRST; > + } > > Perhaps I'm being thick, but it seems to me that we're always re-trying > on pr_setrctl() failure, without a limit to how many times we re-try -- > at some point you've got to give up, even if it seems wrong for > pr_setrctl() to fail repeatedly.
If we have a list of 3 resources, A, B and C, lets assume that all 3 are not RCPRIV_SYSTEM resources. The first call to pr_getrctl() will return "A". Using the modified code, if the RCTL_DELETE on "A" fails then which becomes RCTL_NEXT and the next call to pr_getrctl() will return "B". We delete this successfully and start over because 'which' is set back to RCTL_FIRST. The basis for me doing this is I'm assuming that the RCTL_DELETE on "B" could cause unexpected things to happen if I do another pr_getrcl() with RCTL_NEXT - the thing I want to go "next" from is now gone. So "B" has been deleted and we see "A" again, which again returns an error, so 'which' gets set to RCTL_NEXT and then we return "C". If deleting "C" returns an error then we try to pr_getrctl() again and oops, we're at the end. If "C" doesn't return an error, then 'which' gets reset back to RCTL_FIRST, the next pr_getrctl() call returns "A", RCTL_DELETE fails so 'which' becomes RCTL_NEXT and we get to the end. It might seem silly to retry RCTL_DELETE on "A" three times but short of creating another "work list", it's just a hazard of the implementation. To examine other situations, if RCTL_DELETE fails for each of "A", "B" and "C", then the list is walked once (RCTL_FIRST, RCTL_NEXT, RCTL_NEXT, RCTL_NEXT) and SETFAILED is returned. If "A" is the only one deleted, the sequence is RCTL_FIRST, RCTL_FIRST, RCTL_NEXT, RCTL_NEXT. If you can splot any flaws or something I've missed in the logic, please work it through and demonstrate the problem. If it helps, I'll expand on the comments for that loop so that it's clear about what's going on. Darren
