On Sat, Dec 17, 2005 at 11:02:11AM +0100, Mathieu Roy wrote:
> vendredi 16 décembre, vers 22h, Sylvain Beucler écrivit :
>
> > I ran into a nasty issue yesterday at Savannah, while trying to
> > recreate the users public keys.
> >
> > I got overwhelmed with numerous sv_users processes. Here's what I
> > think is the cause:
> >
> > > From Groups.pm:
> > my $pid = open (GPG, "|-");
> >
> > if ($pid) { # parent
> > print GPG $key;
> > close (GPG);
> > return $?;
> > } else { # child
> > exec (@gpg_args) || return 1;
> > }
> >
> > If exec fails, what happens? We have two processes, the father that
> > will return after reading nothing on GPG, _and_ the son that will
> > 'return 1' after failing to exec. Back in sv_users, they will both
> > update the remaining keys. Repeat for at least 15 users and you get a
> > fork bomb :/
> >
> > (I'm not talking about security, just misconfiguration)
> >
> > For reference, it may also be of interest that if two "children" try
> > to access MySQL, you can get a "MySQL server has gone away" error.
> > You should be able to reproduce the error by tweaking:
> >
> > perl -MSavane -e 'print GetGroupName("101") . "\n"; open(FORK, "|-") ||
> > sleep
> > 1; print GetGroupName("102") . "\n";'
> >
> > If you already got this kind of errors, that's a path
> > to investigate.
> >
> > About the issue, I suggest we change "return 1" into a die
> > statement. If I'm right, this is an important point to keep in mind
> > when programming :)
>
> I think I grabbed part of this from the work done by someone else,
> modifying it a bit, without taking time to really understand all it
> involves.
> So far, I did not noticed problems. Indeed, since the bug seems to
> reside in error handling, as I never experienced errors, I had no
> issue.
>
> Using die seems fine. I cannot explain why there was a return 1, maybe
> it made sense from where I grabbed the code but not here.
>
I actually added this code during the Savannah merge one year ago.
http://cvs.gna.org/cvsweb/savane.old-tree/lib/Savannah/Attic/User.pm.diff?r1=1.31.2.1;r2=1.31.2.2;cvsroot=savane;only_with_tag=DEV_2004-12-28_Savannah;f=h
It comes from [EMAIL PROTECTED] I'm not blaming anybody, just
discussing a possible issue (and a possible fix).
Incidentally, I cannot find a way to retrieve that information from
the SVN repository :'(
> I'm actually not quite sure that we want a die(). GPG stuff is extra
> bonus. If it fails, we do not necessarily want the rest of sv_users to
> fail.
> If it fails, the error should probably be logged but sv_users should
> probably continue, what do you think?
That's what I think, but the problem is a bit more complicated: when
you are in the exec block, you already forked, and if you don't kill
the fork when exec fails, then you get two concurrent 'sv_user'. Then
those two sv_users can give birth to 4 'sv_user', etc.
If you kill the fork when exec fails, the father continues to live.
--
Sylvain
