public inbox for [email protected]  

help / color / mirror / Atom feed

From: Heikki Linnakangas <[email protected]>
To: Mats Kindahl <[email protected]>
To: Tom Lane <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: [email protected]
Subject: Re: glibc qsort() vulnerability
Date: Wed, 7 Feb 2024 20:46:56 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+14425TqyUZ3-O7zYsW6u85yY5diCz-p_TPQ2bZovn-N-fQ9A@mail.gmail.com>
References: <CA+14426g2Wa9QuUpmakwPxXFWG_1FaY0AsApkvcTBy-YfS6uaw@mail.gmail.com>
	<[email protected]>
	<20240206205305.GB3891538@nathanxps13>
	<[email protected]>
	<CA+14425TqyUZ3-O7zYsW6u85yY5diCz-p_TPQ2bZovn-N-fQ9A@mail.gmail.com>

On 07/02/2024 11:09, Mats Kindahl wrote:
> On Tue, Feb 6, 2024 at 9:56 PM Tom Lane <[email protected] 
> <mailto:[email protected]>> wrote:
> 
>   Nathan Bossart <[email protected]
>     <mailto:[email protected]>> writes:
>      > Even if the glibc issue doesn't apply to Postgres, I'm tempted to
>     suggest
>      > that we make it project policy that comparison functions must be
>      > transitive.  There might be no real issues today, but if we write all
>      > comparison functions the way Mats is suggesting, it should be
>     easier to
>      > reason about overflow risks.
> 
>     A comparison routine that is not is probably broken, agreed.
>     I didn't look through the details of the patch --- I was more
>     curious whether we had a version of the qsort bug, because
>     if we do, we should fix that too.
> 
> The patch basically removes the risk of overflow in three routines and 
> just returns -1, 0, or 1, and adds a comment in one.
> 
> The routines modified do a subtraction of int:s and return that, which 
> can cause an overflow. This method is used for some int16 as well but 
> since standard conversions in C will perform the arithmetics in "int" 
> precision, this cannot overflow, so added a comment there. It might 
> still be a good idea to follow the same pattern for the int16 routines, 
> but since there is no bug there, I did not add them to the patch.

Doesn't hurt to fix the comparison functions, and +1 on using the same 
pattern everywhere.

However, we use our qsort() with user-defined comparison functions, and 
we cannot make any guarantees about what they might do. So we must 
ensure that our qsort() doesn't overflow, no matter what the comparison 
function does.

Looking at our ST_SORT(), it seems safe to me.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

view thread (16+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: glibc qsort() vulnerability
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox