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=-2.9 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, T_SCC_BODY_TEXT_LINE 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 B97AB2C618 for ; Wed, 7 Feb 2024 22:18:10 +0100 (CET) Received: (qmail 21533 invoked by uid 550); 7 Feb 2024 21:15:23 -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 1917 invoked from network); 7 Feb 2024 20:57:15 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; t=1707339589; x=1707944389; darn=lists.openwall.com; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=kcWkuD6oZFGZB5ax9Kw6ral04t1waW7xgdM77VvzksU=; b=AuWv/Rs+gc/mMZ1F272VosAwRxD5tECAUhn2gF/FzXWiV5aYSQL+b8lxiIPCLwa1a9 I5/FrcjmtDBXGkUVUWJO9LMNz9ymb+reAJc2Nqjs2KJmsaDkGmrutSR5eORVl9+AsJez 51pg+V203mW2+BJ0MKPkSuBY9almg2rEbe6Iw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707339589; x=1707944389; h=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=kcWkuD6oZFGZB5ax9Kw6ral04t1waW7xgdM77VvzksU=; b=p6ZdDwjPun3ESv4T7Vx0y+6iXHJFio6LD02fqo2GLmN2XSiygGYVclxG7SaQ8avDO7 0Cotux1MGwZMb1reLyZV/bWzzWAjMyDAYyARObys6rNrrk+MbUSSQsLYC/7VfL12Fw2a GNO79OKGosfiZf8UPW0OFfVDfqGgY909tybIxYWNP5/ZltHgiWFhA/10aHQhuNlz8Mio gc/XhAsfJGg0FudXZQiMsZFQV62Ctq7XLJel75Edgmxo5PGMkfvZf/5RLyqZ8oR2CBnB OppUN+ctNKQOzYLYha3aDumA7uTRk9JZ7DD8zASGnbwyk8rwK+U1vvqmb0/u4Ka6o/6h KTbw== X-Gm-Message-State: AOJu0YxnoSvXmaE/sf7nYAkR5b+XsNWSEUY9+KqA7/PPqD785LozU2ER yA7RVXdYQs7GaLcg0+zetVyB1u94SzZw4af//yKMZ3QaNCKjYmjipzsCDiVm/gs4efdK+mbgVuX Nc6FsXV7tTWLqVSPWPpzjZ342A16eBi+FTaKzTOY/XZMj7nYCtSzJ X-Google-Smtp-Source: AGHT+IFN1PxpudokzaR/X1uueVKBgaQMv8VEIOVd/cRhhScLqovz9o16P8N2KXl/tfv4QXsrt2a4LediTtq2KWhCxBI= X-Received: by 2002:a17:90b:4b52:b0:296:94e0:71db with SMTP id mi18-20020a17090b4b5200b0029694e071dbmr7299233pjb.1.1707339588408; Wed, 07 Feb 2024 12:59:48 -0800 (PST) MIME-Version: 1.0 References: <20240207012247.1121273-1-mmayer@broadcom.com> <20240207012247.1121273-2-mmayer@broadcom.com> <20240207173023.GX4163@brightrain.aerifal.cx> In-Reply-To: <20240207173023.GX4163@brightrain.aerifal.cx> From: Markus Mayer Date: Wed, 7 Feb 2024 12:59:37 -0800 Message-ID: To: Musl Mailing List Cc: Rich Felker Content-Type: text/plain; charset="UTF-8" Subject: Re: [musl] [PATCH 1/1] ldso: continue searching if wrong architecture is found first On Wed, 7 Feb 2024 at 09:30, Rich Felker wrote: > > On Tue, Feb 06, 2024 at 05:22:43PM -0800, Markus Mayer wrote: > > When LD_LIBRARY_PATH is being used, the shared library loader may end up > > finding a shared library with the correct name but from a wrong > > architecture. This primarily happens when a 32-bit / 64-bit mismatch > > occurs. > > > > Rather than giving up immediately and aborting, the shared library > > loader should continue to look in all known locations for a matching > > library that may work. > > I don't think the concept is objectionable, but the implementation > looks wrong in at least two major ways: I had a feeling that my approach might be too simplistic. At least it served as a proof-of-concept. > 1. It does not seem to continue the path search where it left off, but > just skips the rest of the LD_LIBRARY_PATH search on retry. Yes > this solves your problem of having a wrong-arch path element > present, but it breaks search of any correct-arch path elements > also present later there. This might not be a big deal, but it > strongly violates principle of least surprise, I think. You are correct. The implementation does not deal with the remainder of LD_LIBRARY_PATH and just searches system directories after it finds an issue with a library in LD_LIBRARY_PATH. I can see how that could be problematic in other scenarios. > The bigger one is: > > 2. No consideration is made for the *cause of failure* when retrying. > Any in particular, lots of potential errors that were supposed to > be reportable errors now turn into vectors for loading the wrong > library, potentially under control of malicious inputs or > environmental factors (kernel resource exhaustion, etc.). Okay, That makes a lot of sense. We definitely wouldn't want to introduce potential instabilities or even create attack vectors. > If "wrong library arch" is going to be a continue-path-search > operation, it needs to be distinguishable (as "we successfully read > the Ehdrs and determined the library is not suitable for this arch") > from inconclusive errors (like out-of-memory, too many mmaps, etc.) I did some more digging and found where and how it is failing in my 32-bit / 64-bit scenario. I am getting this output when I sprinkle some debug messages throughout the code. map_library, 647: fd=3, phsize=324108288, l=960 map_library, 652: fd=3, phsize=324108288, l=0 map_library, 816: errno=8 Of course the line numbers are off between the output and the actual sources. 640 ssize_t l = read(fd, buf, sizeof buf); 641 eh = buf; 642 if (l<0) return 0; 643 if (le_type != ET_DYN && eh->e_type != ET_EXEC)) 644 goto noexec; 645 phsize = eh->e_phentsize * eh->e_phnum; > "Line 647" output 646 if (phsize > sizeof buf - sizeof *eh) { 647 allocated_buf = malloc(phsize); 648 if (!allocated_buf) return 0; 649 l = pread(fd, allocated_buf, phsize, eh->e_phoff); > "Line 652" output 650 if (l < 0) goto error; 651 if (l != phsize) goto noexec; 652 ph = ph0 = allocated_buf; 653 } else if (eh->e_phoff + phsize > l) { ... 803 error: > "Line 816" output 804 if (map!=MAP_FAILED) unmap_library(dso); 805 free(allocated_buf); 806 return 0; 807 } So it is finding out only rather late in the process that mapping the library didn't work. By this point, it isn't easy to go back to searching. Conversely, it knows right away when open() fails, so it can easily keep looking. > This probably means some minor refactoring is called for, replacing > the open calls in path_open and the direct use of open for absolute > paths by a new function like lib_open or something that would also > place the start of the file into a caller-provided buffer and validate > that it's usable before returning success, moving that logic out of > map_library and letting map_library assume it already has the initial > buffer contents available on entry. I'll mull this one over a bit. I guess we'd want a way to treat "exec format error" in the same way as open() failing, i.e. as "we didn't find anything useful, keep on searching". So, we need to know sooner whether or not mapping will work or not. If I am understanding your proposal correctly, that is exactly what you are getting at, as well. > If you think there are less invasive ways to do this, I'd be happy to > hear other ideas too. We'll see. :-) Regards, -Markus