Re: [PATCH v4 3/3] modversions: treat symbol CRCs as 32 bit quantities on 64 bit archs

On 19 January 2017 at 17:24, Linus Torvalds
[off-list ref] wrote:

On Thu, Jan 19, 2017 at 1:22 AM, Ard Biesheuvel
[off-list ref] wrote:

quoted

quoted

Your genksyms.c change is not exactly obvious. I looked at it, and my
brain just shut down. Why both the

  LONG(0x%08lx);

_and_ the

  "%s__crc_%s = 0x%08lx;\n"

in the linker script? I'm sure there's a good reason, but I'd like to
see a more explicit explanation fo what the generated linker script
does and what the rules are.

This is simply because modpost still uses the value of the symbol
rather than the value it points to to generate the /other/ side of the
comparison (i.e., Module.symvers etc)

Ahh, now that you explained it, it was obvious. Thanks.

But yes, I don't think we want that "both belt and suspenders"
approach, so your updated patch that does things just one way is I
think the right way.

quoted

I will look into updating modpost to dereference the symbol as well,
and update the RFC patch accordingly.

Yes, so your updated patch looks good to me.

I think our old "symbol with an absolute value" model was simpler
conceptually, but given the existing absolute (sic) braindamage of
linkers, I think your latest patch is probably the way to go.

OK.

If for no other reason than the fact that it doesn't depend on
something that clearly nobody else uses, and even the linker people
were confused about.

So I think the slightly more complex model of relative offsets is the
simpler one in the end if it means that we don't have to have
completely insane workarounds for linker damage.

But maybe somebody else wants to pipe up. Preferably somebody who
doesn't hate the symversions code as much as I do by now, and actually
_uses_ it ;)

I am not crazy about it either: I am simply trying to get rid of the
~10,000 pointless relocations in the arm64 KASLR kernel rather than
having to rely on the dodgy code that 'repairs' the CRCs at runtime.

I have noticed one slight snag though: the ARM module loader currently
has no support for the R_ARM_REL32 relocations that are emitted by the
relative references in the kcrctabs. I looked at other arches, x86,
ia64, s390, power and arm64, and those all seem to be fully equipped
in this regard, but it would be good if we could get some coverage for
this code on other architectures to find out which ones need to have
this support added as well.

Thanks,
Ard.