On Thu, Jan 15, 2009 at 10:56, Brad Nicholes <bnicho...@novell.com> wrote: >>>> On 1/14/2009 at 3:10 PM, in message > <dbdc3b250901141410l1b0acc6ar2cef894903829...@mail.gmail.com>, "Jesse Becker" > <haw...@gmail.com> wrote: >> On Wed, Jan 14, 2009 at 12:56, Brad Nicholes <bnicho...@novell.com> wrote: >>> Sounds good, can we condense the description below into something that we >> can put into the 3.1 STATUS file so that we can start the process of getting >> this backported into 3.1? >> >> Done. Committed as r1947. >> >> I suggest that once this is accepted, we release 3.1.2 ASAP. >> >> Also, a big thanks to Spike for finding and tracking this problem down. > > > Spike, > Correct me if I am wrong, but I think we still have a one-off problem with > the patch. > > len = q-p; > /* +1 not needed as q-p is already accounting for that */ > element = malloc(len); > strncpy(element, p, len); > element[len] = '\0'; > > > In the above code suppose "len" is 10 so we malloc() 10 bytes which means > that the array indexing for element is 0-9. Then we strncpy() 10 bytes into > "element" which is OK. However setting element[len] = "\0", IOW element[10] > = "\0" is indexing into the element array 1 byte beyond the actual length of > element. So either the code needs to be element = malloc(len+1) or > element[len-1] = "\0" correct?
That sounds right. > I would suggest that maybe rather than allowing the code to malloc() any size > buffer to match the incoming element length that instead or in addition we > limit the element length as well. Allowing the code to malloc() an unlimited > buffer can also lead to security issues. Since the original code defined a > 256 byte buffer, 256 seems to be a good limit for elements of the path. It > might be a better idea to simply add the limit checking rather than doing a > lot of malloc and free calls. If an element is larger than the limit, then > produce a mal-formed error message and bail out. Well, part of the original problem was that a static-length buffer was used, and there wasn't proper bounds checking in the first place. ;-) > > Also I don't think that the current patch will work without the multi-path > patch that was also submitted, without being reworked. The current patch > removes the recursive call to process_path() which means that without the for > loop that was provided in the multi-path patch, only one element of the path > will be processed and the rest will be ignored. The current patch probably > needs to be reworked and separated from the multi-path patch. For reference, most filesystem don't allow individual file or directory names to be longer than 255 characters. However, the full path can be much longer... -- Jesse Becker GPG Fingerprint -- BD00 7AA4 4483 AFCC 82D0 2720 0083 0931 9A2B 06A2 ------------------------------------------------------------------------------ This SF.net email is sponsored by: SourcForge Community SourceForge wants to tell your story. http://p.sf.net/sfu/sf-spreadtheword _______________________________________________ Ganglia-developers mailing list Ganglia-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ganglia-developers
