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 1374 invoked from network); 24 Nov 2020 03:41:33 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 24 Nov 2020 03:41:33 -0000 Received: (qmail 7919 invoked by uid 550); 24 Nov 2020 03:41:30 -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 7901 invoked from network); 24 Nov 2020 03:41:30 -0000 MIME-Version: 1.0 Date: Tue, 24 Nov 2020 06:41:18 +0300 From: Alexey Izbyshev To: musl@lists.openwall.com In-Reply-To: <20201123031932.GS534@brightrain.aerifal.cx> References: <20201122225619.GR534@brightrain.aerifal.cx> <97dd3cf7c69673e5962e9ccd46ea5131@ispras.ru> <20201123031932.GS534@brightrain.aerifal.cx> User-Agent: Roundcube Webmail/1.4.4 Message-ID: <27e3e0726b0ce24409462851b04a19b3@ispras.ru> X-Sender: izbyshev@ispras.ru Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [musl] realpath without procfs -- should be ready for inclusion On 2020-11-23 06:19, Rich Felker wrote: > On Mon, Nov 23, 2020 at 05:03:25AM +0300, Alexey Izbyshev wrote: >> On 2020-11-23 01:56, Rich Felker wrote: >> >I originally considered keeping the procfs based version and only >> >using the new one as a fallback, but I discovered there are cases >> >(involving chroot, namespaces, etc.) where the answer from procfs is >> >wrong and validating it requires basically the same procedure as >> >implementing it manually (walking and performing readlink on each path >> >component). >> > >> Pity that the simple and fast procfs-based implementation goes away. >> Do you have any specific example of a wrong answer from procfs at >> hand, or at least a more specific direction to look at than just >> "chroot/namespaces"? > > Assuming you even have procfs, the name read from readlink on procfs > will be relative to the real root of the mount namespace, not the > chroot. Probably I misunderstand what you mean, but when I tried a straightforward test on a 5.4-based kernel (with procfs-based musl, of course), I failed: $ mkdir -p chr/proc $ cat test.c #include #include int main(int argc, char *argv[]) { puts(realpath(argv[1], NULL)); } $ musl-gcc -static -std=gnu99 test.c -o chr/a.out $ unshare -Urm # mount --rbind /proc chr/proc # chroot chr ./a.out ./a.out /a.out > If the mount namespace has bind mounts over top of anything, > it can also give you a pathname that's no longer valid because > something is mounted over it. > I see how this can be the case if a bind mount was created on top of something *after* musl opened the fd in realpath(), but in presence of concurrent FS modifications the new implementation can return a stale result too. > There may be other ways this arises too. At the very least, you need > to do fstat/stat to match them up like we do now; otherwise you can > get wildly wrong results. But even if the stat matches up, it's still > possible that the resulting pathname is an absolute pathname outside > the chroot or behind the bind mount or whatever, but is also valid but > involving symlink traversal in when processed from in the current > process context. This means you return a result that does not satisfy > the contract to be symlink-free. > If a path that is not meaningful in the context of the current process can be returned by the kernel in absence of concurrent FS modifications, it is disturbing. The kernel code for open() does more-or-less the same as the new realpath() implementation, and it's not clear to me what goes wrong when the kernel walks the parent chain back to rebuild the path (and AFAIR, the kernel does remember the chain of mount points that it traversed while resolving a path, and it certainly does account for chroot). > There's also a matter I didn't mention that the current code is wrong > in an unsafe way on per-O_PATH kernels. Other places we mitigate that > by using O_NOFOLLOW and O_NOCTTY to avoid *most* of the possible > unwanted side effects if opening an actual file on a kernel that > doesn't have O_PATH, but on realpath we specifically can't use > O_NOFOLLOW, and this makes it susceptible to tricking root (or any > user with read access) into opening device nodes, in ways that might > have side effects. > Ugh, yes, pre-O_PATH kernels are a problem. Apart from what you described, you can't open a non-readable file in a readable directory without O_PATH, and you'll have trouble with Unix domain sockets as well. But at least this class of problems will go away with time. > So, there are a lot of bad things about the current implementation. > Even the minor mitigations present now for some of them (the stat > check) along with the overhead (open/close) makes it questionable > whether it's faster for lots of inputs. For deep paths the new one is > probably slower, but for typical ones it's not as clear and I didn't > measure. > Thanks for the detailed response. While I was aware about problems with procfs-based path resolution for long-lived fds (i.e. when FS tree changes after the file was opened), I'm a bit baffled that, according to your description, things can go wrong even in realpath() use case. Alexey