From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2,WEIRD_QUOTING autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 26796 invoked from network); 24 Feb 2023 16:12:39 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 24 Feb 2023 16:12:39 -0000 Received: (qmail 21549 invoked by uid 550); 24 Feb 2023 16:12:36 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 21507 invoked from network); 24 Feb 2023 16:12:34 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=kFY51QMi/8du/9RLwpypW+UymaKC+xV7uS9YVEycFwA=; b=nPldLu+zHy+P/9+pUTtJMizifsuy0aA5ccJbelJvbU72sB1o0bqHzFkrnZ4YF6JLoN mL0bxil2uzvKgkzqQDStZGBObbrS3PP2qSuD9J2svC5x6pnZgG2ve77W5YWWx3jHjBOj hu2HULCfNU+Cpg7LXLjkzIsI0vq0eFEcrWCylmEv6ruqsGYhZUPcYNLdTkQ8G0HlIxEH TCUbPDlRqO3Iy1YfvfFSly2Lj55kCqQoiH46YRJhk+wVZ3LIfRMad1Fc/vwQlWUGBS8C 9HGGaY1UtStreRj6rUUYkMit/8RLrz18hNj7fF/LOZwb9bE4KQF+GwFd782RkjhpXbXI cDfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=kFY51QMi/8du/9RLwpypW+UymaKC+xV7uS9YVEycFwA=; b=JaFb6bDFPrR2zvZGOW4G3MzaPfk7R8cynv5plyTnQ5uq+PxyzXceXttOw4JU15s1IS 4tk8MMVxZ1BFdv83bLn/v1zHJZ/DOoCTklNv/yO4jAuLykO42BIn9rxCFFS3XF0RwJFN VgR7VySn96NCx9Z/An6G37JP4FP3R07h2zD7xgTVtazcBvaVi7Lyj7GMLoNDm9GTodfi 3XxT29+va0/BSpwV8rWN4qvN8IfCO67qqZZF6krjkeVhToAiDRZkwcLn5oVLEOcvfjPk c1sC0TRJXSPPZbSTbK23NGhsRFMDoAWxnXr8dS+AcL6CVemfUBjpKkgazFEQUcb/kB9a sycg== X-Gm-Message-State: AO0yUKW1gs4HMyiTBi28I9m9TBNIPzi6ZeheJ7moOjj1Mmpya/bKEo78 SJM2lBLdKd4Xr05jcnBUGQ4UnId4x922lxnEH59v X-Google-Smtp-Source: AK7set/gIDAsYpjYhbgbKb3AN2r2w0dHRV6WKy2TdvUzLDwCMrszrxdAtHZPLRn6cNEoPN8eXtwx1rd/3AlmP87wHZg= X-Received: by 2002:a25:f90b:0:b0:a24:1001:1fd2 with SMTP id q11-20020a25f90b000000b00a2410011fd2mr3586175ybe.0.1677255142338; Fri, 24 Feb 2023 08:12:22 -0800 (PST) MIME-Version: 1.0 References: <20230224133413.GE4163@brightrain.aerifal.cx> <20230224135511.iunglqtcvpjeqgtv@gen2.localdomain> <20230224140739.GF4163@brightrain.aerifal.cx> <20230224141731.dm4w7fbfkczbx42e@gen2.localdomain> <20230224151334.ycgddzdc4o5a4m5q@gen2.localdomain> In-Reply-To: <20230224151334.ycgddzdc4o5a4m5q@gen2.localdomain> From: Tamir Duberstein Date: Fri, 24 Feb 2023 11:12:11 -0500 Message-ID: To: NRK Cc: Rich Felker , musl@lists.openwall.com Content-Type: multipart/alternative; boundary="000000000000f5e60805f5746458" Subject: Re: [musl] undefined behavior in fread.c --000000000000f5e60805f5746458 Content-Type: text/plain; charset="UTF-8" I agree, the caller's behavior is UB. I'll send them (freetype2) a patch. That said, do we want to avoid internal UB here anyway? - As mentioned earlier, glibc avoids the UB (and the lock). - llvm-libc does the same starting with https://github.com/llvm/llvm-project/commit/53c251b - uclibc avoids the UB but still locks: https://github.com/gittup/uClibc/blob/9dbf00b/libc/stdio/fread.c#L25 - FreeBSD avoids the UB but still locks: https://svnweb.freebsd.org/base/head/lib/libc/stdio/fread.c?view=markup#l76 - Android (bionic) avoids the UB but still locks: https://cs.android.com/android/platform/superproject/+/master:bionic/libc/stdio/stdio.cpp;l=1099;drc=4aa8f499f21ebf84101de34d68682d5388667001 Does this persuade? On Fri, Feb 24, 2023 at 10:13 AM NRK wrote: > On Fri, Feb 24, 2023 at 09:42:00AM -0500, Tamir Duberstein wrote: > > We could take the lock and still avoid UB with an early return. > > As Jens has pointed out, the UB in this case is the caller calling fread > with NULL - not in musl. > > And on a sidenote, I've always found - especially for the various mem* > functions - accepting 0 size but not accepting NULL arg (when n is 0) to > be a poor choice. A lot of the value that accepting 0 size provides is > diminished by not accepting NULL. > > And this affects more than just libc, too. Compilers like gcc/clang will > see a call like `memcmp(p, q, 0)` and will ""determine"" `p` and `q` are > non-null (which can lead to deleting any subsequent null-checks on those > pointers). > > But anyways, that was just a small rant. > > As things currently are, *even if* musl deal with the NULL pointer - any > caller calling fread with NULL is still in danger from compilers and > needs to fix it on their side. > > - NRK > --000000000000f5e60805f5746458 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I agree, the caller's behavior is UB. I'll send th= em (freetype2) a patch.

That said, do we want to avoid i= nternal UB here anyway?

- As mentioned earlier, glibc av= oids the UB (and the lock).
- llvm-libc does the same starting wi= th=C2=A0htt= ps://github.com/llvm/llvm-project/commit/53c251b
- uclibc avo= ids the UB but still locks:=C2=A0https://github.com/gittup/uClibc/blo= b/9dbf00b/libc/stdio/fread.c#L25

Does this pe= rsuade?

On Fri, Feb 24, 2023 at 10:13 AM NRK <nrk@disroot.org> wrote:
On Fri, Feb 24, 2023 at 09:42:00AM -0500= , Tamir Duberstein wrote:
> We could take the lock and still avoid UB with an early return.

As Jens has pointed out, the UB in this case is the caller calling fread with NULL - not in musl.

And on a sidenote, I've always found - especially for the various mem*<= br> functions - accepting 0 size but not accepting NULL arg (when n is 0) to be a poor choice. A lot of the value that accepting 0 size provides is
diminished by not accepting NULL.

And this affects more than just libc, too. Compilers like gcc/clang will see a call like `memcmp(p, q, 0)` and will ""determine""= ; `p` and `q` are
non-null (which can lead to deleting any subsequent null-checks on those pointers).

But anyways, that was just a small rant.

As things currently are, *even if* musl deal with the NULL pointer - any caller calling fread with NULL is still in danger from compilers and
needs to fix it on their side.

- NRK
--000000000000f5e60805f5746458--