From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 25133 invoked from network); 12 Oct 2020 20:31:17 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 12 Oct 2020 20:31:17 -0000 Received: (qmail 1824 invoked by uid 550); 12 Oct 2020 20:31:10 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 1782 invoked from network); 12 Oct 2020 20:31:09 -0000 MIME-Version: 1.0 Date: Mon, 12 Oct 2020 23:30:58 +0300 From: Alexey Izbyshev To: musl@lists.openwall.com In-Reply-To: <20201012145549.GG17637@brightrain.aerifal.cx> References: <93cbaeffbc860a145843e0380058c50e@ispras.ru> <20201012145549.GG17637@brightrain.aerifal.cx> User-Agent: Roundcube Webmail/1.4.4 Message-ID: X-Sender: izbyshev@ispras.ru Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [musl] Calling setxid() in a vfork()-child Thank you for the response, Rich! On 2020-10-12 17:55, Rich Felker wrote: > Note that in addition to the issue you're asking about, it's > fundamentally a bad idea to be using set*id() in a vforked child (or > anywhere in a process that calls vfork) because it leaves moments > where there are tasks in different privilege domains executing from > the same VM space. If the task that's dropped privileges does anything > that could lead to an attacker seizing control of the flow of > execution, rather than just getting access to the set*id()-reduced > privilege domain, they have full access to the original privilege > domain. This is why musl's multithreaded set*id() (__synccall) takes > care not to admit forward progress of any application code during the > transition, and goes to the trouble of having a thread list lock that > unlocks atomically with kernel task exit so that there is no race > window where a still-live thread can be missed. > Yes, I initially learned about this security issue from your post[1]. I proposed to ignore it for the moment in my email because it was hard for me to imagine any security implications in a constrained case: if the only use of setuid() in a privileged app is to drop privileges in a vfork()-child before calling execve(). However, thinking about it more, I see that dropping privileges could open the child to new ways of interaction from outside of the app in a window before execve(), so, if, say, another unprivileged process can ptrace it at the right moment, bad things could happen. But now I don't see why wouldn't the same attack be applicable to __synccall(). While __synccall() seizes execution of application code, I don't see how it could prevent ptrace-like kind of an outside attack on a thread at the moment when another thread is in a different privilege domain. > In any case, IMO unless you're programming for NOMMU compatibility, > you should just forget vfork ever existed. There's no good reason to > use it. If a process can't fork because it's too big or the fork would > impact performance too much, posix_spawn can do far more than > vfork+execve can do portably. It can't do everything you can do with > vfork+execve if you're willing to break portability rules (i.e. invoke > UB), but with a helper executable to run in the child you can get that > all back. > Unfortunately, while posix_spawn() + a helper executable would be fine in many cases (more so for applications than libraries), addition of a helper to an existing library exposing a process creation API that can't be implemented in terms of posix_spawn() may be not straightforward. If the helper is just an executable file, we'll need to locate it, which may be simply impossible when our library is called if, say, switching mount namespaces is involved (which may be out of control of our library). Solutions may exist (locate and open() at startup or memfd_create(), and then execveat()?), but vfork() (+ preventing execution of signal handlers in the child) seems so much simpler, and doesn't add the overhead of an extra execve() too. Alexey [1] https://ewontfix.com/7