From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1033.google.com (mail-pj1-x1033.google.com [IPv6:2607:f8b0:4864:20::1033]) by sourceware.org (Postfix) with ESMTPS id 69B6C384781A for ; Mon, 28 Jun 2021 13:36:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 69B6C384781A Received: by mail-pj1-x1033.google.com with SMTP id g24so10145465pji.4 for ; Mon, 28 Jun 2021 06:36:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=hkxn2k3Jogkuxtiyqn7lbls7IF01bB0FtuIG31FOGYE=; b=XxBTtsagxJg9+gFx9fvGCYk3p5WINKQfA1cyigBl1sUXUrEbT/khwBJAx9jTiotCGV X32UWvQhkkmb1QQzx/6Xh5ztztac3MCHBryI23XQaLgrt58CqBYlIqAvctzNrBSj8iUh RMFMA5Y0EiEtm6MgnmurkSoDi/Lf9vq+D/g1jjbnybEfwFQmVK0gKK8hqquD3ik6RA1p QDnn3uuNJJCEDXx6aqT9/V75mXCa9+ooUa69n1BDtn3IXVOdLlMVZ7meEDTF+XpPjE72 xmERHXMXx0yP+hBrbx91FEXB/GG9tldvJvlihy/+hEwwNqxhCdw5cIiOhVMI/uQoHGQg S7GQ== X-Gm-Message-State: AOAM533UvIT3KREPFa+xWzhDZf7LpIpgJTyRCC5LriSThFhuH82LVE3O zQ1Z6n8kkBZu68HHv/0NNZzIiufYzjwOUw== X-Google-Smtp-Source: ABdhPJzeCanmEXzXgmGQlanlzm9K253DV/4sPQURcdZFh7mRkSfF/LZx8T2EpAu50BxCZ22AK/dChg== X-Received: by 2002:a17:902:ea84:b029:120:4515:5542 with SMTP id x4-20020a170902ea84b029012045155542mr23258185plb.53.1624887377050; Mon, 28 Jun 2021 06:36:17 -0700 (PDT) Received: from [192.168.1.108] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id t189sm8005955pfb.59.2021.06.28.06.36.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Jun 2021 06:36:16 -0700 (PDT) Subject: Re: [PATCH v3 05/24] linux: Add fallback for 64-bit time_t SO_TIMESTAMP{NS} To: Florian Weimer Cc: Adhemerval Zanella via Libc-alpha 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> <87zgvd7o1b.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella Message-ID: <6e08af41-f3d5-c57b-5d3a-f08b8cda8e25@linaro.org> Date: Mon, 28 Jun 2021 10:36:14 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <87zgvd7o1b.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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: Mon, 28 Jun 2021 13:36:20 -0000 On 25/06/2021 16:16, Florian Weimer wrote: > * Adhemerval Zanella: > >>> See: >>> >>> ruby: FTBFS with test suite failure (glibc 2.34 related) >>> >> >> Do we have a more contained testcase? I am trying to trigger using the ruby >> 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 least >> on i686 I can't really reproduce the issue (I am also running on 5.11 >> kernel). > > The ruby test case needs “set disable-randomization off” 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. I will create a testcase with a different struct msghdr sizes and alignment, along with different timestamps configurations. > >>> The disassembly suggests that GCC has detected some undefined behavior. >>> >>> This looks like a related bug: >>> >>> __cmsg_nxthdr in cmsg_nxthdr.c (CMSG_NXTHDR) has undefined behavior when setting up ancillary data >>> >>> >>> 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. Ok, it might the case. But on my testing I also tried to add random initialization data on msghdr. > >>> 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—are 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. There are two fold problems here: 1. With __ASSUME_TIME64_SYSCALLS, the SO_* timestamp constants being used will always 64-bit. It means we will need translation for programs built *without* _TIME_BITS=64. 2. Without __ASSUME_TIME64_SYSCALLS, we can't know what kind of timestamp is being configured on the socket. The setsockopt() will still maps to old 32-bit timestamps for ENOPROTOOPT. A new 64-bit entrypoint does not help much in my opinion: we can't know for sure if the socket has being configured with an old library or either pass with 32-bit timestamps with SCM_RIGHTS. > >> 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/unix/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; >> >> struct cmsghdr *cmsg, *last = NULL; >> int type = 0; >> @@ -69,7 +70,10 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize) >> >> /* fallthrough */ >> common: >> - memcpy (tvts, CMSG_DATA (cmsg), sizeof (tvts)); >> + memcpy (&tmp, CMSG_DATA (cmsg), sizeof (tmp)); >> + tvts[0] = tmp; >> + memcpy (&tmp, CMSG_DATA (cmsg) + sizeof (tmp), sizeof (tmp)); >> + tvts[1] = tmp; >> break; >> } > > Sorry, I can't quite wrap my head around this right now. > > Thanks, > Florian >