----- Original Message ----- From: "Svante Signell" <[email protected]> To: "Steven Hartland" <[email protected]> Cc: "rrdtool dev list" <[email protected]> Sent: Thursday, May 08, 2014 6:18 AM Subject: Re: [rrd-developers] RFC: [PATCH] Portability by avoiding PATH_MAX
> On Thu, 2014-05-08 at 02:05 +0100, Steven Hartland wrote: >> ----- Original Message ----- >> From: "Svante Signell" <[email protected]> >> To: "rrdtool dev list" <[email protected]> >> Sent: Tuesday, April 29, 2014 1:35 PM >> Subject: [rrd-developers] RFC: [PATCH] Portability by avoiding PATH_MAX >> >> >> > Hello, >> > >> > Here are updated patches for rrdtool/rrd_client.c and >> > rrdtool/rrd_daemon.c. We had some discussions in August last year. I >> > created a branch and the diffs are against latest git. I've run the >> > code, especially rrd_daemon with valgrind under Linux, but need some >> > help to check that also rrd_client works OK (maybe rrd_daemon too). Can >> > you help me with test cases to run the execute the code modified paths. >> > I know one application using rrdtool, lm-sensors (build-dependency on >> > librrd2-dev), but am not sure my computers have the sensors to be a good >> > test case. >> > >> > Note that the modified functions get_path() and get_abs_path() are >> > static, so they don't change the API. >> >> This looks like it would cause a lot of unnessary malloc free surely >> the correct fix for this is to fix the OS config to define PATH_MAX. >> >> Quick look at the patches also show its quite broken:- >> >> + tmp = malloc(len); >> + snprintf(tmp, len, "%s/%s", config_base_dir, *filename); >> *filename = tmp; >> + free(tmp); >> >> So you just malloced some memory, assigned the pointer to it then freed >> the memory, so your now going to get a use after free and BOOM! > > This is of course wrong, thanks! You see the reason for having tests > together with valgrind to find out these problems. Did you find more? Obviously this means that any calls to get_abs_path need reworking in the patch. I've not had time to go through it in detail but looks like the journal_dir / realpath changes also have issues. Also there's some formatting issues with tabs instead of spacing. Regards Steve Really needs a proper review as there's lots of potential for issues with this. _______________________________________________ rrd-developers mailing list [email protected] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
