Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t5OwN-005rWa-2O for pgsql-hackers@arkaria.postgresql.org; Mon, 28 Oct 2024 12:33:47 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1t5OwK-006IRn-5z for pgsql-hackers@arkaria.postgresql.org; Mon, 28 Oct 2024 12:33:44 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t5OwJ-006IRf-Oy for pgsql-hackers@lists.postgresql.org; Mon, 28 Oct 2024 12:33:44 +0000 Received: from mail-pj1-x1036.google.com ([2607:f8b0:4864:20::1036]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1t5OwI-003PiD-1Z for pgsql-hackers@postgresql.org; Mon, 28 Oct 2024 12:33:43 +0000 Received: by mail-pj1-x1036.google.com with SMTP id 98e67ed59e1d1-2e91403950dso386417a91.3 for ; Mon, 28 Oct 2024 05:33:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730118819; x=1730723619; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=GYjanXO4hCXss2+o0hU606MxhPskiGHnq+A6xNCvsy4=; b=jgYdymYTHAYh8lTEH0FZah//tWrAG1YWgU22uLi2EzzzoHgcFT0HH8P6SK3msorMwj iOrrFzcd4IW/4PHl5CXBsu6JyciJXtvtUOaZBt5EDKzcNIoUnlukRY8XmdeKDXK/bOsS XsdQHj/FdDRpj1344t3LP79yLy36TeguLcZ88tHHZSXPhEH0AwHDLA//eqg/QaChNvw0 IYexVlSfsXAANL/6YeQFZzN4YqKusRJdP9qvm8MTzBvZ1l7tcgvjc8wlbRnHkmwscpd0 hjnQgWutlNTFWiC+fJ2f/Ywx2O3FnaTZEgOQp6SBiqH6MJ6XrUrMHwsoRqX4mT4Vtaxz JFxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730118819; x=1730723619; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=GYjanXO4hCXss2+o0hU606MxhPskiGHnq+A6xNCvsy4=; b=huYdFQUP3bDAMvx5oQO5+RoDwR6goCyWsBuZk3j5f/ieNaSEM1+ExLfUTbOPtchOKn v+EispMNRh0389sE4K1ZC4DIIkULVJuKe1yyvd+pZKciOB0c+yrhXOpscE5CutUJHS+3 1s/SZgJXZiPf86nebxhqz2TpyQaeDcwPDvQCilJu24nf/6+UfX3ZNtxik/zo/lWC94/e y3Rt7DRJOdfE5S9hUnLMuBS8jhDR3eFikBBXx6/cIfHCwYQn5U5FUW9yigEHi5yYYty5 DdzeOAFoT7+V9WxGEDaVezTD1QjgVOcVOUzEG/qQrkPzT09sAhZkXRTHJfitbFvyrnag gJkg== X-Forwarded-Encrypted: i=1; AJvYcCV5SrnrnkuyjPkOr8HIAK++uwIxlHkcvDqO8U2yBtEhUlv/CKxJpn0pV7jwtQHzEsdv4DyK2w4mnzGHhi05@postgresql.org X-Gm-Message-State: AOJu0YzOC8DSiGN9+Fxtmxcuu8T3qkn4xFo+zvpFk8nK42leqeG3h7E0 3cN3GcA9U52W4J8abQwtQ5uldjhV0oRmta+YmJx+MJy5GRqgiSk09h51ztSnI94Yn4aZtGh1M85 TFHilAH/6hzePhs6Cst2ZGfmMJ2s= X-Google-Smtp-Source: AGHT+IFo1m2I4zgyvrYC4UWnBzKw896iFtrQQrkWaSgXxoLKgGPP11u9mZ4xL71tbp685UYqH8iZAuGP9qpRrj/qhss= X-Received: by 2002:a17:90b:4b06:b0:2e2:cd62:549c with SMTP id 98e67ed59e1d1-2e8f10867cemr10033777a91.22.1730118819505; Mon, 28 Oct 2024 05:33:39 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Ranier Vilela Date: Mon, 28 Oct 2024 09:33:27 -0300 Message-ID: Subject: Re: Avoid possible overflow (src/port/bsearch_arg.c) To: Heikki Linnakangas Cc: Nathan Bossart , Tomas Vondra , Pg Hackers Content-Type: multipart/alternative; boundary="000000000000a7ed11062588ad22" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000a7ed11062588ad22 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Em seg., 28 de out. de 2024 =C3=A0s 09:13, Heikki Linnakangas escreveu: > On 09/10/2024 19:16, Ranier Vilela wrote: > > Em ter., 8 de out. de 2024 =C3=A0s 18:28, Nathan Bossart > > > escreveu: > > > > On Tue, Oct 08, 2024 at 04:09:00PM -0300, Ranier Vilela wrote: > > > The port function *bsearch_arg* mimics the C function > > > *bsearch*. > > > > > > The API signature is: > > > void * > > > bsearch_arg(const void *key, const void *base0, > > > size_t nmemb, size_t size, > > > int (*compar) (const void *, const void *, void *), > > > void *arg) > > > > > > So, the parameter *nmemb* is size_t. > > > Therefore, a call with nmemb greater than INT_MAX is possible. > > > > > > Internally the code uses the *int* type to iterate through the > > number of > > > members, which makes overflow possible. > > > > I traced this back to commit bfa2cee (v14), which both moved > > bsearch_arg() > > to its current location and adjusted the style a bit. Your patch > looks > > reasonable to me. > > > > Thanks for looking. > > Committed, thanks. > Thank you. > > Based on the original discussion on bfa2cee, I couldn't figure out where > exactly this new bsearch implementation originated from, but googling > around, probably *BSD or libiberty. Tomas, do you remember? Not that it > matters, but I'm curious. > > Some of those other implementations have fixed this, others have not. > And they all seem to also have the "involes" typo in the comment that we > fixed in commit 7ef8b52cf07 :-). Ranier, you might want to submit this > fix to those other projects too. > Sure, I can try. best regards, Ranier Vilela --000000000000a7ed11062588ad22 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Em seg., 28 de out. de 2024 =C3=A0s 09:13, Heikki Linnakangas <hlinnaka@iki.fi> escreveu:
On 09/10/2024 19:16, Ran= ier Vilela wrote:
> Em ter., 8 de out. de 2024 =C3=A0s 18:28, Nathan Bossart
> <nath= andbossart@gmail.com <mailto:nathandbossart@gmail.com>> escreveu:
>
>=C2=A0 =C2=A0 =C2=A0On Tue, Oct 08, 2024 at 04:09:00PM -0300, Ranier Vi= lela wrote:
>=C2=A0 =C2=A0 =C2=A0 > The port function *bsearch_arg* mimics the C = function
>=C2=A0 =C2=A0 =C2=A0 > *bsearch*.
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > The API signature is:
>=C2=A0 =C2=A0 =C2=A0 > void *
>=C2=A0 =C2=A0 =C2=A0 > bsearch_arg(const void *key, const void *base= 0,
>=C2=A0 =C2=A0 =C2=A0 > size_t nmemb, size_t size,
>=C2=A0 =C2=A0 =C2=A0 > int (*compar) (const void *, const void *, vo= id *),
>=C2=A0 =C2=A0 =C2=A0 > void *arg)
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > So, the parameter *nmemb* is size_t.
>=C2=A0 =C2=A0 =C2=A0 > Therefore, a call with nmemb greater than INT= _MAX is possible.
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > Internally the code uses the *int* type to it= erate through the
>=C2=A0 =C2=A0 =C2=A0number of
>=C2=A0 =C2=A0 =C2=A0 > members, which makes overflow possible.
>
>=C2=A0 =C2=A0 =C2=A0I traced this back to commit bfa2cee (v14), which b= oth moved
>=C2=A0 =C2=A0 =C2=A0bsearch_arg()
>=C2=A0 =C2=A0 =C2=A0to its current location and adjusted the style a bi= t.=C2=A0 Your patch looks
>=C2=A0 =C2=A0 =C2=A0reasonable to me.
>
> Thanks for looking.

Committed, thanks.
Thank you.
=C2=A0

Based on the original discussion on bfa2cee, I couldn't figure out wher= e
exactly this new bsearch implementation originated from, but googling
around, probably *BSD or libiberty. Tomas, do you remember? Not that it matters, but I'm curious.

Some of those other implementations have fixed this, others have not.
And they all seem to also have the "involes" typo in the comment = that we
fixed in commit 7ef8b52cf07 :-). Ranier, you might want to submit this
fix to those other projects too.
Sure, I can try.

best regards,
Ranier Vilela
--000000000000a7ed11062588ad22--