Hi Aleksander and Junwang,
Thanks for your comments. I have updated the patch accordingly.
Best Regards,
Xing
On Wed, Nov 1, 2023 at 7:44 PM Aleksander Alekseev <aleksan...@timescale.com>
wrote:
> Hi,
>
> > > I found that there's a nullable pointer being passed to strcmp() and
> > > can make the server crash. It can be reproduced on the latest master
> > > branch by crafting an extension[1]. Patch for fixing it is attatched.
> > >
> > > [1] https://github.com/higuoxing/guc_crash/tree/pg
>
> Thanks for reporting. I can confirm that the issue reproduces on the
> `master` branch and the proposed patch fixes it.
>
> > Can we set a string guc to NULL? If not, `*lconf->variable == NULL` would
> > be unnecessary.
>
> Judging by the rest of the code we better keep it, at least for consistenc.
>
> I see one more place with a similar code in guc.c around line 1472.
> Although I don't have exact steps to trigger a crash I suggest adding
> a similar check there.
>
> --
> Best regards,
> Aleksander Alekseev
>
From bf0a9f848e69097a034692ed7b0be0ad81a5d991 Mon Sep 17 00:00:00 2001
From: Xing Guo <higuox...@gmail.com>
Date: Wed, 1 Nov 2023 21:00:13 +0800
Subject: [PATCH v2] Don't use strcmp() with nullable pointers.
Passing a NULL pointer to strcmp() is an undefined behavior. It can make
the PostgreSQL server crash. This patch helps fix it.
---
src/backend/utils/misc/guc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 39d3775e80..1d62523412 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1473,7 +1473,8 @@ check_GUC_init(struct config_generic *gconf)
{
struct config_string *conf = (struct config_string *) gconf;
- if (*conf->variable != NULL && strcmp(*conf->variable, conf->boot_val) != 0)
+ if (*conf->variable != NULL &&
+ (conf->boot_val == NULL || strcmp(*conf->variable, conf->boot_val) != 0))
{
elog(LOG, "GUC (PGC_STRING) %s, boot_val=%s, C-var=%s",
conf->gen.name, conf->boot_val ? conf->boot_val : "<null>", *conf->variable);
@@ -5255,7 +5256,9 @@ get_explain_guc_options(int *num)
{
struct config_string *lconf = (struct config_string *) conf;
- modified = (strcmp(lconf->boot_val, *(lconf->variable)) != 0);
+ modified = (lconf->boot_val == NULL ||
+ *lconf->variable == NULL ||
+ strcmp(lconf->boot_val, *(lconf->variable)) != 0);
}
break;
--
2.42.0
