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

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Jessica Yu <jeyu@redhat.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	"Suzuki K. Poulose" <suzuki.poulose@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>, Arnd Bergmann <arnd@arndb.de>,
	Al Viro <viro@zeniv.linux.org.uk>,
	ppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v4 3/3] modversions: treat symbol CRCs as 32 bit quantities on 64 bit archs
Date: Thu, 19 Jan 2017 09:24:41 -0800	[thread overview]
Message-ID: <CA+55aFze-554PcrE8O4_ZFN5_veQ7s3M3HFOTbpj7rxVKY-KjQ@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu_5hL617uMzT37sn6-1R_9fwoRnmr_jE=zDB1XyHGvDAQ@mail.gmail.com>

On Thu, Jan 19, 2017 at 1:22 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>>
>> 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.

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

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

                Linus