From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.efficios.com (mail.efficios.com [167.114.26.124]) by sourceware.org (Postfix) with ESMTPS id 005133896815 for ; Thu, 30 Apr 2020 19:39:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 005133896815 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 990A7291A3A; Thu, 30 Apr 2020 15:39:06 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 5Gowbg0YD9Xp; Thu, 30 Apr 2020 15:39:06 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 513382917E2; Thu, 30 Apr 2020 15:39:06 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 513382917E2 X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id wCFmdoviJtgi; Thu, 30 Apr 2020 15:39:06 -0400 (EDT) Received: from mail03.efficios.com (mail03.efficios.com [167.114.26.124]) by mail.efficios.com (Postfix) with ESMTP id 485BF291572; Thu, 30 Apr 2020 15:39:06 -0400 (EDT) Date: Thu, 30 Apr 2020 15:39:06 -0400 (EDT) From: Mathieu Desnoyers To: Florian Weimer Cc: carlos , Joseph Myers , Szabolcs Nagy , libc-alpha Message-ID: <1904112038.78406.1588275546194.JavaMail.zimbra@efficios.com> In-Reply-To: <87zhaskgsp.fsf@oldenburg2.str.redhat.com> References: <20200428171513.22926-1-mathieu.desnoyers@efficios.com> <875zdhmaft.fsf@oldenburg2.str.redhat.com> <1287616647.77866.1588263099045.JavaMail.zimbra@efficios.com> <878sidkk0z.fsf@oldenburg2.str.redhat.com> <1972833271.77975.1588265754974.JavaMail.zimbra@efficios.com> <874kt0lx6i.fsf@oldenburg2.str.redhat.com> <729499446.78182.1588267203324.JavaMail.zimbra@efficios.com> <87zhaskgsp.fsf@oldenburg2.str.redhat.com> Subject: Re: [RFC PATCH glibc 1/3] glibc: Perform rseq(2) registration at C startup and thread creation (v18) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Originating-IP: [167.114.26.124] X-Mailer: Zimbra 8.8.15_GA_3918 (ZimbraWebClient - FF75 (Linux)/8.8.15_GA_3895) Thread-Topic: glibc: Perform rseq(2) registration at C startup and thread creation (v18) Thread-Index: 6EyI1Wf26PYasEGkxyGnpXBYzwu+fQ== X-Spam-Status: No, score=-14.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_2, 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: Thu, 30 Apr 2020 19:39:08 -0000 ----- On Apr 30, 2020, at 1:46 PM, Florian Weimer fweimer@redhat.com wrote: > * Mathieu Desnoyers: >=20 >> ----- On Apr 30, 2020, at 1:07 PM, Florian Weimer fweimer@redhat.com wro= te: >> [...] >>> __libc_fatal does not attribute the error to glibc, so I suggest to >>> start the error messages with =E2=80=9Cglibc fatal error: =E2=80=9D, so= that people know >>> where to look. >> >> OK. Is there a strict requirement on limiting to 80 columns for code >> including an error message string in glibc ? IOW: >> >> if (__rseq_abi.cpu_id !=3D RSEQ_CPU_ID_UNINITIALIZED) >> __libc_fatal ("glibc fatal error: rseq already initialized for this = thread\n"); >> >> or >> >> if (__rseq_abi.cpu_id !=3D RSEQ_CPU_ID_UNINITIALIZED) >> __libc_fatal ("glibc fatal error: " >> "rseq already initialized for this thread\n"); >> >> ? >=20 > The latter, please. Some code also uses >=20 > if (__rseq_abi.cpu_id !=3D RSEQ_CPU_ID_UNINITIALIZED) > __libc_fatal ("\ > glibc fatal error: rseq already initialized for this thread\n"); >=20 > But that's not really my preference. >=20 > (Trimmed the Cc: list a bit, we are really down to glibc specifics at > this point.) One last question with respect to handling of rseq errno values. We current= ly have (based on my own rseq(2) man page, not upstream yet): ERRORS EINVAL Either flags contains an invalid value, or rseq contains an a= ddress which is not appropriately aligned, or rseq_len contains a size that does not match the size rece= ived on registration. ENOSYS The rseq() system call is not implemented by this kernel. EFAULT rseq is an invalid address. EBUSY Restartable sequence is already registered for this thread. EPERM The sig argument on unregistration does not match the signatu= re received on registration. So with the current suggestions, we basically treat "EBUSY" as a __libc_fat= al (), which is fine, and all other errno values (EINVAL, ENOSYS, EFAULT, EPERM) a= s conditions which will just disable rseq for the thread by marking cpu_id as RSEQ_CPU_ID_REGISTRATION_FAILED. I'm hesitant to treat "EINVAL", and "EFAULT" in this way, as those errno sh= ould IMHO really abort libc as well with an appropriate __libc_fatal () message, beca= use something is clearly going wrong and we don't want to hide it under the carpet by jus= t disabling rseq support silently. Also, I personally consider that adding an additional errno value to an existing system call for a given set of supported system call parameters is an ABI breakage, but I _know_ the Linux kernel community as a whole does not feel that way, and they are known to have pretty much silently added additional errno values to existing system calls as long as nobody complains. Considering this, I wonder if we should be strict and e.g. do: const char *msg =3D NULL; switch (INTERNAL_SYSCALL_ERRNO (ret)) { case ENOSYS: case EPERM: /* rseq system call is unavailable or not permitted. */ __rseq_abi.cpu_id =3D RSEQ_CPU_ID_REGISTRATION_FAILED; break; case EINVAL: msg =3D "glibc fatal error: rseq already registered for this thread\n"; break; case EBUSY: msg =3D "glibc fatal error: rseq parameters are invalid"; case EFAULT: msg =3D "glibc fatal error: rseq is an invalid address"; break; default: msg =3D "glibc fatal error: unexpected rseq errno"; break; } if (msg) __libc_fatal (msg); Also considering that __libc_fatal only takes a string as parameter, I wonder if there is a facility to print the errno string I could use instead of __libc_fatal () ? Thanks, Mathieu --=20 Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com