Bug#573638: [rrd-developers] Bug#573638: rrdtool: rrdcgi crashes at printlasttime()
Tobias Oetiker writes: Hi Tobias, > in 2012:2013 I put in the patch below ... this should provide a > more generic fix for the problem. rrd_cgi functions all used to > fail horribly when a librrd function tried to touch ARGV[0] (mostly The patch fixes the crash, but you still need to fix the printlasttime() function by removing `+ 1' in line 991. I mean replacing last = rrd_last(argc + 1, (char **) args - 1); with last = rrd_last(argc, (char **) args - 1); Without the change, iptotal gives the following output: Network usage Database last updated:[ERROR: Usage: rrdtool rrdcgi [--daemon ] ] Server Time: Sun Mar 14 21:36:35 20 After removing the `+ 1' thing, iptotal works as expected and prints time instead of the usage error message. Thanks a lot, Robert -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#573638: [rrd-developers] Bug#573638: rrdtool: rrdcgi crashes at printlasttime()
Hi Robert, Yesterday Robert Luberda wrote: > Tobias Oetiker writes: > > Hi Tobias, > > > in 2012:2013 I put in the patch below ... this should provide a > > more generic fix for the problem. rrd_cgi functions all used to > > fail horribly when a librrd function tried to touch ARGV[0] (mostly > > > The patch fixes the crash, but you still need to fix the printlasttime() > function by removing `+ 1' in line 991. I mean replacing > last = rrd_last(argc + 1, (char **) args - 1); > with > last = rrd_last(argc, (char **) args - 1); > > Without the change, iptotal gives the following output: > Network usage > Database last updated:[ERROR: Usage: rrdtool rrdcgi [--daemon ] ] > Server Time:Sun Mar 14 21:36:35 20 > > After removing the `+ 1' thing, iptotal works as expected and prints > time instead of the usage error message. right, because the strftime argument is used locally and not passed on to rrd_last ... fix is in r2029:2030 thanks tobi > > Thanks a lot, > Robert > > -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch t...@oetiker.ch ++41 62 775 9902 / sb: -9900 -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#573638: [rrd-developers] Bug#573638: rrdtool: rrdcgi crashes at printlasttime()
Hi Robert,
in 2012:2013 I put in the patch below ... this should provide a
more generic fix for the problem. rrd_cgi functions all used to
fail horribly when a librrd function tried to touch ARGV[0] (mostly
to print an errror message including the name name of the calling
program) ...
obviously an even better fix would be to remove the whole argv
magic from rrd_cgi ... but this patch would be rather substantial.
cheers
tobi
--- rrd_cgi.c (revision 2012)
+++ rrd_cgi.c (working copy)
@@ -1053,7 +1053,7 @@
int curarg_contains_rrd_directives;
/* local array of arguments while parsing */
-int argc = 0;
+int argc = 1;
char**argv;
#ifdef DEBUG_PARSER
@@ -1069,6 +1069,7 @@
if (!argv) {
return NULL;
}
+argv[0] = "rrdcgi";
/* skip leading blanks */
while (isspace((int) *line)) {
@@ -1172,7 +1173,7 @@
argv[argc - 1] = rrd_expand_vars(stralloc(argv[argc - 1]));
}
#ifdef DEBUG_PARSER
-if (argc > 0) {
+if (argc > 1) {
int n;
printf("<-- arguments found [%d]\n", argc);
@@ -1186,9 +1187,18 @@
#endif
/* update caller's notion of the argument array and it's size */
-*arguments = argv;
-*argument_count = argc;
+/* note this is a bit of a hack since the rrd_cgi code used to just put
+ its arguments into a normal array starting at 0 ... since the rrd_*
+ commands expect and argc/argv array we used to just shift everything
+ by -1 ... this in turn exploded when a rrd_* function tried to print
+ argv[0] ... hence we are now doing everything in argv style but hand
+ over seemingly the old array ... but doing argv-1 will actually end
+ up in a 'good' place now. */
+
+*arguments = argv+1;
+*argument_count = argc-1;
+
if (Quote) {
return NULL;
}
@@ -1241,7 +1251,7 @@
if (end) {
/* got arguments, call function for 'tag' with arguments */
val = func(argc, (const char **) args);
-free(args);
+free(args-1);
} else {
/* unable to parse arguments, undo 0-termination by scanargs */
for (; argc > 0; argc--) {
at Today Sebastian Harl wrote:
> Hi Robert,
>
> On Thu, Mar 11, 2010 at 01:34:01PM +0100, Robert Luberda wrote:
> > iptotal.cgi (from the iptotal package) contains the following line
> >
> > which causes rrdcgi to crash with the following backtrace:
> >
> > (gdb) bt
> > #0 strlen () at ../sysdeps/i386/i486/strlen.S:40
> > #1 0xb73a681e in _IO_vfprintf_internal (s=0xbfa4086c,
> > format=0xb781edd0 "Usage: rrdtool %s [--daemon ] ",
> > ap=0xbfa40988 "\021\001\202?")
> > at vfprintf.c:1601
> > #2 0xb73c56b4 in _IO_vsnprintf (string=0xb78269c0 "Usage: rrdtool ",
> > maxlen=4096,
> > format=0xb781edd0 "Usage: rrdtool %s [--daemon ] ",
> > args=0xbfa40984 "\211")
> > at vsnprintf.c:120
> > #3 0xb78140c4 in rrd_set_error () from /usr/lib/librrd.so.4
> > #4 0xb7805be4 in rrd_last () from /usr/lib/librrd.so.4
> > #5 0x0804b211 in printtimelast ()
> > #6 0x0804aa83 in ?? ()
> > #7 0x0804c265 in ?? ()
> > #8 0xb737bb55 in __libc_start_main (main=0x804bf70, argc=2,
> > ubp_av=0xbfa40bb4, init=0x804c5c0,
> > fini=0x804c5b0, rtld_fini=0xb78629b0 <_dl_fini>,
> > stack_end=0xbfa40bac) at libc-start.c:222
>
> Thanks for reporting this!
>
> > Afer some investigation, I found that the problem is in the line 991
> > of rrd_cgi.c:
> >
> > last = rrd_last(argc + 1, (char **) args - 1);
> >
> > The first argument of rrd_last() should obviously be argc (which is 2),
> > not argc + 1. Also please note that second argument of the function
> > refers to address before the start of the array, which seems to
> > be a very bad programming style, and which in fact is a root cause of the
> > crash as rrd_last() tries to display argv[0] in an error message.
>
> Ouch! What an ugly hack ?
>
> > The attached patch fixes the problem.
>
> Thanks for tracing that back and providing a patch! Imho, the patch
> looks fine. With this E-mail, I'm forwarding the issue and the patch
> upstream, hoping for inclusion in the upstream SVN. I'll upload a fixed
> package to Debian soonish.
>
> Cheers,
> Sebastian
>
>
--
Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland
http://it.oetiker.ch t...@oetiker.ch ++41 62 775 9902 / sb: -9900
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#573638: rrdtool: rrdcgi crashes at printlasttime()
Hi Robert,
On Thu, Mar 11, 2010 at 01:34:01PM +0100, Robert Luberda wrote:
> iptotal.cgi (from the iptotal package) contains the following line
>
> which causes rrdcgi to crash with the following backtrace:
>
> (gdb) bt
> #0 strlen () at ../sysdeps/i386/i486/strlen.S:40
> #1 0xb73a681e in _IO_vfprintf_internal (s=0xbfa4086c,
> format=0xb781edd0 "Usage: rrdtool %s [--daemon ] ",
> ap=0xbfa40988 "\021\001\202ˇ")
> at vfprintf.c:1601
> #2 0xb73c56b4 in _IO_vsnprintf (string=0xb78269c0 "Usage: rrdtool ",
> maxlen=4096,
> format=0xb781edd0 "Usage: rrdtool %s [--daemon ] ",
> args=0xbfa40984 "\211")
> at vsnprintf.c:120
> #3 0xb78140c4 in rrd_set_error () from /usr/lib/librrd.so.4
> #4 0xb7805be4 in rrd_last () from /usr/lib/librrd.so.4
> #5 0x0804b211 in printtimelast ()
> #6 0x0804aa83 in ?? ()
> #7 0x0804c265 in ?? ()
> #8 0xb737bb55 in __libc_start_main (main=0x804bf70, argc=2,
> ubp_av=0xbfa40bb4, init=0x804c5c0,
> fini=0x804c5b0, rtld_fini=0xb78629b0 <_dl_fini>,
> stack_end=0xbfa40bac) at libc-start.c:222
Thanks for reporting this!
> Afer some investigation, I found that the problem is in the line 991
> of rrd_cgi.c:
>
> last = rrd_last(argc + 1, (char **) args - 1);
>
> The first argument of rrd_last() should obviously be argc (which is 2),
> not argc + 1. Also please note that second argument of the function
> refers to address before the start of the array, which seems to
> be a very bad programming style, and which in fact is a root cause of the
> crash as rrd_last() tries to display argv[0] in an error message.
Ouch! What an ugly hack …
> The attached patch fixes the problem.
Thanks for tracing that back and providing a patch! Imho, the patch
looks fine. With this E-mail, I'm forwarding the issue and the patch
upstream, hoping for inclusion in the upstream SVN. I'll upload a fixed
package to Debian soonish.
Cheers,
Sebastian
--
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/
Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin
--- rrdtool-1.4.2.old/src/rrd_cgi.c 2009-11-15 12:54:23.0 +0100
+++ rrdtool-1.4.2/src/rrd_cgi.c 2010-03-11 13:31:16.0 +0100
@@ -987,8 +987,9 @@
buf = malloc(255);
if (buf == NULL) {
return stralloc("[ERROR: allocating strftime buffer]");
-};
-last = rrd_last(argc + 1, (char **) args - 1);
+}
+const char *newargs[] = { "rrdcgi", args[0], NULL };
+last = rrd_last(2, (char **) (newargs));
if (rrd_test_error()) {
char *err =
malloc((strlen(rrd_get_error()) +
signature.asc
Description: Digital signature
Bug#573638: rrdtool: rrdcgi crashes at printlasttime()
Package: rrdtool
Version: 1.4.2-1+b1
Severity: serious
Justification: breaks iptotal
Hi,
iptotal.cgi (from the iptotal package) contains the following line
which causes rrdcgi to crash with the following backtrace:
(gdb) bt
#0 strlen () at ../sysdeps/i386/i486/strlen.S:40
#1 0xb73a681e in _IO_vfprintf_internal (s=0xbfa4086c,
format=0xb781edd0 "Usage: rrdtool %s [--daemon ] ",
ap=0xbfa40988 "\021\001\202ˇ")
at vfprintf.c:1601
#2 0xb73c56b4 in _IO_vsnprintf (string=0xb78269c0 "Usage: rrdtool ",
maxlen=4096,
format=0xb781edd0 "Usage: rrdtool %s [--daemon ] ",
args=0xbfa40984 "\211")
at vsnprintf.c:120
#3 0xb78140c4 in rrd_set_error () from /usr/lib/librrd.so.4
#4 0xb7805be4 in rrd_last () from /usr/lib/librrd.so.4
#5 0x0804b211 in printtimelast ()
#6 0x0804aa83 in ?? ()
#7 0x0804c265 in ?? ()
#8 0xb737bb55 in __libc_start_main (main=0x804bf70, argc=2,
ubp_av=0xbfa40bb4, init=0x804c5c0,
fini=0x804c5b0, rtld_fini=0xb78629b0 <_dl_fini>,
stack_end=0xbfa40bac) at libc-start.c:222
Afer some investigation, I found that the problem is in the line 991
of rrd_cgi.c:
last = rrd_last(argc + 1, (char **) args - 1);
The first argument of rrd_last() should obviously be argc (which is 2),
not argc + 1. Also please note that second argument of the function
refers to address before the start of the array, which seems to
be a very bad programming style, and which in fact is a root cause of the
crash as rrd_last() tries to display argv[0] in an error message.
The attached patch fixes the problem.
Regards,
robert
-- System Information:
Debian Release: squeeze/sid
APT prefers unstable
APT policy: (990, 'unstable'), (200, 'testing')
Architecture: i386 (i686)
Kernel: Linux 2.6.32
Locale: LANG=pl_PL, LC_CTYPE=pl_PL (charmap=ISO-8859-2)
Shell: /bin/sh linked to /bin/pdksh
Versions of packages rrdtool depends on:
ii libc62.10.2-6Embedded GNU C Library: Shared lib
ii libcairo21.8.10-2The Cairo 2D vector graphics libra
ii libdbi0 0.8.2-3 Database Independent Abstraction L
ii libglib2.0-0 2.22.4-1The GLib library of C routines
ii libpango1.0-01.26.2-1Layout and rendering of internatio
ii libpng12-0 1.2.43-1PNG library - runtime
ii librrd4 1.4.2-1+b1 time-series data storage and displ
ii libxml2 2.7.6.dfsg-2+b1 GNOME XML library
rrdtool recommends no packages.
Versions of packages rrdtool suggests:
pn librrds-perl (no description available)
-- no debconf information
-- debsums errors found:
debsums: changed file /usr/bin/rrdcgi (from rrdtool package)
--- rrdtool-1.4.2.old/src/rrd_cgi.c 2009-11-15 12:54:23.0 +0100
+++ rrdtool-1.4.2/src/rrd_cgi.c 2010-03-11 13:31:16.0 +0100
@@ -987,8 +987,9 @@
buf = malloc(255);
if (buf == NULL) {
return stralloc("[ERROR: allocating strftime buffer]");
-};
-last = rrd_last(argc + 1, (char **) args - 1);
+}
+const char *newargs[] = { "rrdcgi", args[0], NULL };
+last = rrd_last(2, (char **) (newargs));
if (rrd_test_error()) {
char *err =
malloc((strlen(rrd_get_error()) +
