From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.5 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 37C6824F1C for ; Sun, 28 Apr 2024 04:30:02 +0200 (CEST) Received: (qmail 1775 invoked by uid 550); 28 Apr 2024 02:29:55 -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 1722 invoked from network); 28 Apr 2024 02:29:54 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1714271386; x=1714876186; darn=lists.openwall.com; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=9O7HguT1bO1pQbv+/eJE4mXISxfqpLiEBXIMhkdf13g=; b=a3XfqFHA5F2BOFSt4b5QLRJTOgQXzDTpDgu9q70NhbjK0injj4/2Lb6h3zZduZWEyZ kt25S8LW3Mzch/Xqsfbwj+2rVqPAI6zHiGni1qZ1z08XO3llamDFzxpp3ax+at2+VEID CscvkPZqJdrEozltppIgJU16ZexZ9TXFsS49jhDAdye8wudD/WahLEiFdUz7jFjyuNsY 36wMMqcL6ThJSrPxGxfDklESAhRGYjRjfQx4eYDHlm2rJLQb7rffaWHF16prhW8jxBhF 4xnFGYP49KVL6vCCnbzJC3cunIGss9lgiEkw5Gc+F/c2ODw7gCKWwtTk1TewvCHesXuu OwEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714271386; x=1714876186; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9O7HguT1bO1pQbv+/eJE4mXISxfqpLiEBXIMhkdf13g=; b=qeoLZZTSjg8JJpoVYsvOGDeBnY/guDhD33d0n6A3B0nc+l3LXZGnM7uUo4XPp7yYJU rKFxyjVQPtN4Va1hdj5X7bMEcaZBN53JnY3WrpNIYkHIxDUZpG9xWyQxn1UX+N9vVuKT 1O4X9d91860e+bQJnu/BhhAgdGqfnPXN5doSL7PUzFZszFDGvqxbSR7AkJzEhvXeH8j4 7tlwVRvEkvHdg3y3BdtReFbpMkeED2LuZKljgXJzi+Of3uqwI584tUbyTUTcOSOfXJtE BOuVtgblw8TT2h//mGCHzc4EBgFF0rcmOvY+9ooH6kf/D0wJKoe7beEut1SWJ16F9GKg jQiw== X-Gm-Message-State: AOJu0YwRkUE3t58vDy5vra1RddEvFem47DqjMwNtBd/8hu5Qe+oVBKbk /lT7p4+4XqEExaRTPt9dFbCbEZ+8HCgFJN3XSTz0WdTLj41kYSjd1wK2VyyMOk89MRd0IVUaeAZ Iolti0tAzJpg6yAJ2h3bSZ1adZvo= X-Google-Smtp-Source: AGHT+IHaCq6vk+igdyE+Y5FXTfyFfaoghimnJ5BvddnQJhTvkubAcfETGxti/CiOH7+MA6S1AjiUXQGxX2qYJ1sTz6M= X-Received: by 2002:a05:6870:9347:b0:233:a2c:470e with SMTP id j7-20020a056870934700b002330a2c470emr7772389oak.57.1714271386296; Sat, 27 Apr 2024 19:29:46 -0700 (PDT) MIME-Version: 1.0 References: <20200119121247.37310-1-info@bnoordhuis.nl> <20220831190735.52016-1-dunk@denkimushi.com> <20220831190735.52016-2-dunk@denkimushi.com> <20240224165632.GA4163@brightrain.aerifal.cx> <20240424193038.GP4163@brightrain.aerifal.cx> <20240425122548.GQ4163@brightrain.aerifal.cx> In-Reply-To: <20240425122548.GQ4163@brightrain.aerifal.cx> From: lolzery wowzery Date: Sat, 27 Apr 2024 22:29:35 -0400 Message-ID: To: Rich Felker Cc: musl@lists.openwall.com, Duncan Bellamy , info@bnoordhuis.nl, tony.ambardar@gmail.co Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [musl] [PATCH 1/2] V3 resubmitting old statx patch with changes Hi, Update: you've given me a lot to think about in my suggestions and proposals to musl and, because you took time to respond to me and explain things clearly, I feel much more compelled to put good effort into making sure my changes are solid and indisputably beneficial (not as in more=3Dbetter kind of way but less=3Dmore but keeping in mind your responses about musl's design.) > statx is already in musl 1.2.5 (commit b817541f1cfd). If there are > other problems than the ones I fixed when merging it and the > stx_attributes field, please report them here rather than spending > time trying to make a comprehensive changeset for a lot of things that > might or might not actually be wrong. I must have dyslexia or something because it turned out my efforts were a wild goose chase for xstat, not statx! I'm so embarrassed, ha ha. > > symbol > > weakness problems, > > This sounds unlikely. It's more likely that you misunderstand how/why > they're used. But I'm happy to look at your findings. There are some symbol weakness inconsistencies with glibc I found in musl but trying to track them all down by hand would be insane. I will get toget= her a tool to difference musl's and glibc symbols and list all changes to you o= ne of these days. For reference, symbol weakness only affects what happens when linking two libraries with same symbol names, which is used to override libc methods for various necessary purposes. Glibc is the golden standard software is built to link to, so, if anything, this will only help some software work in musl. > Can you clarify what you mean? There are some places where correctly > atomic fallback is impossible and the fallback is best-effort only. > This is generally only for missing O_CLOEXEC type functionality. If > there are others, please report. My original thinking was that my proposed solution of trying both the full syscall and the fallback and seeing which work to handle seccomp would introduce a race condition where the file is created between the two calls. That thought is invalid now that I know we are not in the business of supporting bad seccomp configurations. > It's expected to fail catastrophically if seccomp filters modify > syscall behavior in ways that make forward progress impossible (e.g. > blocking close or something) or that lie about the cause of failure > (the classic but badly wrong practice of using EPERM where ENOSYS was > the correct code for syscalls not available in the sandbox). This is > inherently not a supportable configuration. > > Generally musl always uses the oldest-possible syscall that can fully > meet the requirements. If it has to try a new syscall first (like > statx on 32-bit, since the legacy syscalls don't support full-range > time_t and we don't know until after stat whether the times fit in 32 > bits), it falls back on ENOSYS. So as long as seccomp filters are > using semantically correct error codes, things should generally work. > But it's the responsibility of the seccomp filter authors to ensure > they're doing this right. Got it and thank you for the explanation. I now understand. > It's never undefined. That's not how this works. > > If you're making out-of-tree bare-metal ports and don't want the > overhead of having to add new syscalls like this, you can do something > like define the SYS_* macros for them such that syscall_arch.h can > statically catch that they're nonexistant (e.g. by given them values > in some high range) and directly return -ENOSYS; then the code would > collapse down as you want with the ENOSYS path being always-taken. > > Note that there is no way to emulate the nonzero flags for renameat2 > without race conditions. Since this is not standard/mandatory > functionality, the right thing to do is just return an error for > "unsupported flags", not try to emulate them. Got it and thanks for explaining. Now that I've fixed my eyes and am looking at statx, I see four main things to be fixed: 1. Remove the `#ifndef SYS_fstatat` because it's nonsensible 2. Add comments to explain things 3. Correctly validate the flags and EINVAL if unsupported by fallback 4. Zero all extraneous fields like __pad1 for future proofing. 5. The stx_rdev_major and stx_rdev_minor fields were not correctly filled i= n Please do not make these 5 changes yourself yet as I might find more and I have some great comments I want to add to explain why things are. I also discovered and would like to do a very minor cleanup on src/stat/fstatat.c, which has a duplicate copy of the struct statx for no reason. Additionally, I am working on a proper fallback and implementation for fsta= tx and lstatx, which are the open-fd and symlink equivalents of statx. It will properly use ioctl_iflags to polyfill the attributes field for fstatx. This cannot be done for lstatx, statx, or statatx as they accept a file name and no equivalent to ioctl_iflags exists which accepts a file name, so it requires an open file.= RTM: > These functions return information about a file, in the buffer > pointed to by statbuf. No permissions are required on the file > itself, but=E2=80=94in the case of stat(), fstatat(), and lstat()=E2=80= =94execute > (search) permission is required on all of the directories in > pathname that lead to the file. I swear I've spent the last 5 hours digging into the nitty gritty depths of these little-documented methods to ensure musl will have proper fallbacks. QUESTION: The glibc wrapper for statx explicitly sets the errno to ENOSYS upon successful execution of its fallback to fstat64 (yes you read that rig= ht.) I swore I misread something or this was a bug until quadruple checking that this is works-as-intended over the next 2 hours. Will check around glibc more tomorrow but what should musl's policy be about this (and perhaps other works-as-intended POSIX violations around glibc?) I'm personally strongly leaning towards do-what-glibc-does for compatibility > OK, please send. Working hard to keep my changes small, localized, and important. Jack On Thu, Apr 25, 2024 at 8:25=E2=80=AFAM Rich Felker wrote= : > > On Wed, Apr 24, 2024 at 07:55:32PM -0400, lolzery wowzery wrote: > > Hi all! I'm new to git over email, new to musl, and I want to really > > get into contributing to musl. So, I'd really appreciate any > > tips/comments/info for a newbie like me! > > > > Please do not merge these changes yet. There's numerous problems with > > this code above and beyond the issues you pointed out, > > statx is already in musl 1.2.5 (commit b817541f1cfd). If there are > other problems than the ones I fixed when merging it and the > stx_attributes field, please report them here rather than spending > time trying to make a comprehensive changeset for a lot of things that > might or might not actually be wrong. > > > and each of > > those issues led me down successive rabbit holes all over the musl > > source code and I've ended up opening a sizable can of worms, > > including io race condition bugs in fallback conditions, > > Can you clarify what you mean? There are some places where correctly > atomic fallback is impossible and the fallback is best-effort only. > This is generally only for missing O_CLOEXEC type functionality. If > there are others, please report. > > > symbol > > weakness problems, > > This sounds unlikely. It's more likely that you misunderstand how/why > they're used. But I'm happy to look at your findings. > > > header/struct misorganization, and bugs in syscall > > support detection making musl silently/unexpectedly fail under certain > > seccomp configurations. > > It's expected to fail catastrophically if seccomp filters modify > syscall behavior in ways that make forward progress impossible (e.g. > blocking close or something) or that lie about the cause of failure > (the classic but badly wrong practice of using EPERM where ENOSYS was > the correct code for syscalls not available in the sandbox). This is > inherently not a supportable configuration. > > Generally musl always uses the oldest-possible syscall that can fully > meet the requirements. If it has to try a new syscall first (like > statx on 32-bit, since the legacy syscalls don't support full-range > time_t and we don't know until after stat whether the times fit in 32 > bits), it falls back on ENOSYS. So as long as seccomp filters are > using semantically correct error codes, things should generally work. > But it's the responsibility of the seccomp filter authors to ensure > they're doing this right. > > > Also, let me cite the earlier patch from tony.ambardar@gmail.com about > > renameat2. My patch will include a full proper renameat2 > > implementation with validation and fallback and will compile on > > systems where SYS_renameat2 is undefined (using exclusively the > > fallback.) > > It's never undefined. That's not how this works. > > If you're making out-of-tree bare-metal ports and don't want the > overhead of having to add new syscalls like this, you can do something > like define the SYS_* macros for them such that syscall_arch.h can > statically catch that they're nonexistant (e.g. by given them values > in some high range) and directly return -ENOSYS; then the code would > collapse down as you want with the ENOSYS path being always-taken. > > Note that there is no way to emulate the nonzero flags for renameat2 > without race conditions. Since this is not standard/mandatory > functionality, the right thing to do is just return an error for > "unsupported flags", not try to emulate them. > > > I recognize I probably sound like an over-eager-beaver fussing over > > fools-gold as I'm new and none of you know me. (That's the assumption > > I myself would make if a new guy showed up claiming to have lots of > > bug fixes and stuff.) So, please suspend your disbelief for a short > > while until I can give my full report and all fixes tomorrow for > > everyone to review. Give me a chance to prove my commitment to musl > > and watch as I uphold my pledge to maintain my areas of the code in > > musl and keep them ship-shape. > > > > Have a great day everyone and really looking forwards to sharing my > > full report tomorrow!, > > OK, please send. > > Rich