Re: glibc qsort() vulnerability

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Mats Kindahl <mats@timescale.com>, Thomas Munro <thomas.munro@gmail.com>, Andres Freund <andres@anarazel.de>, Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@lists.postgresql.org
Date: 2024-02-08T18:44:02Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Use new overflow-safe integer comparison functions.

  2. Introduce overflow-safe integer comparison functions.

  3. Replace calls to pg_qsort() with the qsort() macro.

  4. Switch over to using our own qsort() all the time, as has been proposed

Nathan Bossart <nathandbossart@gmail.com> writes:
> On Thu, Feb 08, 2024 at 02:16:11PM +0100, Mats Kindahl wrote:
>> +/*
>> + * Compare two integers and return -1, 0, or 1 without risking overflow.
>> + *
>> + * This macro is used to avoid running into overflow issues because a simple
>> + * subtraction of the two values when implementing a cmp function for qsort().
>> +*/
>> +#define INT_CMP(lhs,rhs) (((lhs) > (rhs)) - ((lhs) < (rhs)))

> I think we should offer a few different macros, i.e., separate macros for
> int8, uint8, int16, uint16, int32, etc.  For int16, we can do something
> faster like

> 	(int32) (lhs) - (int32) (rhs)

> but for int32, we need to do someting more like what's in the patch.

Are we okay with using macros that (a) have double evaluation hazards
and (b) don't enforce the data types being compared are the same?
I think static inlines might be a safer technology.  Perhaps it's
okay given the only expected use is in qsort comparators, but ...

			regards, tom lane