Re: A bug when use get_bit() function for a long bytea string

From: "movead(dot)li(at)highgo(dot)ca" <movead(dot)li(at)highgo(dot)ca>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: ashutosh(dot)bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: A bug when use get_bit() function for a long bytea string
Date: 2020-04-02 06:26:08
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

>I don't think this has really solved the overflow hazards. For example,
>in binary_encode we've got

>resultlen = enc->encode_len(VARDATA_ANY(data), datalen);
>result = palloc(VARHDRSZ + resultlen);

>and all you've done about that is changed resultlen from int to int64.
>On a 64-bit machine, sure, palloc will be able to detect if the
>result exceeds what can be allocated --- but on a 32-bit machine
>it'd be possible for the size_t argument to overflow altogether.
>(Or if you want to argue that it can't overflow because no encoder
>expands the data more than 4X, then we don't need to be making this
>change at all.)

>I don't think there's really any way to do that safely without an
>explicit check before we call palloc.
I am sorry I do not very understand these words, and especially
what's the mean by 'size_t'.
Here I change resultlen from int to int64, is because we can get a right
error report value other than '-1' or another strange number.

>Why did you change the outputs from unsigned to signed? Why didn't
>you change the dlen inputs? I grant that we know that the input
>can't exceed 1GB in Postgres' usage, but these signatures are just
>randomly inconsistent, and you didn't even add a comment explaining
>why. Personally I think I'd make them like
>uint64 (*encode_len) (const char *data, size_t dlen);
>which makes it explicit that the dlen argument describes the length
>of a chunk of allocated memory, while the result might exceed that.
I think it makes sence and followed.

>Lastly, there is a component of this that can be back-patched and
>a component that can't --- we do not change system catalog contents
>in released branches. Cramming both parts into the same patch
>is forcing the committer to pull them apart, which is kind of
Sorry about that, attached is the changed patch for PG13, and the one
for older branches will send sooner.

Highgo Software (Canada/China/Pakistan)
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Attachment Content-Type Size
long_bytea_string_bug_fix_ver6.patch application/octet-stream 10.9 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2020-04-02 06:29:31 Re: User Interface for WAL usage data
Previous Message Michael Paquier 2020-04-02 06:21:50 Re: potential stuck lock in SaveSlotToPath()