pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Tomas Vondra <tomas@vondra.me>
Cc: Noah Misch <noah@leadboat.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Date: Sun, 16 Feb 2025 13:12:18 -0500
Message-ID: <626104.1739729538@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+hUKGL8bAsQNj-bB_qxFFzoQW+RMZ3P3zK2SVa6b08G55BdRg@mail.gmail.com>
References: <CAAKRu_ZuYsf4MNGzm7_KvG0SKsu-EtYRhFHy2x3q0heX8KLMTg@mail.gmail.com>
	<CAAKRu_bQgf4e9OveCQX++zTEJLKbZbqLeapA7SNkNuMjxcCO1w@mail.gmail.com>
	<20240306234733.nd4a636colxkgq2e@liskov>
	<CAAKRu_aiaf+CufVA5Rrksz4Lw2fvZ3M-OgdBZ8WpzvYoQY96MQ@mail.gmail.com>
	<CA+hUKGKHb3i8Wy72VCKZGA2B5djoj7tAzYAzbeS=Gwr_SdhgRw@mail.gmail.com>
	<CAAKRu_byDppRvNJ+p5kq-SmXZDQuL6L8N6D06pHbO6WnBnanWQ@mail.gmail.com>
	<CA+hUKGLY4Q4ZY4f1rvnFtv6+PkjNf8MejdPkcju3Qii9DYqqcQ@mail.gmail.com>
	<CAAKRu_bbkmwAzSBgnezancgJeXrQZXy4G4kBTd+5=cr86H5yew@mail.gmail.com>
	<20240707144944.58.nmisch@google.com>
	<CA+hUKGK3sB-T+Ao8EBCOu5M1MsCMPnPpNyXuQGkQYHU4WC35eA@mail.gmail.com>
	<20240716015226.60.nmisch@google.com>
	<CA+hUKGKN3oy0bN_3yv8hd78a4+M1tJC9z7mD8+f+yA+GeoFUwQ@mail.gmail.com>
	<eb657bfe-ba96-4aef-82f9-b952724de863@vondra.me>
	<CAAKRu_aLwANZpxHc0tC-6OT0OQT4TftDGkKAO5yigMUOv_Tcsw@mail.gmail.com>
	<85696b8e-f1bf-459e-ba97-5608c644c185@vondra.me>
	<CAAKRu_ZoOSZHexy=7YZ9W0CfeSHc1tGx7VCperZHWBxxz0gxzw@mail.gmail.com>
	<CA! AKRu_bmx33jTqATP5GKNFYwAg02a9dDtk4U_ciEjgBHZSVkOQ@mail.gmail.com>
	<CAD21AoDu_a0bECcOuhev08J_VUUR_3wL2fBM1D7_49Vy7A1XqA@mail.gmail.com>
	<CAAKRu_YP=-Mu9rG+q8CBGFMsurMMv-8T-=is80RMr0kR+VbAfA@mail.gmail.com>
	<CAD21AoDdx87ur-YA6qz3mKO0C7-wnQJW+45H6eEkDSP_rNB8hA@mail.gmail.com>
	<CAAKRu_Zbay2UeANcg-X455Q10nZJ_XH31Qph2wM4qDrZVa28JQ@mail.gmail.com>
	<CAAKRu_bR4ghLfsgDNnHm0gK2SOcG6qicmeyaPG2y0Ppjz1G6cw@mail.gmail.com>
	<CAAKRu_ac9DMqN2sSxLLK-D6m4V=2=uKufQUrmv82mQjch4F67A@mail.gmail.com>
	<CA+hUKG+g6aXpi2FEHqeLOzE+xYw=OV+-N5jhOEnnV+F0USM9xA@mail.gmail.com>
	<CA+hUKGJ84L0yFD2S05WeCOXmBgBcYb2S5wMmzyohcMJeCwgksw@mail.gmail.com>
	<3915749.1739574230@sss.pgh.pa.us>
	<CA+hUKGKgHs9xjOZrjjuaEoYSxBvQiYOgDDQiQ6yTSzqFwx6d2Q@mail.gmail.com>
	<CAAKRu_Z35kymRA5ru1TeAZtnOd46o7iDCyJgF9P8TH5geKy46A@mail.gmail.com>
	<CA+hUKGL8bAsQNj-bB_qxFFzoQW+RMZ3P3zK2SVa6b08G55BdRg@mail.gmail.com>

Thomas Munro <thomas.munro@gmail.com> writes:
> Thanks!  It's green again.

The security team's Coverity instance complained about this patch:

*** CID 1642971:  Null pointer dereferences  (FORWARD_NULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/access/heap/vacuumlazy.c: 1295 in lazy_scan_heap()
1289     		buf = read_stream_next_buffer(stream, &per_buffer_data);
1290     
1291     		/* The relation is exhausted. */
1292     		if (!BufferIsValid(buf))
1293     			break;
1294     
>>>     CID 1642971:  Null pointer dereferences  (FORWARD_NULL)
>>>     Dereferencing null pointer "per_buffer_data".
1295     		blk_info = *((uint8 *) per_buffer_data);
1296     		CheckBufferIsPinnedOnce(buf);
1297     		page = BufferGetPage(buf);
1298     		blkno = BufferGetBlockNumber(buf);
1299     
1300     		vacrel->scanned_pages++;

Basically, Coverity doesn't understand that a successful call to
read_stream_next_buffer must set per_buffer_data here.  I don't
think there's much chance of teaching it that, so we'll just
have to dismiss this item as "intentional, not a bug".  However,
I do have a suggestion: I think the "per_buffer_data" variable
should be declared inside the "while (true)" loop not outside.
That way there is no chance of a value being carried across
iterations, so that if for some reason read_stream_next_buffer
failed to do what we expect and did not set per_buffer_data,
we'd be certain to get a null-pointer core dump rather than
accessing data from a previous buffer.

			regards, tom lane





view thread (81+ messages)  latest in thread

Message-ID: <626104.1739729538@sss.pgh.pa.us>
Permalink:  ../626104.1739729538@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/626104.1739729538@sss.pgh.pa.us

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, melanieplageman@gmail.com, thomas.munro@gmail.com, sawada.mshk@gmail.com, tomas@vondra.me, noah@leadboat.com, vignesh21@gmail.com, andres@anarazel.de, hlinnaka@iki.fi, byavuz81@gmail.com, robertmhaas@gmail.com, x4mmm@yandex-team.ru
  Subject: Re: Confine vacuum skip logic to lazy_scan_skip
  In-Reply-To: <626104.1739729538@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox