Re: glibc qsort() vulnerability

On Thu, Feb 8, 2024 at 7:44 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Nathan Bossart <nathandbossart(at)gmail(dot)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 ...
>

I picked a macro simply because it can deal with all kinds of integers, but
if we want to have separate implementations for each then inline functions
work just as well and will be safer.

Best wishes,
Mats Kindahl

> regards, tom lane
>