From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x731.google.com (mail-qk1-x731.google.com [IPv6:2607:f8b0:4864:20::731]) by sourceware.org (Postfix) with ESMTPS id EE1D9395540E for ; Tue, 8 Jun 2021 13:50:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EE1D9395540E Received: by mail-qk1-x731.google.com with SMTP id i67so20183338qkc.4 for ; Tue, 08 Jun 2021 06:50:46 -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=wowbm6uR1PKvA9luqAkdqdtSU9L+o5sjXoD9DWNO5p8=; b=MgSva5jxHWLweZDHpkRvMNrwIR1mwzPeMi9ofFJRqZEJviAZaTUG3MLtmMM4HPYghA pPlDXSBUFRerp3zm/ILXYX9L9xFrAjXqfe/F9vnlvVZ3G7vDyDW832+/mYhNvCKRbxhW I29E5GEk+UTFuoh0ksWp+X1VVfCRGTIkdIjZtbf40zs4sRA+DA2aN/YaCtq5bBWa2I2T BMvYCRiLVS8pHM8uY2Uw8CFR9WW042ZLJh+KVNPzQnEhuSiGtOd3sOYmXxxLlBsGTkVs GcnUlqaH+zaQUTX+ypBZUzJnxUGmpygykegAXn2VGzoKy83TN0U5zDcbd3YCViKvGJ8/ zDyg== X-Gm-Message-State: AOAM5326hchXI7ONJlNVUTGXcFuipwZx2lSjsPYP3lKmeNbHsSIUk6zz AKBRvbAXuP4lv2oNFAWxxNoQHyx30gu7ww== X-Google-Smtp-Source: ABdhPJzm+n1EdcTSJrVvJY2AahJhtglohdNiJSFVpFXdTo6GEziQcTS7PM/hYE0Ukpt2t2PkNmMYjA== X-Received: by 2002:a37:b17:: with SMTP id 23mr21729293qkl.60.1623160246312; Tue, 08 Jun 2021 06:50:46 -0700 (PDT) Received: from [192.168.1.4] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id q190sm11530613qkf.133.2021.06.08.06.50.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 08 Jun 2021 06:50:45 -0700 (PDT) Subject: Re: supporting terminal ownership assignment (tcsetpgrp()) in posix_spawn To: Godmar Back Cc: Libc-help References: <7298cb72-becb-80bb-b2df-d97bdb201e95@linaro.org> From: Adhemerval Zanella Message-ID: Date: Tue, 8 Jun 2021 10:50:43 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, 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-help@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-help mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Jun 2021 13:50:51 -0000 On 07/06/2021 20:57, Godmar Back wrote: > On Mon, Jun 7, 2021 at 5:36 PM Adhemerval Zanella < > adhemerval.zanella@linaro.org> wrote: > >> >> Yeah I forgot the inherent race condition on this scheme. I think it >> should be possible to add such interface as a posix_spawn file action: >> >> int posix_spawn_file_actions_tcsetpgrp_np (int fd) >> >> Not sure if it make sense to support a pid different than the created >> helper process (which will eventually call execve) since it is something >> the parent can do it itself. >> >> > The Blackberry implementation [1] has a single flag, with no arguments. It > doesn't pass a file descriptor. It appears undocumented, but I would guess > that this means that the child's process group becomes the foreground > process group of the controlling terminal, which in a kernel implementation > may be directly accessible. In a library implementation, it would require > obtaining a file descriptor, perhaps via `ctermid`, which would introduce > another error situation that would need to be handled. > > I think either approach (flag or file action) would be fine; in the > envisioned use case, it's likely that the caller already has an open file > descriptor available that refers to the intended terminal. > > Incidentally, I also sent an email to the mailing list of the Austin group, > though haven't received any replies. > > - Godmar > > [1] > https://developer.blackberry.com/native/reference/core/com.qnx.doc.neutrino.lib_ref/topic/p/posix_spawnattr_setxflags.html > It seems that another shell implementation have stumbled over the same issue [1]. Setting the terminal ownership does seem to be a good API extension because some developers are leaning of using vfork(), which is another really bad API (I have improved glibc posix_spawn() exactly to give developers a better options than using vfork+exec). I think the only issue is which would be better API to provide such extension. The QNX API you referred in indeed is really scarce regarding documentation [2], it only states that: | If you want to: Do the following: | Start a new terminal group Set POSIX_SPAWN_TCSETPGROUP It does not really specify which process group will be used to tcsetpgrp call, neither how it interacts with POSIX_SPAWN_SETPGROUP or POSIX_SPAWN_SETSID (which order the flags commands are execute and if they interfere with each other). So one options might be to add something as the one I suggested before, but as generic extension instead of a file extension (which does not make sense in fact): int posix_spawnattr_tcsetpgrp_np (posix_spawnattr_t *__attr, int fd, pid_t pgrp); Similar to tcgetpgrp, it make the created process group with process group instructed by the PGRP argument the foreground process group on the terminal associated to FD. If PGRP is 0, the current group obtained with getpgrp() will be used, otherwise PGRP will be used. This is done after just after setting the process group ID (POSIX_SPAWN_SETPGROUP) and right before setting the effective user and group id (POSIX_SPAWN_RESETIDS). So if the called want to create a new session id, it can issue: posix_spawnattr_t attr; posix_spawnattr_setflags (&attr, POSIX_SPAWN_SETSID); posix_spawnattr_tcsetpgrp_np (&attr, fd, 0); And the created process will issue the following in the order: setsid (); tcsetpgrp (fd, getpgid (0)); If the caller already has group it want to use, it can issue instead: posix_spawnattr_t attr; posix_spawnattr_setpgroup (&attr, groupid); posix_spawnattr_tcsetpgrp_np (&attr, fd, groupid); Which in turn will make the created process to issue: setpgid (0, groupid); tcsetpgrp (fd, groupid); What do you think? Does it help your shell implementation and avoid the potential race condition? [1] https://github.com/ksh93/ksh/issues/79 [2] http://www.qnx.com/developers/docs/7.0.0/index.html#com.qnx.doc.neutrino.lib_ref/topic/p/posix_spawn.html