maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Fri, Apr 16, 2021 at 8:08 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Monty!
>
> On Apr 16, Michael Widenius wrote:
> > revision-id: fbe1dad15a1 (mariadb-10.5.2-555-gfbe1dad15a1)
> > parent(s): 57c19902326
> > author: Michael Widenius <michael.widenius@xxxxxxxxx>
> > committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> > timestamp: 2021-03-25 12:06:34 +0200
> > message:
> >
> > Ensure that we do not allocate strings bigger than 4G in String objects.
> >
> > This is needed as we are using uint32 for allocated and current length.
> >
> > diff --git a/sql/sql_string.cc b/sql/sql_string.cc
> > index 9c57bb22085..fedf5f4a48a 100644
> > --- a/sql/sql_string.cc
> > +++ b/sql/sql_string.cc
> > @@ -37,6 +37,8 @@ bool Binary_string::real_alloc(size_t length)
> >    DBUG_ASSERT(arg_length > length);
> >    if (arg_length <= length)
> >      return TRUE;                                 /* Overflow */
> > +  if (arg_length >= UINT_MAX32)
> > +    return FALSE;
> >    str_length=0;
> >    if (Alloced_length < arg_length)
> >    {
> > @@ -45,7 +47,6 @@ bool Binary_string::real_alloc(size_t length)
> >                                  arg_length,MYF(MY_WME | (thread_specific ?
> >                                                  MY_THREAD_SPECIFIC : 0)))))
> >        return TRUE;
> > -    DBUG_ASSERT(length < UINT_MAX32);
> >      Alloced_length=(uint32) arg_length;
> >      alloced=1;
>
> I liked assert more. It meant that we should never under any
> circumstances request more than 4G from Binary_string::real_alloc.

I did not just remove the assert for the fun of it.

The problem with the assert is there was ways with wrong test cases to
get sql_string to allocate
too big strings. Instead of having these fail with an assert, we now
get a signal 'allocation failed' for these
(for code that checks the string argument).

I must have got this problem while testing something.

It may however be a good idea to generate a malloc fail error for this case.

Regards.
Monty