From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 3E7BB3858034 for ; Fri, 25 Jun 2021 19:16:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3E7BB3858034 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-105-RvPq8HduNreZNz5z5isutQ-1; Fri, 25 Jun 2021 15:16:36 -0400 X-MC-Unique: RvPq8HduNreZNz5z5isutQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4C721100C633; Fri, 25 Jun 2021 19:16:35 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-228.ams2.redhat.com [10.36.112.228]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6F0D85D6D7; Fri, 25 Jun 2021 19:16:34 +0000 (UTC) From: Florian Weimer To: Adhemerval Zanella Cc: Adhemerval Zanella via Libc-alpha Subject: Re: [PATCH v3 05/24] linux: Add fallback for 64-bit time_t SO_TIMESTAMP{NS} References: <20210607203613.282543-1-adhemerval.zanella@linaro.org> <20210607203613.282543-6-adhemerval.zanella@linaro.org> <87k0mi7yzh.fsf@oldenburg.str.redhat.com> <24aa519a-a9d3-c452-2478-29592cc29a23@linaro.org> Date: Fri, 25 Jun 2021 21:16:32 +0200 In-Reply-To: <24aa519a-a9d3-c452-2478-29592cc29a23@linaro.org> (Adhemerval Zanella's message of "Fri, 25 Jun 2021 15:11:11 -0300") Message-ID: <87zgvd7o1b.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Jun 2021 19:16:46 -0000 * Adhemerval Zanella: >> See: >>=20 >> ruby: FTBFS with test suite failure (glibc 2.34 related) >> > > Do we have a more contained testcase? I am trying to trigger using the ru= by > example it is kind hard to no make it use the system libraries. > > I am trying to create a testcase with different cmsghdr sizes, but at lea= st=20 > on i686 I can't really reproduce the issue (I am also running on 5.11 > kernel). The ruby test case needs =E2=80=9Cset disable-randomization off=E2=80=9D in= GDB and even then passes sporadically. Without it, the test always succeeds for some reason. With the GCC 11 compiled binaries in Fedora, we fault in an isolated error path (null pointer store followed by ud2 instruction). I haven't had time to look at the disassembly and GIMPLE dumps to see what is going on. >> The disassembly suggests that GCC has detected some undefined behavior. >>=20 >> This looks like a related bug: >>=20 >> __cmsg_nxthdr in cmsg_nxthdr.c (CMSG_NXTHDR) has undefined behavior wh= en setting up ancillary data >> >>=20 >> I believe you cannot use CMSG_NXTHDR to append data in this way. > > I think I am getting luck here because the example provided does pass, > even when tested against valgrind, asan, and ubsan (using gcc 10). Hmm. I think it depends on previous buffer contents. >> The other question is why this code is running at all. Doing this >> complex conversion for a 32-bit applications doing a 32-bit function >> call on a kernel which supports 32-bit system calls does not make much >> sense to me. > > Mainly because kernel does not provide a 64-bit recvmsg, different than > recvmmg. So for 64-bit time_t calls we need to do the conversion, > although it won't help much if the caller does not provide a buffer large > enough. Yes, but the Fedora build does not use 64-bit time_t, so this conversion is kind of pointless there. I see that we didn't add a __recvmsg_time64 entrypoint (and similar entrypoints for the other cases). I think that's a mistake, we should have those to future-proof things (similar for ioctl and fcntl=E2=80=94are = there any other multiplexers?). > I don't think we can improve it much by adding a 64-bit symbol: the > underlying syscall will be the same we don't prior hand which > SO_TIMESTAMP value were used to setup the timer (32-bit or 64-bit > one). Yes, but old 32-bit binaries can avoid running the new code if we have separate entrypoint. > Revising the code I found one issue with __convert_scm_timestamps, > where the memcpy might indeed being accessing invalid memory: > > diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/u= nix/sysv/linux/convert_scm_timestamps.c > index d75a4618dd..2c61267fec 100644 > --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c > +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c > @@ -44,7 +44,8 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t= msgsize) > 'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a > 'struct __kernel_timespec'. In either case it is two uint64_t > members. */ > - uint64_t tvts[2]; > + int64_t tvts[2]; > + int32_t tmp; > =20 > struct cmsghdr *cmsg, *last =3D NULL; > int type =3D 0; > @@ -69,7 +70,10 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_= t msgsize) > =20 > /* fallthrough */ > common: > - memcpy (tvts, CMSG_DATA (cmsg), sizeof (tvts)); > + memcpy (&tmp, CMSG_DATA (cmsg), sizeof (tmp)); > + tvts[0] =3D tmp; > + memcpy (&tmp, CMSG_DATA (cmsg) + sizeof (tmp), sizeof (tmp)); > + tvts[1] =3D tmp; > break; > } Sorry, I can't quite wrap my head around this right now. Thanks, Florian