You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 2011/10/26 17:55:27 UTC
Re: svn commit: r1188950 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/httpd.h include/util_varbuf.h modules/filters/mod_substitute.c server/util.c
On Oct 25, 2011, at 6:29 PM, sf@apache.org wrote:
>
> + if (len > maxlen && maxlen > 0)
> + return APR_ENOMEM;
> +
> if (!vb) {
> - dest = dst = apr_pcalloc(p, len + 1);
> + *result = dst = apr_pcalloc(p, len + 1);
if len == maxlen and == APR_SIZE_MAX then doesn't
the len+1 blow us up?
Re: svn commit: r1188950 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/httpd.h include/util_varbuf.h modules/filters/mod_substitute.c server/util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Oct 27, 2011, at 4:21 PM, Stefan Fritsch wrote:
>
> That's correct, it would crash if len == APR_SIZE_MAX. But my point
> was that it would also crash for len == APR_SIZE_MAX-1000, because by
> definition, the machine cannot have that much free mem and apr_pcalloc
> would call abort(). So in both cases, the amount of free memory is the
> limit. But aborting with out-of-mem is more correct than simply
> segfaulting, therefore I have fixed it.
Worrying about what the machine would do and what the code
would do are 2 different things. The code is obviously broken
should len == APR_SIZE_MAX, no matter whether or not
the machine allows it or not, and that is, imo, unacceptable.
Re: svn commit: r1188950 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/httpd.h include/util_varbuf.h modules/filters/mod_substitute.c server/util.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Thursday 27 October 2011, Jim Jagielski wrote:
> On Oct 26, 2011, at 3:46 PM, Stefan Fritsch wrote:
> > On Wednesday 26 October 2011, Jim Jagielski wrote:
> >> On Oct 25, 2011, at 6:29 PM, sf@apache.org wrote:
> >>> + if (len > maxlen && maxlen > 0)
> >>> + return APR_ENOMEM;
> >>> +
> >>>
> >>> if (!vb) {
> >>>
> >>> - dest = dst = apr_pcalloc(p, len + 1);
> >>> + *result = dst = apr_pcalloc(p, len + 1);
> >>
> >> if len == maxlen and == APR_SIZE_MAX then doesn't
> >> the len+1 blow us up?
> >
> > APR_SIZE_MAX means no limit, i.e. it will blow up when there is
> > no memory left, which will be well before APR_SIZE_MAX. I guess
> > I should have used 0, that would have been clearer.
>
> I thought that
>
> #define APR_SIZE_MAX (~((apr_size_t)0))
>
> returned the one's complement of 0, which would basically
> be the largest value possible for (apr_size_t)0, and that
> adding 1 to it would overflow… right? Or am I still jet-lagged
> (which is quite possible).
That's correct, it would crash if len == APR_SIZE_MAX. But my point
was that it would also crash for len == APR_SIZE_MAX-1000, because by
definition, the machine cannot have that much free mem and apr_pcalloc
would call abort(). So in both cases, the amount of free memory is the
limit. But aborting with out-of-mem is more correct than simply
segfaulting, therefore I have fixed it.
> The main question is whether or not maxlen accounts for
> the NUL or not; we seem inconsistent in that regard with
> this patch.
True. I think this is also fixed with r1189985.
Re: svn commit: r1188950 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/httpd.h include/util_varbuf.h modules/filters/mod_substitute.c server/util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Oct 26, 2011, at 3:46 PM, Stefan Fritsch wrote:
> On Wednesday 26 October 2011, Jim Jagielski wrote:
>> On Oct 25, 2011, at 6:29 PM, sf@apache.org wrote:
>>> + if (len > maxlen && maxlen > 0)
>>> + return APR_ENOMEM;
>>> +
>>>
>>> if (!vb) {
>>>
>>> - dest = dst = apr_pcalloc(p, len + 1);
>>> + *result = dst = apr_pcalloc(p, len + 1);
>>
>> if len == maxlen and == APR_SIZE_MAX then doesn't
>> the len+1 blow us up?
>
> APR_SIZE_MAX means no limit, i.e. it will blow up when there is no
> memory left, which will be well before APR_SIZE_MAX. I guess I should
> have used 0, that would have been clearer.
>
I thought that
#define APR_SIZE_MAX (~((apr_size_t)0))
returned the one's complement of 0, which would basically
be the largest value possible for (apr_size_t)0, and that
adding 1 to it would overflow… right? Or am I still jet-lagged
(which is quite possible).
The main question is whether or not maxlen accounts for
the NUL or not; we seem inconsistent in that regard with
this patch.
Re: svn commit: r1188950 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/httpd.h include/util_varbuf.h modules/filters/mod_substitute.c server/util.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 26 October 2011, Jim Jagielski wrote:
> On Oct 25, 2011, at 6:29 PM, sf@apache.org wrote:
> > + if (len > maxlen && maxlen > 0)
> > + return APR_ENOMEM;
> > +
> >
> > if (!vb) {
> >
> > - dest = dst = apr_pcalloc(p, len + 1);
> > + *result = dst = apr_pcalloc(p, len + 1);
>
> if len == maxlen and == APR_SIZE_MAX then doesn't
> the len+1 blow us up?
APR_SIZE_MAX means no limit, i.e. it will blow up when there is no
memory left, which will be well before APR_SIZE_MAX. I guess I should
have used 0, that would have been clearer.