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.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,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 24246 invoked from network); 18 Nov 2021 19:21:30 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 18 Nov 2021 19:21:30 -0000 Received: (qmail 32476 invoked by uid 550); 18 Nov 2021 19:21:27 -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 32444 invoked from network); 18 Nov 2021 19:21:26 -0000 X-Virus-Scanned: Debian amavisd-new at disroot.org Mime-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1637263272; bh=oxTv2D9YNKLK5f6j9VvZszXShKmG90SY+DexqgX7U+k=; h=Cc:Subject:From:To:Date:In-Reply-To; b=Mqo/vzICuExLhxeb88Unnjs6ZMXYLGjYxVJD8zOokZOloJqfQcKIyLTHHFUdf6Si5 HjAlLK85O4LJPKfcWD0d2wAOEbMkhhrULV2xZpUP4Pa7VyUvMPcELBqoEWZGz+FFjr 4Bhy2fWT5SRqIepQ9K3RcpM6m0nuLI7gdvTqJ/HSWTbBat1E/Rarx4rt5WNkOKsjzq cMxqBtZvyks0h/+VUkAgG9w8wg1X8r8QNuhZDJsfTBfDxXzHQZ1d4/041TIgywLkkp gzaQEV3tKBxigGmVzAV9OJHLyMr1mLRHoAyR/bcSCML0ymrZaYu99nONZ2+UWbgSL0 KHWzAPhsKVRGQ== Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Cc: From: =?utf-8?q?=C3=89rico_Nogueira?= To: "Alexander Sosedkin" , Date: Thu, 18 Nov 2021 16:15:52 -0300 Message-Id: In-Reply-To: <256fa366-6e6b-8248-3176-1736c8db55a4@unboiled.info> Subject: Re: [musl] $ORIGIN rpath expansion without /proc: code looks wrong On Wed Nov 17, 2021 at 4:25 PM -03, Alexander Sosedkin wrote: > On 11/17/21 18:00, =C3=89rico Nogueira wrote: > > On Wed Nov 17, 2021 at 11:04 AM -03, Alexander Sosedkin wrote: > >> Hello, I've encountered a case of a binary with an rpath of > >> /some/meaningful/lib:$ORIGIN/../lib > >> not starting up due to not finding /some/meaningful/lib/libxyz.so. > >> ldd'ing said it's there though. > >> And the library was found alright when I patchelf'd rpath to just > >> /some/meaningful/lib > >> > >> I dug into musl code and came across that bit that checks /proc. > >> Sure enough, when I tried mounting /proc, it started working fine. > >> Yet the error handling from accessing /proc puzzles me: > >> > >> ldso/dynlink.c, fixup_rpath(): > >> l =3D readlink("/proc/self/exe", buf, buf_size); > >> if (l =3D=3D -1) switch (errno) { > >> case ENOENT: > >> case ENOTDIR: > >> case EACCES: > >> break; > >> default: > >> return -1; > >> } > >> if (l >=3D buf_size) > >> return 0; > >> buf[l] =3D 0; > >> origin =3D buf; > >> > >> hitting that break like I had means zeroing buf[-1], right? > >=20 > > No. Because `l` is size_t (unsigned long), it's the biggest possible > > value for size_t, and `l >=3D buf_size` will be true, > > Oh! Thanks a lot, that's what confused me. Sorry for the noise then. > > > which means the > > function returns 0. This conditional also catches the case where > > truncation happens in readlink(3). > > Documenting this in a comment or changing `break;` for `return 0;` migh= t > > make sense, though. > > Yeah, I'd say a `return 0;` there would've been easier to comprehend. > I don't think there's much need for a comment... That sounds reasonable to me. I can make a patch for it. > > >> Could somebody take a look at this and double-check that > >> this codepath makes sense? > >=20 > > It does, but it might not be as robust as you wish. fixup_rpath() treat= s > > the RPATH entry as a single string, and does all $ORIGIN substitutions > > in one go (what splits the string by ":" is open_path()). This means > > that the entire RPATH entry containing $ORIGIN will be ignored if > > /proc/self/exe can't be accessed, despite one or more of them not > > depending on $ORIGIN. > > ... because if running /proc-less isn't supported in general, > then such separate expansions are probably not worth the effort. > And it's far from being the only mention of /proc in the code, > so I'm going to presume it's not. https://musl.libc.org/doc/1.1.24/manual.html does document the requirement that /proc exist, and at a an arbitrary guess, complex binaries which rely on RPATH are unlikely to run before the boot process has mounted /proc. I can think of some ways to try and change the order the operations are done in, but in the end, it's probably not worth it to make changes to this part of the code. > > >> My attempts at comprehending it fail irrecoverably at this line. > >> > >> (CC me on replies, please. > >> No nice context to provide, building my own toolchain at > >> https://github.com/t184256/bootstrap-from-tcc) > > Thanks for the answer, appreciated. Glad to help!