Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200226201202.GB2769@voyager>
Date: Wed, 26 Feb 2020 21:12:02 +0100
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Add REL_COPY size change detection

On Wed, Feb 26, 2020 at 12:36:21PM -0500, Rich Felker wrote:
> In theory copy relocations should have been gone for everyone who
> switched to PIE, but then the gcc/binutils folks brought them back as
> an "optimization" (allowing pc-rel access to external symbols). :-(
>

Several quotes about optimization come to mind. Especially ones with the
word "premature" in them.

I just checked and I saw that there are no copy relocs in my libraries,
but my binaries are indeed PIEs. Therefore I have to conclude that gcc
compiles PIE and PIC differently. Weren't they meant to be the same,
except maybe for the selection of object files during the linker phase?

> At the very least I think we ought to catch and error on the case
> where def.sym->st_size>sym->st_size, since we can't honor it and
> failure to honor it can produce silent memory corruption. I'm less
> sure about what to do if def.sym->st_size<sym->st-size; this case
> seems safe and might be desirable not to break (I vaguely recall an
> intent that it be ok), but if you think there are reasons it's
> dangerous I'm ok with disallowing it too. I'm having a hard time now
> thinking of a reason it would really help to support that, anyway.
>

If the application did import sym->st_size bytes, it probably wanted to
do something with them. Finding less than that at the symbol and garbage
afterwards (and, from the point of view of the dynlinker, you have no
way of knowing what counts as garbage and what doesn't) might break the
program. Sometimes. That is exactly the kind of rare subtle breakage I
added the error for.

> Missing , after name, and the indent is weird (2 extra levels). I'd
> either align (spaces) with the opening paren or use just one indent
> level, and fewer lines.
>

The missing comma is weird. Must have gone missing as I was cutting that
line down to size (originally, I had this as one line, but then it was
around 200 columns long, and I thought that was a bit excessive). So
then I tried to stay below 80 and you see the result before you.

The indent was just vim's default setting. Usually this suits me, but
usually I have shiftwidth=4 and expandtab. For musl, it's sw=8 and noet.

> Semantically, st_size is size_t not ulong, so I'd probably lean
> towards a cast to (size_t) here with %zu.
>

Isn't on Linux size_t always a ulong? I will admit I only went for ulong
because it's easier to write the cast.

Ciao,
Markus

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.