Re: A bug when use get_bit() function for a long bytea string
movead.li@highgo.ca <movead.li@highgo.ca>
From: "movead.li@highgo.ca" <movead.li@highgo.ca>
To: "Tom Lane" <tgl@sss.pgh.pa.us>
Cc: ashutosh.bapat <ashutosh.bapat@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-04-02T06:26:08Z
Lists: pgsql-hackers
Attachments
- long_bytea_string_bug_fix_ver6.patch (application/octet-stream) patch
>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 >unfriendly. Sorry about that, attached is the changed patch for PG13, and the one for older branches will send sooner. Regards, Highgo Software (Canada/China/Pakistan) URL : www.highgo.ca EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
Commits
-
Adjust bytea get_bit/set_bit to cope with bytea strings > 256MB.
- e40c4d4914fd 9.6.18 landed
- dbb121038c74 9.5.22 landed
- 889786e0e8c1 10.13 landed
- 6e6b74a206c3 12.3 landed
- 5d79fc60c57d 11.8 landed
-
Adjust bytea get_bit/set_bit to use int8 not int4 for bit numbering.
- 26a944cf29ba 13.0 landed