Re: [PATCH 4/7] s390/module: Use s390_kernel_write() for relocations

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Thu, Apr 16, 2020 at 08:16:35AM -0500, Josh Poimboeuf wrote:
> On Thu, Apr 16, 2020 at 07:06:51AM -0500, Josh Poimboeuf wrote:
> > On Thu, Apr 16, 2020 at 10:56:02AM +0200, Miroslav Benes wrote:
> > > > +	bool early = me->state == MODULE_STATE_UNFORMED;
> > > > +
> > > > +	return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me,
> > > > +				    early ? memcpy : s390_kernel_write);
> > > 
> > > The compiler warns about
> > > 
> > > arch/s390/kernel/module.c: In function 'apply_relocate_add':
> > > arch/s390/kernel/module.c:453:24: warning: pointer type mismatch in conditional expression
> > >          early ? memcpy : s390_kernel_write);
> > 
> > Thanks, I'll get all that cleaned up.
> > 
> > I could have sworn I got a SUCCESS message from the kbuild bot.  Does it
> > ignore warnings nowadays?
> 
> Here's a fix on top of the original patch.
> 
> I changed s390_kernel_write() to return "void *" to match memcpy()
> (probably a separate patch).
> 
> I also grabbed the text_mutex for the !early case in
> apply_relocate_add() -- will do something similar for x86.
> 
> Will try to test this on a 390 box.

...and that borked the box pretty nicely.  Oops, big endian!  Need
something like this on top.

Sorry about not testing the patch in the first place, it looked trivial
and somehow I was thinking Peter writes exclusively bug-free code.

diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
index ee0904a23e24..513e640430ae 100644
--- a/arch/s390/kernel/module.c
+++ b/arch/s390/kernel/module.c
@@ -198,21 +198,25 @@ static int apply_rela_bits(Elf_Addr loc, Elf_Addr val,
 	}
 
 	if (bits == 8) {
-		write(dest, &val, 1);
+		unsigned char tmp = val;
+		write(dest, &tmp, 1);
 	} else if (bits == 12) {
 		unsigned short tmp = (val & 0xfff) |
 			(*(unsigned short *) loc & 0xf000);
 		write(dest, &tmp, 2);
 	} else if (bits == 16) {
-		write(dest, &val, 2);
+		unsigned short tmp = val;
+		write(dest, &tmp, 2);
 	} else if (bits == 20) {
 		unsigned int tmp = (val & 0xfff) << 16 |
 			(val & 0xff000) >> 4 | (*(unsigned int *) loc & 0xf00000ff);
 		write(dest, &tmp, 4);
 	} else if (bits == 32) {
-		write(dest, &val, 4);
+		unsigned int tmp = val;
+		write(dest, &tmp, 4);
 	} else if (bits == 64) {
-		write(dest, &val, 8);
+		unsigned long tmp = val;
+		write(dest, &tmp, 8);
 	}
 	return 0;
 }

-- 
Josh