postgresql-interfaces/psqlodbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
8+ messages / 3 participants
[nested] [flat]
* [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2025-11-21 15:05 "markmaker (@markmaker)" <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: markmaker (@markmaker) @ 2025-11-21 15:05 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
# Problem
We get spurious heap corruption.
# Diagnosis
After a steep learning curve in such matters, we finally found a way to pinpoint it using `valgrind` (the key was using the redzone, so it would not immediately corrupt everything):
```bash
valgrind --tool=memcheck --leak-check=summary --redzone-size=8 <executable>
```
It outputs these sections (snippets).
**EDIT:** installed Debug symbol, now we can even better pinpoint it:
```
==2666205== Invalid write of size 1
==2666205== at 0x154AEDA7: UnknownInlinedFun (convert.c:6414)
==2666205== by 0x154AEDA7: convert_from_pgbinary.isra.0 (convert.c:6235)
==2666205== by 0x154ABE8E: UnknownInlinedFun (convert.c:1115)
==2666205== by 0x154ABE8E: UnknownInlinedFun (convert.c:1189)
==2666205== by 0x154ABE8E: copy_and_convert_field.isra.0 (convert.c:1726)
==2666205== by 0x154754FA: UnknownInlinedFun (convert.c:810)
==2666205== by 0x154754FA: SC_fetch (statement.c:1839)
==2666205== by 0x15469B14: PGAPI_ExtendedFetch (results.c:1902)
==2666205== by 0x15485922: SQLFetchScroll (odbcapi30.c:245)
==2666205== by 0x556BF4D: SQLFetchScroll (in /usr/lib/x86_64-linux-gnu/libodbc.so.2.0.0)
...
==2666205== Address 0x180015b8 is 0 bytes after a block of size 3,576 alloc'd
==2666205== at 0x4851E10: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2666205== by 0x154ACDDB: UnknownInlinedFun (convert.c:1071)
==2666205== by 0x154ACDDB: UnknownInlinedFun (convert.c:1189)
==2666205== by 0x154ACDDB: copy_and_convert_field.isra.0 (convert.c:1726)
==2666205== by 0x154754FA: UnknownInlinedFun (convert.c:810)
==2666205== by 0x154754FA: SC_fetch (statement.c:1839)
==2666205== by 0x15469B14: PGAPI_ExtendedFetch (results.c:1902)
==2666205== by 0x15485922: SQLFetchScroll (odbcapi30.c:245)
==2666205== by 0x556BF4D: SQLFetchScroll (in /usr/lib/x86_64-linux-gnu/libodbc.so.2.0.0)
...
```
Due to the sequence of ODBC calls, and the exact size of the buffer (3576), we knew it was the BYTEA column in our row:
```psql
v_base_pg=# select length(prstntpacked) from v.gusrstns where usrstgusr_userv = 46035;
length
--------
3576
(1 row)
```
Because we use bound buffers that are smaller than that, and we haven't yet come to the point of calling `SQLGetData()`, we know this must be an internal buffer that already knows the size of the BYTEA data. Therefore it couldn't be one of _our_ buffers being too small.
It follows that this looks like the extra NULL terminator byte that so plagues the C world. And sure, the code says:
https://github.com/postgresql-interfaces/psqlodbc/blob/6e99e750163d8cf572d71ea958acc7ef3b561ce5/conv...
# Proposed fix
It seems that immediately after this, the `len_for_wcs_term` is not included in the comparison to assess the need of `realloc()`. If by chance the buffer was already exactly `needbuflen` bytes large, it would not be realloc'd.
https://github.com/postgresql-interfaces/psqlodbc/blob/6e99e750163d8cf572d71ea958acc7ef3b561ce5/conv...
Therefore this PR intends to fix that.
~~We were unable to test this due to time constraints (this is currently impeding our rollout, we have our hands full 🤕!)~~
Help in setting up a psqlodbc compilation and local installation and deployment is welcome.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2025-11-21 18:29 ` "markmaker (@markmaker)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: markmaker (@markmaker) @ 2025-11-21 18:29 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
I'm trying to compile the driver myself. Any feedback very welcome. So far I managed to scratch this together:
```sh
# this is on a Kubuntu 25.04 with installation according
# to https://wiki.postgresql.org/wiki/Apt (modernized).
sudo apt-get install unixodbc odbcinst unixodbc-dev libpq-dev
# my PR version
git clone https://github.com/markmaker/psqlodbc.git
./bootstrap
# if somebody knows a better/automatic/more correct way to configure these paths, please speak up ðŸ¤
./configure \
--with-unixodbc=/usr/include/x86_64-linux-gnu/unixODBC/ \
--with-libpq=/usr/include/postgresql \
--enable-pthreads \
CPPFLAGS="-DSQLCOLATTRIBUTE_SQLLEN"
make
sudo make install
```
Then I copied the regular `[PostgreSQL Unicode]` section in the `/etc/odbcinst.ini` to have the `/usr/local/lib/` path prefix where make install reported it installed:
```ini
...
[PostgreSQL Unicode Fixed]
Description=PostgreSQL ODBC driver (Unicode version)
Driver=/usr/local/lib/psqlodbcw.so
Setup=/usr/local/lib/libodbcpsqlS.so
Debug=0
CommLog=1
UsageCount=2
```
Then I changed my driver connect to point to the new one...
`"DRIVER={PostgreSQL Unicode Fixed};SERVER=..."`
... tested again and it seems the problem is gone! 🎈🎉
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2025-11-24 15:10 ` "markmaker (@markmaker)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: markmaker (@markmaker) @ 2025-11-24 15:10 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
@davecramer, thank you for the fast merge. 🚀
How do we know when this went [into the apt repo](https://apt.postgresql.org/pub/repos/apt/)? How long do you suppose this takes?
Sorry, I'm not very familiar with how these repos work :sweat_smile:
_Mark
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2025-11-25 13:55 ` "davecramer (@davecramer)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: davecramer (@davecramer) @ 2025-11-25 13:55 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
probably not yet, let me send an email to the guys who do that stuff
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2026-04-01 08:46 ` "jarvis24young (@jarvis24young)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: jarvis24young (@jarvis24young) @ 2026-04-01 08:46 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
i tried too many times to repeat this bug but i couldnt do it, can someone pls give me a exact test case with main function that i can make this bug repeat and see it pls?
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2026-04-01 19:52 ` "markmaker (@markmaker)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: markmaker (@markmaker) @ 2026-04-01 19:52 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
@jarvis24young,
you need to provoke a case, where the previously used `pgdc->ttlbuf` is exactly one byte too small (for the null terminator byte), or in other words, the largest previously buffered blob needs to have a length that is one less.
I guess it would suffice to create a table with blobs that grow in size row-by-row by one byte. Then using a fresh ODBC connection, fetch those rows ordered by length, ascending.
For `valgrind` it might trigger the error for every length, but for a real memory corruption, you would also need to hit a certain length that exactly fills up the internal `malloc()` block allocation (which is typically rounded up to some granularity), so that the buffer overrun null terminator byte corrupts the neighboring block.
I don't hand-write SQL very often so I used AI to help generate this script. It seems to work, creating such rows. But I haven't tested reproducing the bug with the old faulty ODBC driver.
```sql
-- Drop table if it already exists
DROP TABLE IF EXISTS bytea_test;
-- Create table with sequential primary key and bytea column
CREATE TABLE bytea_test (
id BIGSERIAL PRIMARY KEY,
data BYTEA NOT NULL
);
-- Insert rows with increasing bytea length
INSERT INTO bytea_test (data)
SELECT repeat('D', n)::bytea
FROM generate_series(1000, 1100) AS n;
-- Verify lengths
SELECT id, octet_length(data) AS byte_length
FROM bytea_test
ORDER BY id;
```
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2026-04-03 02:02 ` "jarvis24young (@jarvis24young)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: jarvis24young (@jarvis24young) @ 2026-04-03 02:02 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
@markmaker I used the old faulty ODBC tried to write the test code, but i cant get the error. could i pls know which exactly the old ODBC driver will corrupts exactly? is it here?
<img width="479" height="88" alt="image" src="https://github.com/user-attachments/assets/84ad51a1-c4f5-4b4f-941e-d919dd9fe316"; />
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
@ 2026-04-03 10:00 ` "markmaker (@markmaker)" <[email protected]>
6 siblings, 0 replies; 8+ messages in thread
From: markmaker (@markmaker) @ 2026-04-03 10:00 UTC (permalink / raw)
To: postgresql-interfaces/psqlodbc <[email protected]>
Guess it must have been release 17.00.0006.
Are you using valgrind?
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2026-04-03 10:00 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-11-21 15:05 [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed "markmaker (@markmaker)" <[email protected]>
2025-11-21 18:29 ` "markmaker (@markmaker)" <[email protected]>
2025-11-24 15:10 ` "markmaker (@markmaker)" <[email protected]>
2025-11-25 13:55 ` "davecramer (@davecramer)" <[email protected]>
2026-04-01 08:46 ` "jarvis24young (@jarvis24young)" <[email protected]>
2026-04-01 19:52 ` "markmaker (@markmaker)" <[email protected]>
2026-04-03 02:02 ` "jarvis24young (@jarvis24young)" <[email protected]>
2026-04-03 10:00 ` "markmaker (@markmaker)" <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox