Re: [NBTREE] Possible NULL pointer dereference (backend/access/nbtree/nbutils.c) - Mailing list pgsql-hackers
| From | Ranier Vilela |
|---|---|
| Subject | Re: [NBTREE] Possible NULL pointer dereference (backend/access/nbtree/nbutils.c) |
| Date | |
| Msg-id | CAEudQApXvh8QZZQ7Ok3QXHe-56hVu7R6n0GVNBTFqU5bSfaiGw@mail.gmail.com Whole thread Raw |
| In response to | Re: [NBTREE] Possible NULL pointer dereference (backend/access/nbtree/nbutils.c) (Peter Geoghegan <pg@bowt.ie>) |
| Responses |
Re: [NBTREE] Possible NULL pointer dereference (backend/access/nbtree/nbutils.c)
|
| List | pgsql-hackers |
Em qua., 2 de set. de 2020 às 20:17, Peter Geoghegan <pg@bowt.ie> escreveu:
On Wed, Sep 2, 2020 at 3:16 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
Perhaps you recall our discussion of a similar false positive in
nbtsplitloc.c; that had a similar feel to it. For example, if your
static analysis tool says that code that has been around for many
years is riddled with bugs, then the problem is almost certainly with
the tool (or with how the tool has been used).
I'm using about 4 static analysis tools.
Only one of them has a report of 67690 pages.
Only one of them has a report of 67690 pages.
Assuming an optimistic trend, only 1% could be real failures.
It would have 670 real flaws in the entire code.
In addition, if the code is compiled with -Wextra, hundreds of type mismatch alerts will be reported,
It would have 670 real flaws in the entire code.
In addition, if the code is compiled with -Wextra, hundreds of type mismatch alerts will be reported,
especially comparison between int (signed) and unsigned types.
If you try to leaks, you will find this:
== 42483 == ERROR: LeakSanitizer: detected memory leaks
Direct leak of 576 byte (s) in 1 object (s) allocated from:
# 0 0x7f3204fb7bc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
# 1 0x561c58d5b766 in save_ps_display_args /usr/src/postgres/src/backend/utils/misc/ps_status.c:178
# 2 0x561c584a90b3 in main /usr/src/postgres/src/backend/main/main.c:89
# 3 0x7f3204b7f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
Indirect leak of 14 byte (s) in 1 object (s) allocated from:
# 0 0x7f3204f403dd in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x963dd)
# 1 0x561c58d5b817 in save_ps_display_args /usr/src/postgres/src/backend/utils/misc/ps_status.c:186
# 2 0x561c584a90b3 in main /usr/src/postgres/src/backend/main/main.c:89
# 3 0x7f3204b7f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
For each reported failure, I have analyzed the code, without focusing on the "intention of the code",
If you try to leaks, you will find this:
== 42483 == ERROR: LeakSanitizer: detected memory leaks
Direct leak of 576 byte (s) in 1 object (s) allocated from:
# 0 0x7f3204fb7bc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
# 1 0x561c58d5b766 in save_ps_display_args /usr/src/postgres/src/backend/utils/misc/ps_status.c:178
# 2 0x561c584a90b3 in main /usr/src/postgres/src/backend/main/main.c:89
# 3 0x7f3204b7f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
Indirect leak of 14 byte (s) in 1 object (s) allocated from:
# 0 0x7f3204f403dd in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x963dd)
# 1 0x561c58d5b817 in save_ps_display_args /usr/src/postgres/src/backend/utils/misc/ps_status.c:186
# 2 0x561c584a90b3 in main /usr/src/postgres/src/backend/main/main.c:89
# 3 0x7f3204b7f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
For each reported failure, I have analyzed the code, without focusing on the "intention of the code",
but trying to abstract and concentrate on following the flow of the variables to make sure that the alert is invalid.
Maybe I didn't realize it, but I'm not "getting out of my head", these alerts.
Maybe I didn't realize it, but I'm not "getting out of my head", these alerts.
High performance C code very often relies on representational
invariants that may not be readily apparent to a compiler --
especially when dealing with on-disk state. Barring some obvious
problem like a hard crash, you have to do the work of assessing if the
code is correct based on the actual assumptions/context of the code. A
static analysis tool is of very little use if you're not going to put
the work in.
I'm sure that all these problems will be solved in the future. But how many others will be added.
I have realized that maybe, it may be hindering the development course, so I decided to stop.
I have realized that maybe, it may be hindering the development course, so I decided to stop.
I went to a lot of trouble to clearly document the code
in question here (the heap TID stuff in _bt_truncate()) -- did you
even bother to read those comments once?
Yes, I read.
Maybe in this specific case, only this, it would solve for the code and the static analysis tool, but it's just a thought.
pivotheaptid = (ItemPointer) ((char *) tidpivot + IndexTupleSize(tidpivot) -
sizeof(ItemPointerData));
sizeof(ItemPointerData));
Ranier Vilela
pgsql-hackers by date: