Re: [musl] [PATCH 1/1] ldso: continue searching if wrong architecture is found first

[Rich Felker <dalias@libc.org>]

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:

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.

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.).
   
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.)

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.

If you think there are less invasive ways to do this, I'd be happy to
hear other ideas too.

Rich