From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x733.google.com (mail-qk1-x733.google.com [IPv6:2607:f8b0:4864:20::733]) by sourceware.org (Postfix) with ESMTPS id 7D612398580C for ; Wed, 9 Jun 2021 12:00:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7D612398580C Received: by mail-qk1-x733.google.com with SMTP id o27so23289273qkj.9 for ; Wed, 09 Jun 2021 05:00:16 -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=9OfRqcOEF34F6TJjP3IFTq3Nf46uTX6p1BUgIagdoGk=; b=rQVjgBrlJNJ6N4o/T9TUkeqy+8ZHmb9kfZzKMU5d0r73SWqWEBpPSgehU5D9zXJ+vy 7vwVtxvYtETDmcm9GiOcRPF+7dcJiOMrJgYbJqB+2nUlrhPBl8veHf6Vd1Sb/Dc5pZMC 1lXjsrgO4y1DcwYPhOMuv0ALjdw8B+OuQBfIOhLPLMbDgrixdlSNdBbUqv8nXVZWSH8q fkrscpdBlP9YwxFRzb16u9zgm7rYf9yJTk9Tjh1kS7jyF5ZCWat6TmNvJDymZDXPilcS H+s0e7uhklxo40Lc7iIwHF9aKV9/RrX9+rRVCPWYoBhm2k19+QHT9sfBu+OESN9pSaj9 hY8A== X-Gm-Message-State: AOAM5311i99rVbvKGQJtXlcWjJ/kygl1rI819NyYNW0zLARi3YzeDx/g SPumGsZtNKPpYRUStmRtmfILU0xKx6WQdw== X-Google-Smtp-Source: ABdhPJx+Xce7oo4JzRVT2kVyyQHfe7LdAKichJCXQ9ZZ1lVfjzhHGqwP5YZ8/E5cWkj6DKtbkf3PWg== X-Received: by 2002:a05:620a:219b:: with SMTP id g27mr11597705qka.399.1623240015876; Wed, 09 Jun 2021 05:00:15 -0700 (PDT) Received: from [192.168.1.4] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id e4sm7844567qtw.62.2021.06.09.05.00.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 09 Jun 2021 05:00:15 -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> <11145e53-3fbc-0f04-33f8-b2d9981f0ea8@linaro.org> From: Adhemerval Zanella Message-ID: <4429f842-e9c7-b907-3374-6b48c6fc089e@linaro.org> Date: Wed, 9 Jun 2021 09:00:12 -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.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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: Wed, 09 Jun 2021 12:00:19 -0000 On 08/06/2021 19:11, Godmar Back wrote: > On Tue, Jun 8, 2021 at 12:42 PM Adhemerval Zanella < > adhemerval.zanella@linaro.org> wrote: > >>> >>> >>>> >>>> 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); >>>> >>> >>> For this use case, as long as it supports groupid == 0, this should work >> as >>> it is what shells currently do. >> >> So the my question is whether providing the groupid as an argument is >> really required (I would say yes so it can be combined with >> posix_spawnattr_setpgroup). >> > > The Blackberry API does not pass an argument (as a point of reference). > In practice, when spawning a pipeline like a | b | c ... we pass group id 0 > to the > first child, and then the pid if of the first child as the pgrp id to the > second and > third. This means that if the groupid is omitted, it needs to refer to the > group id that was given to POSIX_SPAWN_SETPGROUP, and thus > POSIX_SPAWN_SETPGROUP is a prerequisite for it. > The libc interface should be as generic possible to cover most users cases, I really don't want to add a posix_spawnattr_tcsetpgrp_np that uses the groupid implicit from POSIX_SPAWN_SETPGROUP to someone ask for a posix_spawnattr_tcsetpgrp_np_ex so one can set the groupid. Also, requiring POSIX_SPAWN_SETPGROUP for posix_spawnattr_tcsetpgrp_np is not really a good API, it adds subtle semantics (should it use a default value set by posix_spawnattr_init or should we fail with EINVAL), and adds complexity in the error path (we will need to either pre validate the posix_spawnattr_t input before start the process creation or handle a possible invalid combination on the helper process itself). That's why I am more inclined to follow the tcsetpgrp on the posix_spawn extension and let the caller set the required groups. > >> >> Another question is when to issue the tcsetpgrp related to >> POSIX_SPAWN_SETSID. I would say tcsetpgrp should be issued *before* >> setsid, so tcsetpgrp can return early if it fails. Otherwise tcsetpgrp >> will always fail if POSIX_SPAWN_SETSID is set (it would be a caller >> error, but I think from API viewpoint it should be better if we could >> minimize the possible error scenarios). >> > > I haven't checked how the current implementation works. Do you use a pipe > or something > for the child to report back if something went wrong, or does the parent > somehow check if the > asked-for actions in the child will likely succeed? If the former, doing > setsid first, and then, > if given, doing the tcsetpgrp call will fail as you say and this failure > would be reported back > to the caller. Then they know they've misused the API. > > If you do tcsetpgrp first and then setsid, the tcsetpgrp will succeed, but > will be ineffective, and > the caller won't ever know. > As Florian has said we use a helper process created with CLONE_VM and CLONE_VFORK, meaning that it will share the memory with its parent and CLONE_VFORK avoids any potential race condition. And you raised a good point, I agree we should not hide the possible misuse from the caller.