Content-Type: multipart/mixed; boundary="------------BDWfYQggL7MlsT0gRHQGPqAN"
Message-ID: <f7ab52bb-e0ee-4acd-9bb1-ac28d22e39f5@gmail.com>
Date: Wed, 31 May 2023 09:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17888: Incorrect memory access in gist__int_ops for an input
 array with many elements
Content-Language: en-US
To: Richard Guo <guofenglinux@gmail.com>,
 Ankit Kumar Pandey <itsankitkp@gmail.com>,
 David Rowley <dgrowleyml@gmail.com>, Teodor Sigaev <teodor@sigaev.ru>,
 pgsql-bugs@lists.postgresql.org
References: <17888-f72930e6b5ce8c14@postgresql.org>
 <CAMbWs48s6Uhv9wngJVtv=7DGEyN9a-SzwBKsfz2hY7yS6z7ZQw@mail.gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: 
 <CAMbWs48s6Uhv9wngJVtv=7DGEyN9a-SzwBKsfz2hY7yS6z7ZQw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/f7ab52bb-e0ee-4acd-9bb1-ac28d22e39f5%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------BDWfYQggL7MlsT0gRHQGPqAN
Content-Type: multipart/alternative;
 boundary="------------tfWmyFHSbcV90a1K2ioR7mfF"

--------------tfWmyFHSbcV90a1K2ioR7mfF
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

07.04.2023 11:57, Richard Guo wrote:
>
> On Fri, Apr 7, 2023 at 4:21 AM PG Bug reporting form <noreply@postgresql.org> wrote:
>
>     NOTICE:  input array is too big (199 maximum allowed, 1001 current), use
>     gist__intbig_ops opclass instead
>
>
> In g_int_compress,
>
>  if (ARRNELEMS(r) >= 2 * num_ranges)
>      elog(NOTICE, "input array is too big (%d maximum allowed, %d current), use gist__intbig_ops opclass instead",
>           2 * num_ranges - 1, ARRNELEMS(r));
>
> Wondering why elog with NOTICE rather than Error here.

The NOTICE appeared there in commit 08ee64ebf5 (from year 2005) in an
attempt to get rid of flags (ISLEAFKEY(in)) [1] and probably expressed
an intention to play gently (don't break compatibility?).

The patch proposed at [2] looks correct to me. Thank you, Ankit!
Though I would add a simple case to the intarray regression test and also
add errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED) to other two places in
_int_gist.c for the sake of consistency.

Please look at the patches attached.

[1] https://www.postgresql.org/message-id/43786274.3020200%40sigaev.ru
[2] https://www.postgresql.org/message-id/796b65c3-57b7-bddf-b0d5-a8afafb8b627%40gmail.com
--------------tfWmyFHSbcV90a1K2ioR7mfF
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">07.04.2023 11:57, Richard Guo wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAMbWs48s6Uhv9wngJVtv=7DGEyN9a-SzwBKsfz2hY7yS6z7ZQw@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">
        <div dir="ltr"><br>
        </div>
        <div class="gmail_quote">
          <div dir="ltr" class="gmail_attr">On Fri, Apr 7, 2023 at
            4:21 AM PG Bug reporting form &lt;<a
              href="mailto:noreply@postgresql.org"
              moz-do-not-send="true" class="moz-txt-link-freetext">noreply@postgresql.org</a>&gt;
            wrote:</div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex">NOTICE:  input array is
            too big (199 maximum allowed, 1001 current), use<br>
            gist__intbig_ops opclass instead</blockquote>
          <div><br>
          </div>
          <div>In g_int_compress,<br>
            <br>
             if (ARRNELEMS(r) &gt;= 2 * num_ranges)<br>
                 elog(NOTICE, "input array is too big (%d maximum
            allowed, %d current), use gist__intbig_ops opclass instead",<br>
                      2 * num_ranges - 1, ARRNELEMS(r));<br>
            <br>
            Wondering why elog with NOTICE rather than Error here.<br>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    The NOTICE appeared there in commit 08ee64ebf5 (from year 2005) in
    an<br>
    attempt to get rid of flags (ISLEAFKEY(in)) [1] and probably
    expressed<br>
    an intention to play gently (don't break compatibility?).<br>
    <br>
    The patch proposed at [2] looks correct to me. Thank you, Ankit!<br>
    Though I would add a simple case to the intarray regression test and
    also<br>
    add errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED) to other two places in<br>
    _int_gist.c for the sake of consistency.<br>
    <br>
    Please look at the patches attached.<br>
    <br>
    [1]
    <a class="moz-txt-link-freetext" href="https://www.postgresql.org/message-id/43786274.3020200%40sigaev.ru">https://www.postgresql.org/message-id/43786274.3020200%40sigaev.ru</a><br>
    [2]
