Avoid double-free in vacuumlo error path.

tglsfdc · tglsfdc · commit 99e414cacd06 · 2019-03-24T15:13:21.000-04:00
The code would do "PQclear(res)" twice if lo_unlink failed, evidently due to careless thinking about how far out a "break" would break. Remove the extra PQclear and adjust the loop logic so that we'll fall out of both levels of loop after an error, as was clearly the intent. Spotted by Coverity. I have no idea why it took this long to notice, since the bug has been there since commit 67ccbb0. Accordingly, back-patch to all supported branches.
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
@@ -302,7 +302,7 @@ vacuumlo(const char *database, const struct _param * param)
 
 	deleted = 0;
 
-	while (1)
+	do
 	{
 		res = PQexec(conn, buf);
 		if (PQresultStatus(res) != PGRES_TUPLES_OK)
@@ -340,8 +340,7 @@ vacuumlo(const char *database, const struct _param * param)
 					if (PQtransactionStatus(conn) == PQTRANS_INERROR)
 					{
 						success = false;
-						PQclear(res);
-						break;
+						break;	/* out of inner for-loop */
 					}
 				}
 				else
@@ -379,7 +378,7 @@ vacuumlo(const char *database, const struct _param * param)
 		}
 
 		PQclear(res);
-	}
+	} while (success);
 
 	/*
 	 * That's all folks!

Original file line number	Diff line number	Diff line change
`@@ -302,7 +302,7 @@ vacuumlo(const char database, const struct _param param)`
`302`	`302`
`303`	`303`	`deleted = 0;`
`304`	`304`
`305`		`- while (1)`
	`305`	`+ do`
`306`	`306`	`{`
`307`	`307`	`res = PQexec(conn, buf);`
`308`	`308`	`if (PQresultStatus(res) != PGRES_TUPLES_OK)`
`@@ -340,8 +340,7 @@ vacuumlo(const char database, const struct _param param)`
`340`	`340`	`if (PQtransactionStatus(conn) == PQTRANS_INERROR)`
`341`	`341`	`{`
`342`	`342`	`success = false;`
`343`		`- PQclear(res);`
`344`		`- break;`
	`343`	`+ break; /* out of inner for-loop */`
`345`	`344`	`}`
`346`	`345`	`}`
`347`	`346`	`else`
`@@ -379,7 +378,7 @@ vacuumlo(const char database, const struct _param param)`
`379`	`378`	`}`
`380`	`379`
`381`	`380`	`PQclear(res);`
`382`		`- }`
	`381`	`+ } while (success);`
`383`	`382`
`384`	`383`	`/*`
`385`	`384`	`* That's all folks!`