pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Abhijit Menon-Sen <ams@toroid.org>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: introduce dynamic shared memory registry
Date: Sun, 21 Jan 2024 16:13:20 -0600
Message-ID: <20240121221320.GA1682397@nathanxps13> (raw )
In-Reply-To: <3348869.1705854106@sss.pgh.pa.us >
References: <CALj2ACVaFuUys-Fkq8SQDpTOJnmaqO6C8RshbYtbW-DJD7LbAA@mail.gmail.com >
<20240112172152.GA3803561@nathanxps13 >
<ZaF6UpYImGqVIhVp@toroid.org >
<20240112194555.GA3804945@nathanxps13 >
<20240113214124.GA4162765@nathanxps13 >
<CALj2ACVKFPm7nCrHMko7JJpsFJU23abzYyMS8S2Pz8r4M=rW7w@mail.gmail.com >
<20240116160748.GB100542@nathanxps13 >
<CALj2ACWuiAm7Rr13PFfaiQfOHUoCbJOJvU72jyLnj4xfoEOAEg@mail.gmail.com >
<20240119204636.GA1178173@nathanxps13 >
<3348869.1705854106@sss.pgh.pa.us >
On Sun, Jan 21, 2024 at 11:21:46AM -0500, Tom Lane wrote:
> Coverity complained about this:
>
> *** CID 1586660: Null pointer dereferences (NULL_RETURNS)
> /srv/coverity/git/pgsql-git/postgresql/src/backend/storage/ipc/dsm_registry.c: 185 in GetNamedDSMSegment()
> 179 }
> 180 else if (!dsm_find_mapping(entry->handle))
> 181 {
> 182 /* Attach to existing segment. */
> 183 dsm_segment *seg = dsm_attach(entry->handle);
> 184
>>>> CID 1586660: Null pointer dereferences (NULL_RETURNS)
>>>> Dereferencing a pointer that might be "NULL" "seg" when calling "dsm_pin_mapping".
> 185 dsm_pin_mapping(seg);
> 186 ret = dsm_segment_address(seg);
> 187 }
> 188 else
> 189 {
> 190 /* Return address of an already-attached segment. */
>
> I think it's right --- the comments for dsm_attach explicitly
> point out that a NULL return is possible. You need to handle
> that scenario in some way other than SIGSEGV.
Oops. I've attached an attempt at fixing this. I took the opportunity to
clean up the surrounding code a bit.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-fix-coverity-complaint.patch (text/x-diff, 1.4K) download
From f4c1c7a7ce8bccf7251e384f895f34beb33f839e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Sun, 21 Jan 2024 16:05:16 -0600
Subject: [PATCH v1 1/1] fix coverity complaint
---
src/backend/storage/ipc/dsm_registry.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index ac11f51375..c178173653 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -177,19 +177,22 @@ GetNamedDSMSegment(const char *name, size_t size,
(errmsg("requested DSM segment size does not match size of "
"existing segment")));
}
- else if (!dsm_find_mapping(entry->handle))
+ else
{
- /* Attach to existing segment. */
- dsm_segment *seg = dsm_attach(entry->handle);
+ dsm_segment *seg = dsm_find_mapping(entry->handle);
+
+ /* If the existing segment is not already attached, attach it now. */
+ if (seg == NULL)
+ {
+ seg = dsm_attach(entry->handle);
+ if (seg == NULL)
+ elog(ERROR, "could not map dynamic shared memory segment");
+
+ dsm_pin_mapping(seg);
+ }
- dsm_pin_mapping(seg);
ret = dsm_segment_address(seg);
}
- else
- {
- /* Return address of an already-attached segment. */
- ret = dsm_segment_address(dsm_find_mapping(entry->handle));
- }
dshash_release_lock(dsm_registry_table, entry);
MemoryContextSwitchTo(oldcontext);
--
2.25.1
view thread (51+ messages) latest in thread
Message-ID: <20240121221320.GA1682397@nathanxps13>
Permalink: ../20240121221320.GA1682397@nathanxps13/
Also on: postgresql.org/message-id/20240121221320.GA1682397@nathanxps13
copy link · copy postgr.es · copy git am
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: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, bharath.rupireddyforpostgres@gmail.com, ams@toroid.org, michael@paquier.xyz, robertmhaas@gmail.com
Subject: Re: introduce dynamic shared memory registry
In-Reply-To: <20240121221320.GA1682397@nathanxps13>
* 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