<a class="moz-txt-link-freetext" href="https://www.postgresql.org/message-id/796b65c3-57b7-bddf-b0d5-a8afafb8b627%40gmail.com">https://www.postgresql.org/message-id/796b65c3-57b7-bddf-b0d5-a8afafb8b627%40gmail.com</a><br>
  </body>
</html>

--------------tfWmyFHSbcV90a1K2ioR7mfF--

--------------BDWfYQggL7MlsT0gRHQGPqAN
Content-Type: text/x-patch; charset=UTF-8;
 name="v2-01-gist-int-ops-overflow.patch"
Content-Disposition: attachment; filename="v2-01-gist-int-ops-overflow.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL2NvbnRyaWIvaW50YXJyYXkvX2ludF9naXN0LmMgYi9jb250cmliL2lu
dGFycmF5L19pbnRfZ2lzdC5jCmluZGV4IDdhNDhjZTYyNGQuLjJjODJiODZkZDEgMTAwNjQ0
Ci0tLSBhL2NvbnRyaWIvaW50YXJyYXkvX2ludF9naXN0LmMKKysrIGIvY29udHJpYi9pbnRh
cnJheS9faW50X2dpc3QuYwpAQCAtMTgxLDggKzE4MSw5IEBAIGdfaW50X2NvbXByZXNzKFBH
X0ZVTkNUSU9OX0FSR1MpCiAJCVBSRVBBUkVBUlIocik7CiAKIAkJaWYgKEFSUk5FTEVNUyhy
KSA+PSAyICogbnVtX3JhbmdlcykKLQkJCWVsb2coTk9USUNFLCAiaW5wdXQgYXJyYXkgaXMg
dG9vIGJpZyAoJWQgbWF4aW11bSBhbGxvd2VkLCAlZCBjdXJyZW50KSwgdXNlIGdpc3RfX2lu
dGJpZ19vcHMgb3BjbGFzcyBpbnN0ZWFkIiwKLQkJCQkgMiAqIG51bV9yYW5nZXMgLSAxLCBB
UlJORUxFTVMocikpOworCQkJZXJlcG9ydChFUlJPUiwKKwkJCQkJKGVycm1zZygiaW5wdXQg
YXJyYXkgaXMgdG9vIGJpZyAoJWQgbWF4aW11bSBhbGxvd2VkLCAlZCBjdXJyZW50KSwgdXNl
IGdpc3RfX2ludGJpZ19vcHMgb3BjbGFzcyBpbnN0ZWFkIiwKKwkJCQkJCQkyICogbnVtX3Jh
bmdlcyAtIDEsIEFSUk5FTEVNUyhyKSkpKTsKIAogCQlyZXR2YWwgPSBwYWxsb2Moc2l6ZW9m
KEdJU1RFTlRSWSkpOwogCQlnaXN0ZW50cnlpbml0KCpyZXR2YWwsIFBvaW50ZXJHZXREYXR1
bShyKSwKZGlmZiAtLWdpdCBhL2NvbnRyaWIvaW50YXJyYXkvZXhwZWN0ZWQvX2ludC5vdXQg
Yi9jb250cmliL2ludGFycmF5L2V4cGVjdGVkL19pbnQub3V0CmluZGV4IDZhYmIxN2UyZTUu
LmNhZDM3ZjFhMjQgMTAwNjQ0Ci0tLSBhL2NvbnRyaWIvaW50YXJyYXkvZXhwZWN0ZWQvX2lu
dC5vdXQKKysrIGIvY29udHJpYi9pbnRhcnJheS9leHBlY3RlZC9faW50Lm91dApAQCAtNTY2
LDYgKzU2Niw4IEBAIFNFTEVDVCBjb3VudCgqKSBmcm9tIHRlc3RfX2ludCBXSEVSRSBhIEBA
ICchMjAgJiAhMjEnOwogICA2MzQzCiAoMSByb3cpCiAKK0lOU0VSVCBJTlRPIHRlc3RfX2lu
dCBTRUxFQ1QgYXJyYXkoU0VMRUNUIHggRlJPTSBnZW5lcmF0ZV9zZXJpZXMoMSwgMTAwMSkg
eCk7IC0tIHNob3VsZCBmYWlsCitFUlJPUjogIGlucHV0IGFycmF5IGlzIHRvbyBiaWcgKDE5
OSBtYXhpbXVtIGFsbG93ZWQsIDEwMDEgY3VycmVudCksIHVzZSBnaXN0X19pbnRiaWdfb3Bz
IG9wY2xhc3MgaW5zdGVhZAogRFJPUCBJTkRFWCB0ZXh0X2lkeDsKIENSRUFURSBJTkRFWCB0
ZXh0X2lkeCBvbiB0ZXN0X19pbnQgdXNpbmcgZ2lzdCAoYSBnaXN0X19pbnRfb3BzKG51bXJh
bmdlcyA9IDApKTsKIEVSUk9SOiAgdmFsdWUgMCBvdXQgb2YgYm91bmRzIGZvciBvcHRpb24g
Im51bXJhbmdlcyIKZGlmZiAtLWdpdCBhL2NvbnRyaWIvaW50YXJyYXkvc3FsL19pbnQuc3Fs
IGIvY29udHJpYi9pbnRhcnJheS9zcWwvX2ludC5zcWwKaW5kZXggZDlkOTg3YmRiMS4uYTk0
N2M0ZjhmZCAxMDA2NDQKLS0tIGEvY29udHJpYi9pbnRhcnJheS9zcWwvX2ludC5zcWwKKysr
IGIvY29udHJpYi9pbnRhcnJheS9zcWwvX2ludC5zcWwKQEAgLTEyNSw2ICsxMjUsOCBAQCBT
RUxFQ1QgY291bnQoKikgZnJvbSB0ZXN0X19pbnQgV0hFUkUgYSBAQCAnKDIwJjIzKXwoNTAm
NjgpJzsKIFNFTEVDVCBjb3VudCgqKSBmcm9tIHRlc3RfX2ludCBXSEVSRSBhIEBAICcyMCB8
ICEyMSc7CiBTRUxFQ1QgY291bnQoKikgZnJvbSB0ZXN0X19pbnQgV0hFUkUgYSBAQCAnITIw
ICYgITIxJzsKIAorSU5TRVJUIElOVE8gdGVzdF9faW50IFNFTEVDVCBhcnJheShTRUxFQ1Qg
eCBGUk9NIGdlbmVyYXRlX3NlcmllcygxLCAxMDAxKSB4KTsgLS0gc2hvdWxkIGZhaWwKKwog
RFJPUCBJTkRFWCB0ZXh0X2lkeDsKIENSRUFURSBJTkRFWCB0ZXh0X2lkeCBvbiB0ZXN0X19p
bnQgdXNpbmcgZ2lzdCAoYSBnaXN0X19pbnRfb3BzKG51bXJhbmdlcyA9IDApKTsKIENSRUFU
RSBJTkRFWCB0ZXh0X2lkeCBvbiB0ZXN0X19pbnQgdXNpbmcgZ2lzdCAoYSBnaXN0X19pbnRf
b3BzKG51bXJhbmdlcyA9IDI1MykpOwo=
--------------BDWfYQggL7MlsT0gRHQGPqAN
Content-Type: text/x-patch; charset=UTF-8;
 name="v2-02-gist-int-errcodes.patch"
Content-Disposition: attachment; filename="v2-02-gist-int-errcodes.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL2NvbnRyaWIvaW50YXJyYXkvX2ludF9naXN0LmMgYi9jb250cmliL2lu
dGFycmF5L19pbnRfZ2lzdC5jCmluZGV4IDJjODJiODZkZDEuLjk4MTQ1ZmUzNzAgMTAwNjQ0
Ci0tLSBhL2NvbnRyaWIvaW50YXJyYXkvX2ludF9naXN0LmMKKysrIGIvY29udHJpYi9pbnRh
cnJheS9faW50X2dpc3QuYwpAQCAtMTgyLDcgKzE4Miw4IEBAIGdfaW50X2NvbXByZXNzKFBH
X0ZVTkNUSU9OX0FSR1MpCiAKIAkJaWYgKEFSUk5FTEVNUyhyKSA+PSAyICogbnVtX3Jhbmdl
cykKIAkJCWVyZXBvcnQoRVJST1IsCi0JCQkJCShlcnJtc2coImlucHV0IGFycmF5IGlzIHRv
byBiaWcgKCVkIG1heGltdW0gYWxsb3dlZCwgJWQgY3VycmVudCksIHVzZSBnaXN0X19pbnRi
aWdfb3BzIG9wY2xhc3MgaW5zdGVhZCIsCisJCQkJCShlcnJjb2RlKEVSUkNPREVfUFJPR1JB
TV9MSU1JVF9FWENFRURFRCksCisJCQkJCSBlcnJtc2coImlucHV0IGFycmF5IGlzIHRvbyBi
aWcgKCVkIG1heGltdW0gYWxsb3dlZCwgJWQgY3VycmVudCksIHVzZSBnaXN0X19pbnRiaWdf
b3BzIG9wY2xhc3MgaW5zdGVhZCIsCiAJCQkJCQkJMiAqIG51bV9yYW5nZXMgLSAxLCBBUlJO
RUxFTVMocikpKSk7CiAKIAkJcmV0dmFsID0gcGFsbG9jKHNpemVvZihHSVNURU5UUlkpKTsK
QEAgLTI3MSw3ICsyNzIsOCBAQCBnX2ludF9jb21wcmVzcyhQR19GVU5DVElPTl9BUkdTKQog
CQlsZW5yID0gaW50ZXJuYWxfc2l6ZShkciwgbGVuKTsKIAkJaWYgKGxlbnIgPCAwIHx8IGxl
bnIgPiBNQVhOVU1FTFRTKQogCQkJZXJlcG9ydChFUlJPUiwKLQkJCQkJKGVycm1zZygiZGF0
YSBpcyB0b28gc3BhcnNlLCByZWNyZWF0ZSBpbmRleCB1c2luZyBnaXN0X19pbnRiaWdfb3Bz
IG9wY2xhc3MgaW5zdGVhZCIpKSk7CisJCQkJCShlcnJjb2RlKEVSUkNPREVfUFJPR1JBTV9M
SU1JVF9FWENFRURFRCksCisJCQkJCSBlcnJtc2coImRhdGEgaXMgdG9vIHNwYXJzZSwgcmVj
cmVhdGUgaW5kZXggdXNpbmcgZ2lzdF9faW50YmlnX29wcyBvcGNsYXNzIGluc3RlYWQiKSkp
OwogCiAJCXIgPSByZXNpemVfaW50QXJyYXlUeXBlKHIsIGxlbik7CiAJCXJldHZhbCA9IHBh
bGxvYyhzaXplb2YoR0lTVEVOVFJZKSk7CkBAIC0zMzMsNyArMzM1LDggQEAgZ19pbnRfZGVj
b21wcmVzcyhQR19GVU5DVElPTl9BUkdTKQogCWxlbnIgPSBpbnRlcm5hbF9zaXplKGRpbiwg
bGVuaW4pOwogCWlmIChsZW5yIDwgMCB8fCBsZW5yID4gTUFYTlVNRUxUUykKIAkJZXJlcG9y
dChFUlJPUiwKLQkJCQkoZXJybXNnKCJjb21wcmVzc2VkIGFycmF5IGlzIHRvbyBiaWcsIHJl
Y3JlYXRlIGluZGV4IHVzaW5nIGdpc3RfX2ludGJpZ19vcHMgb3BjbGFzcyBpbnN0ZWFkIikp
KTsKKwkJCQkoZXJyY29kZShFUlJDT0RFX1BST0dSQU1fTElNSVRfRVhDRUVERUQpLAorCQkJ
CSBlcnJtc2coImNvbXByZXNzZWQgYXJyYXkgaXMgdG9vIGJpZywgcmVjcmVhdGUgaW5kZXgg
dXNpbmcgZ2lzdF9faW50YmlnX29wcyBvcGNsYXNzIGluc3RlYWQiKSkpOwogCiAJciA9IG5l
d19pbnRBcnJheVR5cGUobGVucik7CiAJZHIgPSBBUlJQVFIocik7Cg==

--------------BDWfYQggL7MlsT0gRHQGPqAN--