[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: minimum buf_size doesn't fit me anymore
From: |
Chip Seraphine |
Subject: |
Re: minimum buf_size doesn't fit me anymore |
Date: |
Wed, 14 Apr 2004 10:57:41 -0500 |
User-agent: |
KMail/1.5.4 |
OK, finally finished some tests.
Using the current snapshot (1600 today your time), I confirmed that the
second BufferOverflow check in varstring was the one being triggered and
replaced the check at vartstring.c:627 with:
/*if (BufferOverflow(buffer,ALLCLASSBUFFER))
*/
if (strlen(ALLCLASSBUFFER) >= (BUFFER_FACTOR *
bufsize))
(BUFFER_FACTOR is a #define to 16, which is more than adequate for the string
being passed.)
That moved the error to the BufferOverflow check on line 354, which is the
tough one to remove since everybody uses it. Commenting that check out
causes the problem to go away and the string to be passed perfectly, but that
is obviously not a good permanent fix as it removes an apparently important
check. Can we either smarten up the check on line 354 or somehow guarantee
that the buffer size was (intelligently) checked before we get there?
On Wednesday 14 April 2004 08:52, Mark.Burgess@iu.hio.no wrote:
>
> It seems that ExpandVarstring is not used in connection with ALLCLASSBUFFER,
> but rather at line 1842 of cfagent.c there is a limit of bufsize.
> Can that be the answer?
>
> M
>
> On 14 Apr, Chip Seraphine wrote:
> >
> > Upgraded to 2.1.5, but the enlarged ALLCLASSBUFFER is still broken because
the
> > BufferOverflow() function doesn't know that ALLCLASSBUFFER is allowed to
be
> > bigger than a regular buffer, so it fails when called from
ExpandVarstring.
> >
> > ExpandVarstring tests the buffer twice with BufferOverflow, first at line
354
> > then again at 627. The second one can be replaced with a
> > "strlen(ALLCALSSBUFFER) > ALLCLASSBUFFERSIZE" or similar (assuming you
> > #define ALLCLASSBUFFERSIZE to the appropriate value), but the first one is
> > hit by everything that uses ExpandVarstring, and hence is hard to dodge.
> >
> > I'm guessing the way to fix this is to either (a) expand the classlist via
> > something other than ExpandVarstring or (b) only test for buffer overflow
> > after we know the sort of buffer we are dealing with instead of at the top
of
> > the function. The first option sounds tedious and the second one sounds
> > unsafe, at least from my point of view (as someone not overly familiar
with
> > the code).
> >
> > I guess the workaround is to jackup bufsize, but that increases a lot of
> > buffers that don't really need increasing. *sigh*
> >
> > On Saturday 10 April 2004 11:32, Mark.Burgess@iu.hio.no wrote:
> >>
> >> Better to add this only to ALLCLASSBUFFER[4*bufsize]. Will be in 2.1.5
> >>
> >> M
> >>
> >>
> >> On 8 Apr, Chip Seraphine wrote:
> >> > A-whups, got the bufsizes mixed up. Should have been bufsize.
> >> >
> >> > I'm using the RPM stuff to manage my entire packagelist, which at 1000+
> > RPMs
> >> > results in a lot of classes being set and "Buffer overflow constructing
> >> > string. Increase bufsize macro." messages when passing it to a
> > shellcommand.
> >> >
> >> > On Thursday 08 April 2004 16:49, Mark.Burgess@iu.hio.no wrote:
> >> >>
> >> >> Do you mean buf_size (file block size) or bufsize (internal array
size)?
> >> >> What does this have to do with rpms?
> >> >>
> >> >> M
> >> >>
> >> >> On 8 Apr, Chip Seraphine wrote:
> >> >> >
> >> >> > I've been fooling around in v 2.1.3 with the new RPM stuff and so
far
> > it
> >> >> > seems quite nice. The one issue is that I had to significantly
jack
> > up
> >> >> > my minimum buf_size (client.c line 543) from 2048 to 8192; this
seems
> >> >> > somewhat unavoidable if you are using the packages: toys, unless you
> > are
> >> >> > only using it to add the occasional odd RPM instead of the whole
lot.
> >> >> >
> >> >> > Cfengine does a lot more than it used to, and humongeous class
strings
> > are
> >> >> > probably becoming more common. Perhaps we could up it some to
reflect
> >> > this?
> >> >> >
> >> >> > Here's the tiny change I made in 2.1.3 to allow me to do
> >> >> > CFLAGS="-DMIN_CLIENT_BUF_SIZE=8192" on the ./configure line. If
> > others
> >> >> > are having this issue it may prove convenient.
> >> >> >
> >> >> >
> >> >> > --- client.c 2004-04-08 11:15:53.000000000 -0500
> >> >> > +++ client.c.orig 2004-04-08 11:12:40.000000000 -0500
> >> >> > @@ -540,7 +540,7 @@
> >> >> >
> >> >> > if (buf_size < 2048)
> >> >> > {
> >> >> > - buf_size = MIN_CLIENT_BUF_SIZE;
> >> >> > + buf_size = 2048;
> >> >> > }
> >> >> >
> >> >> > if (ip->encrypt == 'y')
> >> >> >
> >> >> >
> >> >> > --- cf.defs.h 2004-04-08 11:15:20.000000000 -0500
> >> >> > +++ cf.defs.h.orig 2004-04-08 11:12:46.000000000 -0500
> >> >> > @@ -345,10 +345,6 @@
> >> >> >
> >> >> > #define CFLOGSIZE 1048576 /* Size of lock-log
before
> >> >> > rotation */
> >> >> >
> >> >> > -#ifndef MIN_CLIENT_BUF_SIZE
> >> >> > -#define MIN_CLIENT_BUF_SIZE 2048
> >> >> > -#endif
> >> >> > -
> >> >> > /* Output control defines */
> >> >> >
> >> >> > #define Verbose if (VERBOSE || DEBUG || D2) printf
> >> >> >
> >> >> >
> >> >> > <FEATURE_REQUEST type="cleverly disguised as speculation">
> >> >> > This might not be as necessary if there were some more granular
means
> > of
> >> >> > assembling ${allclasses}-like strings; like an
> >> >> > AddClassnameToStringButOnlyIfItIsActuallyDefined(class1 class2
classN)
> >> > sort
> >> >> > of thing that jammed its argument classes into ${someclasses} if
they
> > were
> >> >> > true, so that I could then call my scripts/modules with
${someclasses}
> >> > (or
> >> >> > a copy of same) as an argument instead of ${allclasses}.
> >> >> >
> >> >> > Or do most people not routinely use cfengine classes to turn on/off
> > large
> >> >> > numbers of options to scripts and modules?
> >> >> > </FEATURE_REQUEST>
> >> >> >
> >> >> >
> >> >> >
> >> >> > _______________________________________________
> >> >> > Help-cfengine mailing list
> >> >> > Help-cfengine@gnu.org
> >> >> > http://mail.gnu.org/mailman/listinfo/help-cfengine
> >> >>
> >> >>
> >> >>
> >> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> Work: +47 22453272 Email: Mark.Burgess@iu.hio.no
> >> >> Fax : +47 22453205 WWW : http://www.iu.hio.no/~mark
> >> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >>
> >>
> >>
> >>
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> Work: +47 22453272 Email: Mark.Burgess@iu.hio.no
> >> Fax : +47 22453205 WWW : http://www.iu.hio.no/~mark
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
>
>
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Work: +47 22453272 Email: Mark.Burgess@iu.hio.no
> Fax : +47 22453205 WWW : http://www.iu.hio.no/~mark
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>