Avoid crash in rare case of concurrent DROP
authorAlvaro Herrera
Fri, 5 Nov 2021 15:29:34 +0000 (12:29 -0300)
committerAlvaro Herrera
Fri, 5 Nov 2021 15:29:34 +0000 (12:29 -0300)
When a role being dropped contains is referenced by catalog objects that
are concurrently also being dropped, a crash can result while trying to
construct the string that describes the objects.  Suppress that by
ignoring objects whose descriptions are returned as NULL.

The majority of relevant codesites were already cautious about this
already; we had just missed a couple.

This is an old bug, so backpatch all the way back.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/17126-21887f04508cb5c8@postgresql.org

src/backend/catalog/dependency.c
src/backend/catalog/pg_shdepend.c

index 5565e6fc197d54bf2aaee174c70d7fc5640e0ec7..8394a0a241b5a4efe9ee82018bfee49ef86ec17b 100644 (file)
@@ -1109,6 +1109,10 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
 
        objDesc = getObjectDescription(obj);
 
+       /* An object being dropped concurrently doesn't need to be reported */
+       if (objDesc == NULL)
+           continue;
+
        /*
         * If, at any stage of the recursive search, we reached the object via
         * an AUTO, INTERNAL, PARTITION, or EXTENSION dependency, then it's
@@ -1133,23 +1137,28 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
        {
            char       *otherDesc = getObjectDescription(&extra->dependee);
 
-           if (numReportedClient < MAX_REPORTED_DEPS)
+           if (otherDesc)
            {
+               if (numReportedClient < MAX_REPORTED_DEPS)
+               {
+                   /* separate entries with a newline */
+                   if (clientdetail.len != 0)
+                       appendStringInfoChar(&clientdetail, '\n');
+                   appendStringInfo(&clientdetail, _("%s depends on %s"),
+                                    objDesc, otherDesc);
+                   numReportedClient++;
+               }
+               else
+                   numNotReportedClient++;
                /* separate entries with a newline */
-               if (clientdetail.len != 0)
-                   appendStringInfoChar(&clientdetail, '\n');
-               appendStringInfo(&clientdetail, _("%s depends on %s"),
+               if (logdetail.len != 0)
+                   appendStringInfoChar(&logdetail, '\n');
+               appendStringInfo(&logdetail, _("%s depends on %s"),
                                 objDesc, otherDesc);
-               numReportedClient++;
+               pfree(otherDesc);
            }
            else
                numNotReportedClient++;
-           /* separate entries with a newline */
-           if (logdetail.len != 0)
-               appendStringInfoChar(&logdetail, '\n');
-           appendStringInfo(&logdetail, _("%s depends on %s"),
-                            objDesc, otherDesc);
-           pfree(otherDesc);
            ok = false;
        }
        else
index cac89e0667311f2c1c24a3f2a6739347a6d8b43a..50515462a41286765ef8a4e26348b2cf29977c49 100644 (file)
@@ -1197,6 +1197,12 @@ storeObjectDescription(StringInfo descs,
 {
    char       *objdesc = getObjectDescription(object);
 
+   /*
+    * An object being dropped concurrently doesn't need to be reported.
+    */
+   if (objdesc == NULL)
+       return;
+
    /* separate entries with a newline */
    if (descs->len != 0)
        appendStringInfoChar(descs, '\n');