Re: [PATCH] pagealign_alloc: don't loop endlessly
Derek Price dixit: My patch logs the actual error message to the syslog too (via the vsyslog() call), but if your chroot is preventing access to the syslog, that wouldn't help anyhow. Yes, that's why I was suggesting to print the error message to stderr so at least the client gets an idea about what happened and can slap the server administrator. CVS does this for quite some occasions, for example no space available in the server's temporary directory. //mirabile -- Hi, does anyone sell openbsd stickers by themselves and not packaged with other products? No, the only way I've seen them sold is for $40 with a free OpenBSD CD. -- Haroon Khalid and Steve Shockley in gmane.os.openbsd.misc ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [PATCH] pagealign_alloc: don't loop endlessly
Derek Price dixit: Can you provide me with a test case that reproduces this behavior? Sorry, not at the moment, at least not without a great effort. I will do as soon as I've reinstalled my boxen. I could do a temporary test scenario, but we needed to be in sync for that. Join irc.oftc.net and /msg mira if you're interested. //mirabile -- Hi, does anyone sell openbsd stickers by themselves and not packaged with other products? No, the only way I've seen them sold is for $40 with a free OpenBSD CD. -- Haroon Khalid and Steve Shockley in gmane.os.openbsd.misc ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [PATCH] pagealign_alloc: don't loop endlessly
Derek Price dixit: Yes, that's why I was suggesting to print the error message to stderr so at least the client gets an idea about what happened and can slap the server administrator. CVS does this for quite some occasions, for example no space available in the server's temporary directory. Could you point me at the code that does this? Not out of the blue, I had to check. But it happens frequently when e.g. the OpenBSD anoncvs servers are overloaded. How much good would sending the message to stderr really do? This problem can only occur on the server, where sending to stderr, at best, can only be reported to the client as an unrecognized protocol response, if reported at all, once the network buffers are disabled (the network buffer code is what is calling pagealign_alloc() here). No, it's actually displayed by the client (with my first diff). bye, //mirabile -- Hi, does anyone sell openbsd stickers by themselves and not packaged with other products? No, the only way I've seen them sold is for $40 with a free OpenBSD CD. -- Haroon Khalid and Steve Shockley in gmane.os.openbsd.misc ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [PATCH] pagealign_alloc: don't loop endlessly
Does the attached, more general, patch do the trick for you? I think the problem is deeper - error() really needs a way to bypass the memory allocation of the buffer routines, but this should work around the problem for now. Cheers, Derek Thorsten Glaser wrote: Hello! Please apply the following patch. It prevents GNU CVS 1.12.12 from looping endlessly and dumping core due to lack of stack (recursion depth) when /dev/zero cannot be found, for example in a chroot(2) environment. I have copyright assignments for CVS filed with the FSF. //mirabile Index: src/error.c === RCS file: /cvs/ccvs/src/error.c,v retrieving revision 1.42 diff -u -p -r1.42 error.c --- src/error.c 18 Mar 2005 16:41:24 - 1.42 +++ src/error.c 4 May 2005 14:14:29 - @@ -119,6 +119,10 @@ error (int status, int errnum, const cha char statcmdbuf[32]; char *cmdbuf; char *emptybuf = ; +static bool in_error = false; + +if (in_error) goto recursion_error; +in_error = true; /* Initialize these to avoid a lot of special case error handling. */ buf = statbuf; @@ -173,6 +177,9 @@ error (int status, int errnum, const cha /* Restore errno per our charter. */ errno = save_errno; +/* Reset our recursion lock. */ +in_error = false; + /* Done. */ return; @@ -195,6 +202,21 @@ memerror: #endif /* HAVE_SYSLOG_H */ exit (EXIT_FAILURE); + +recursion_error: +#if HAVE_SYSLOG_H +syslog (LOG_DAEMON | LOG_EMERG, + error (%d, %d) called recursively. Message was:, + status, errnum); + +va_start (args, message); +vsyslog (LOG_DAEMON | LOG_EMERG, message, args); +va_end (args); + +syslog (LOG_DAEMON | LOG_EMERG, Aborting.); +#endif /* HAVE_SYSLOG_H */ + +exit (EXIT_FAILURE); } signature.asc Description: OpenPGP digital signature ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [PATCH] pagealign_alloc: don't loop endlessly
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Thorsten Glaser wrote: Derek Price dixit: Yes, that's why I was suggesting to print the error message to stderr so at least the client gets an idea about what happened and can slap the server administrator. CVS does this for quite some occasions, for example no space available in the server's temporary directory. Could you point me at the code that does this? Not out of the blue, I had to check. But it happens frequently when e.g. the OpenBSD anoncvs servers are overloaded. And where is the code that does this? How much good would sending the message to stderr really do? This problem can only occur on the server, where sending to stderr, at best, can only be reported to the client as an unrecognized protocol response, if reported at all, once the network buffers are disabled (the network buffer code is what is calling pagealign_alloc() here). No, it's actually displayed by the client (with my first diff). I'm fairly certain it shouldn't be consistently displayed this way. It might ocassionally appear in the context of Unrecognized response from server: `some message on stderr'. Can you provide me with a test case that reproduces this behavior? Cheers, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCeQxpLD1OTBfyMaQRAgrVAJ9ZeNqHtuhH3dTjgZjAMLLpvr53qQCdFOWC 2Ccd7RA08CDF3ceEpfpmFtc= =bics -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [PATCH] pagealign_alloc: don't loop endlessly
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Thorsten Glaser wrote: Derek Price dixit: Does the attached, more general, patch do the trick for you? It does, but I do not get a diagnostic, neither on stderr (CVS output) nor in syslog on server or client. This is probably because the CVS server is running in a chroot. I suggest a diagnostics on stderr, plus - in this case - the actual solution is to add a device node into the chroot, and from a recursion error nobody would get the idea. My patch logs the actual error message to the syslog too (via the vsyslog() call), but if your chroot is preventing access to the syslog, that wouldn't help anyhow. Cheers, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCeN8FLD1OTBfyMaQRAiHfAKDcLW4aGk+Ek9JHVeTSVbUqERJiqQCguXyo Xcovfa0jOnzz+159lb0oJNw= =RGRl -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [PATCH] pagealign_alloc: don't loop endlessly
I've attached a revised version of my earlier patch that preserves more information and avoids problems when the exit handlers call error() after error() calls exit(). Regards, Derek Index: src/ChangeLog === RCS file: /cvs/ccvs/src/ChangeLog,v retrieving revision 1.3186 diff -u -p -r1.3186 ChangeLog --- src/ChangeLog 4 May 2005 02:48:16 - 1.3186 +++ src/ChangeLog 4 May 2005 20:14:40 - @@ -1,3 +1,7 @@ +2005-05-04 Derek Price [EMAIL PROTECTED] + + * error.c (error): Avoid recursion and syslog the problem. + 2005-05-03 Derek Price [EMAIL PROTECTED] * tag.c (is_in_val_tags): Remove unnecessary existance checking for the Index: src/error.c === RCS file: /cvs/ccvs/src/error.c,v retrieving revision 1.42 diff -u -p -r1.42 error.c --- src/error.c 18 Mar 2005 16:41:24 - 1.42 +++ src/error.c 4 May 2005 20:14:40 - @@ -120,16 +120,26 @@ error (int status, int errnum, const cha char *cmdbuf; char *emptybuf = ; +static const char *last_message = NULL; +static va_list last_args; +static int last_status; +static int last_errnum; + +if (last_message) goto recursion_error; +last_message = message; +va_start (args, message); +last_args = args; +last_status = status; +last_errnum = errnum; + /* Initialize these to avoid a lot of special case error handling. */ buf = statbuf; buf2 = statbuf2; cmdbuf = emptybuf; /* Expand the message the user passed us. */ -va_start (args, message); length = sizeof (statbuf); buf = vasnprintf (statbuf, length, message, args); -va_end (args); if (!buf) goto memerror; /* Expand the cvs commmand name to cmd or [cmd aborted]. @@ -161,6 +171,12 @@ error (int status, int errnum, const cha /* Send the final message to the client or log it. */ cvs_outerr (buf2, length); +/* Reset our recursion lock. This needs to be done before the call to + * exit() to allow the exit handlers to make calls to error(). + */ +last_message = NULL; +va_end (args); + /* Done, if we're exiting. */ if (status) exit (EXIT_FAILURE); @@ -194,6 +210,37 @@ memerror: syslog (LOG_DAEMON | LOG_EMERG, Memory exhausted. Aborting.); #endif /* HAVE_SYSLOG_H */ +goto sidestep_done; + +recursion_error: +#if HAVE_SYSLOG_H +/* Syslog the problem since recursion probably means that we encountered an + * error while attempting to send the last error message to the client. + */ + +syslog (LOG_DAEMON | LOG_EMERG, + error (%d, %d) called recursively. Original message was:, + last_status, last_errnum); +vsyslog (LOG_DAEMON | LOG_EMERG, last_message, last_args); + + +syslog (LOG_DAEMON | LOG_EMERG, +error (%d, %d) called recursively. Second message was:, + status, errnum); +va_start (args, message); +vsyslog (LOG_DAEMON | LOG_EMERG, message, args); +va_end (args); + +syslog (LOG_DAEMON | LOG_EMERG, Aborting.); +#endif /* HAVE_SYSLOG_H */ + +sidestep_done: +/* Reset our recursion lock. This needs to be done before the call to + * exit() to allow the exit handlers to make calls to error(). + */ +last_message = NULL; +va_end (last_args); + exit (EXIT_FAILURE); } signature.asc Description: OpenPGP digital signature ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
