Question about pointer equality

[iii-i]

I've been looking into kpatch on s390x in context of a compiler issue and came up with the following experiment: iii-i/linux@0f400db.

One part of it is a kernel function that returns its own address:

void *patchme(void) { return patchme; }

and we replace it using kpatch:

void *patchme(void) { printk(KERN_NOTICE "patched\n"); return patchme; }

Another part is a sysfs knob to compare patchme with patchme():

	printk(KERN_NOTICE "patchme=0x%lx patchme()=0x%lx\n",
	       (unsigned long)patchme, (unsigned long)patchme());

The output before patching is as expected, the values match:

patchme=0x3ffe0962f78 patchme()=0x3ffe0962f78

After patching the values don't match anymore:

patched
patchme=0x3ffe0962f78 patchme()=0x3ff600a16c8

One may argue, and most likely be correct, that this is an artificially created scenario, which will probably not affect a real workload.

But still I would like to ask: is this an acceptable behavior?
If yes, would it be reasonable to add this to the list of limitations / things to watch out for?
If no, what would be a good way to deal with this? The only thing I can come up with now is to perform all address loads via GOT instead of using relative addressing, but this will probably slow the kernel down quite a bit.

Edit: adding @Andreas-Krebbel and @sumanthkorikkar to the discussion.

[rui314]

One thing to note is that the pointer equality is guaranteed by the C language standard, so it is technically clearly a violation of the language spec (i.e. this may break valid code.) In addition to that, we found actual use cases of this guarantee in several user-land programs, as they break if pointer equality is not upheld. We learned this the hard way while developing linkers (lld and mold), where debugging issues caused by breaking pointer equality was very difficult.

[joe-lawrence]

Hmm, at a high level, this reminds me of the discussion on #755 and #789 where kpatch-build was enhanced to use dynrelas (or klp-relocations in upstream parlance) on function pointer assignments -- this way the original function would be resolved and used on kpatch load.

I don't remember if we implemented the same for s390x, if not, would doing so help answer or simplify this problem?

[iii-i]

Yes, this sounds like a good solution to me. Did it require compiler modifications for other platforms?

[joe-lawrence]

Looking at https://github.com/dynup/kpatch/pull/789/commits, I'm pretty sure that the implementation was completely inside create-diff-object, and no compiler or compiler plugin changes were needed for x86/ppc64le.

[github-actions]

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

[sumanthkorikkar]

Looking into the issue.

[sumanthkorikkar]

Added the pull request

[sumanthkorikkar]

It would be great, if we can add the iii-i/linux@0f400db test in test-integration framework. Any suggestions to integrate it in kpatch ?. Thanks

[github-actions]

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.