From patchwork Sat Apr 15 16:17:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Bugaev X-Patchwork-Id: 1769298 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=wSJfkisQ; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PzJPx5vRLz1yXv for ; Sun, 16 Apr 2023 02:17:57 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 20DB33857737 for ; Sat, 15 Apr 2023 16:17:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 20DB33857737 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1681575469; bh=qPY4q+HY6OPUszq4rb0/0t3sF2nfr5vs3Jx++M99uOw=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=wSJfkisQFvsw3+vdIJ0rMsHd/1fYq4JLo5lM9CCQggwvae+4LLmrY1nejYEIXsI0z //FstbnPew+HsPGyP3thkELXxEgrvT8wAzLObYeR5j8WLt5KbLQGMeV9Vvt5M/aG8A 9YJd0YFdUAW8t0MmEkf8E/6bJ3IxBzRGbS3v/GMw= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by sourceware.org (Postfix) with ESMTPS id DDF103858D20 for ; Sat, 15 Apr 2023 16:17:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DDF103858D20 Received: by mail-lf1-x136.google.com with SMTP id 2adb3069b0e04-4ec816c9b62so472735e87.2 for ; Sat, 15 Apr 2023 09:17:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681575450; x=1684167450; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=qPY4q+HY6OPUszq4rb0/0t3sF2nfr5vs3Jx++M99uOw=; b=cIupCxGIYprX9f/TKsaSVgs+z+off4pro/TnrQQhnykqknj0gWutA7ITYX9rNE2/VZ PcsHPGYYaCeRGMw3AKwaiFthg5t+E4g+alYmBmvWVIq98Azc9VqAApMuOlzmeY1YWBbh gBcUs4BaqjrNP8uWFDz8cPg0j5uv+ezVcR97WdQoXjb4fOndXvZY+F0JchFGCva7Xlpb xfRhmnwAhF8Xy0DX5UIHPvvmkpq/8cfa1AlbL09nzT9lAvEA77RBXBamdwFls2n7Gf6K SDBAXNMLDpzJN3rO6vObxnYZf4cX4mWDLbb4ov55Fb74C2sEh0nkC/cMX8RjXb9yN9Mr Wbbw== X-Gm-Message-State: AAQBX9cz0SqAP5l1jn2fbrLZ/SM5ZVAtfH8gLCbswvR8F1qFPaYcwZhi S3iRmpDyUCYz/SNS5pyQl4jtS9FGacr0fw== X-Google-Smtp-Source: AKy350ZIeEmQ6EipdBuO4awn8GKEWzeid4yKi9v7igufVv4DVen5wbYKYSK+zOmnOR0ZbfhfV754MA== X-Received: by 2002:a19:f702:0:b0:4a4:68b8:f4f1 with SMTP id z2-20020a19f702000000b004a468b8f4f1mr647674lfe.55.1681575449878; Sat, 15 Apr 2023 09:17:29 -0700 (PDT) Received: from surface-pro-6.. ([2a00:1370:818c:4a57:1942:fa6f:6d42:319f]) by smtp.gmail.com with ESMTPSA id k15-20020a2e240f000000b002a79de1dcf4sm1400188ljk.121.2023.04.15.09.17.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 15 Apr 2023 09:17:29 -0700 (PDT) To: libc-alpha@sourceware.org, bug-hurd@gnu.org Cc: Samuel Thibault , Sergey Bugaev Subject: [PATCH 0/1] Let's improve/fix ccty handling Date: Sat, 15 Apr 2023 19:17:17 +0300 Message-Id: <20230415161718.80668-1-bugaevc@gmail.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Sergey Bugaev via Libc-alpha From: Sergey Bugaev Reply-To: Sergey Bugaev Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" Hello! Here are some assorted thoughts on cttys (controlling terminals). Running rpctrace on any simple program, I see this: 13<--33(pid1354)->term_getctty () = 0 4<--39(pid1354) task16(pid1354)->mach_port_deallocate (pn{ 10}) = 0 13<--33(pid1354)->term_open_ctty (0 0) = 0x40000016 (Invalid argument) 13<--33(pid1354)->term_getctty () = 0 4<--39(pid1354) task16(pid1354)->mach_port_deallocate (pn{ 10}) = 0 13<--33(pid1354)->term_open_ctty (0 0) = 0x40000016 (Invalid argument) 13<--33(pid1354)->term_getctty () = 0 4<--39(pid1354) task16(pid1354)->mach_port_deallocate (pn{ 10}) = 0 13<--33(pid1354)->term_open_ctty (0 0) = 0x40000016 (Invalid argument) This happens inside init_dtable () -> _hurd_port2fd (). We can notice that: 1. stdin, stdout, stderr all refer to the same port (/dev/ttyp1 in my case), and yet glibc tries to set up ctty handling for each fd separately, making way too many RPCs. This happens each time a process starts up => not cool! 2. ctty setup actually fails with EINVAL! -- even though this is a valid tty. I'm attaching a patch to fix #1, so now we get this: 13<--33(pid1255)->term_getctty () = 0 4<--39(pid1255) task16(pid1255)->mach_port_deallocate (pn{ 10}) = 0 13<--33(pid1255)->term_open_ctty (0 0) = 0x40000016 (Invalid argument) Much better, but still, what's that error? Well, this is what S_term_open_ctty () checks: if (pid <= 0 || pgrp <= 0) { return EINVAL; } so it makes sense that it errors out if we pass pid = 0, pgrp = 0. But why are we passing that? Looking at the code, it does __term_open_ctty (dport, _hurd_pid, _hurd_pgrp, &ctty); so clearly at this point our _hurd_pid / _hurd_pgrp are not initialized yet, and indeed the proc_getpids_request / proc_getpgrp_request RPCs happen much later in the trace. I think commit 1ccbb9258eed0f667edf459a28ba23a805549b36 "hurd: Notify the proc server later during initialization" was the one that has broken things here (I guess ctty working is not a part of the testsuite? :). The commit makes sense, in that we want to start doing signals late; but apparently we need to fetch our pid/pgrp early. This itself would be fine, but I'm not sure how to implement it. Fetching pid/pgrp is done in init_pids (in hurdpid.c), which is attached to the _hurd_proc_subinit hook (and is the only thing attached there). Do I understand it right that this hooks system is libc-internal? -- or can user programs add their own functions to the hooks? (Looks like it works through the linker-defined start/end section symbols, which wouldn't work across DSOs, which is why I think it must be libc-internal.) If it is internal to libc, we're not bounded by compatibility concerns, and have more liberty in shuffling things around. _hurd_proc_subinit is defined as "Hook for things which should be initialized as soon as the proc server is available." If that is accurate, surely we don't need to have started signals and told the proc server our argv location for it to be "available"? Can we just run this hook sooner, namely in _hurd_init, once we set _hurd_portarray but before we do RUN_RELHOOK (_hurd_subinit, ())? _hurd_subinit is the hook init_dtable is attached to (and again, there's nothing else on it). Note: it's important to avoid calling init_pids multiple times during startup, because each call does two RPCs, and we don't want any extra ones! Now for another thing: glibc also makes these term_getctty RPCs when they are clearly pointless, such as when loading the locale archive: 11<--36(pid1403)->dir_lookup ("usr/lib/locale/locale-archive" 4194305 0) = 0 1 "" 48<--43(pid1403) 48<--43(pid1403)->term_getctty () = 0xfffffed1 ((ipc/mig) bad request message ID) We should be probably using O_IGNORE_CTTY (which makes it not do that) in more places where we expect to open a regular file, not a terminal. But how do we do that, considering O_IGNORE_CTTY is Hurd-specific? We could, for instance, do #ifndef O_IGNORE_CTTY #define O_IGNORE_CTTY 0 #endif in each .c file where we'd like to use it. Or maybe there's some internal version of fcntl.h for the Linux port where we could just #define O_IGNORE_CTTY 0 without exposing it to user code? Sergey