Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()

Alexander Law <exclusion@gmail.com>

From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Date: 2023-09-04T14:00:00Z
Lists: pgsql-bugs

Attachments

Hi Mchael,

Thank you for committing the fix!

04.09.2023 09:35, Michael Paquier wrote:
> Regarding the changes in gtsvectorout(), the output produced is indeed
> confusing when ISALLTRUE is set.
>
> -        int            siglen = GETSIGLEN(key);
> -        int            cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);
> +        if (ISALLTRUE(key))
> +            sprintf(outbuf, "all true bits");
> +        else
> +        {
> +            int            siglen = GETSIGLEN(key);
> +            int            cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);
>   
> -        sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);
> +            sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);
> +        }
>
> In the false branch of ISALLTRUE(key), why isn't cnttrue always
> calculated with sizebitvec()?

Yes, that expression should be simpler there. Please look at the v2 attached.

> It's also not something I would
> backpatch.  That's confusing, for sure, but there is also the argument
> of keeping a consistent output in the stable branches.

I agree. Let's not backpatch it.

Best regards,
Alexander

Commits

  1. Improve description of keys in tsvector

  2. Fix out-of-bound read in gtsvector_picksplit()