From:   Christophe Leroy <christophe.leroy@csgroup.eu>
To:     Josh Poimboeuf <jpoimboe@redhat.com>,
        Jiri Kosina <jikos@kernel.org>,
        Miroslav Benes <mbenes@suse.cz>,
        Petr Mladek <pmladek@suse.com>,
        Joe Lawrence <joe.lawrence@redhat.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>,
        "Naveen N . Rao" <naveen.n.rao@linux.vnet.ibm.com>
CC:     Christophe Leroy <christophe.leroy@csgroup.eu>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
        "live-patching@vger.kernel.org" <live-patching@vger.kernel.org>,
        Russell Currey <ruscur@russell.cc>
Subject: [PATCH v2 03/13] powerpc/module_32: Fix livepatching for RO modules
Thread-Topic: [PATCH v2 03/13] powerpc/module_32: Fix livepatching for RO
 modules
Thread-Index: AQHX9b/55CfsfkREwkW5lW2tFTV26g==
Date:   Mon, 20 Dec 2021 16:38:09 +0000
Message-ID: <d5697157cb7dba3927e19aa17c915a83bc550bb2.1640017960.git.christophe.leroy@csgroup.eu>
References: <cover.1640017960.git.christophe.leroy@csgroup.eu>
In-Reply-To: <cover.1640017960.git.christophe.leroy@csgroup.eu>
Accept-Language: fr-FR, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?sG3rK+HFlesylhsqjaYVrVBujlPgBXZSrkp8JsfKNhEMprIMLMdxQho1Je?=
 =?iso-8859-1?Q?ofzHS4PmcnkMDLIKEFUgF8TNO1eYHeFwxXqE9rlWvKn4FCBKQZdbP2hP6I?=
 =?iso-8859-1?Q?sTCMDGCGIYVWn0B4kd02oGkd7fy4miho61ZZdMqQAG1HOkN6iecHPT9f8U?=
 =?iso-8859-1?Q?f8igSBL6Nba1usoZSIcUap8GWqsJnpnYAYYCFrnileK52oNrqqXKEvBMhm?=
 =?iso-8859-1?Q?EDFjEbHyOd9hwICWZUfnQCz2cL4HLkxDfsIiWYhKzeYpO9cjd5RSyogz5Q?=
 =?iso-8859-1?Q?9BKbmAVchV1c8nKRuNh1xipn19F0Scdl4O9NDGLB52LUwIWFMrh5mIC9bE?=
 =?iso-8859-1?Q?zS3JX/9UqZQyx0NrmsMwuxAgn57H5pEpnl0JMsJNZ7Jz/ow72zstoH+xkw?=
 =?iso-8859-1?Q?cIvq3Cj7Le86kNzvxwkRN5jI5zEdEKdfg1Kaz9oYhsZ8tfOvAqZMl96Pw0?=
 =?iso-8859-1?Q?YKbf6HhzOA/YgOwdA0KkNZ/BN05Tb9eCtir5s1sXuP8VJDpvL0yK5xr5TZ?=
 =?iso-8859-1?Q?BKpxMmv0jsAKMphDLQJX+ek5COa4UtmWQMGuHxbMN3XOV9bALbUV/H5i2r?=
 =?iso-8859-1?Q?QclyMQgnC7osJNz1//+cpSWhOGKFLqNs/OlymyqDgQFhWPSnrSiLvnwjcj?=
 =?iso-8859-1?Q?Omns4rQ696xpqx485AL64fLsGwOBeEmDeowLIwCohmaKnS55n+ag6AYrHv?=
 =?iso-8859-1?Q?Qpd64BU+Y+6NXle3Y9/wEBbhg/BVSZuZEWnCC5BI2425MUu+KayyvBCqPQ?=
 =?iso-8859-1?Q?t6GCYZcxp+gz9YHATgrlqIPavkmIixvs5vu7HFaNFoNnqUxzFIXq75PxXS?=
 =?iso-8859-1?Q?r2VXf/RzpJlGIhfXX5nWNzDtt7wCB3ZBRwXjuBYBnzP0KnJfQH3Mboa2aZ?=
 =?iso-8859-1?Q?8NVK+zK6YHfZ35h7nhUMdh/zzjxNeLniSs/LkoamLXR2D8I3Ad9EyPLciK?=
 =?iso-8859-1?Q?2Dl5bfhVuoOFx7uyzzhPzG9hg4naflI0Cbq8KdCPAvoMDHPU3scF1WSuZd?=
 =?iso-8859-1?Q?vxbfFnAf2Ypj5X/TC83xTgrOKFUBF/yYhuODebbxqjsTio0DFOvUJEz0UM?=
 =?iso-8859-1?Q?oOLuxKuduD0JxZsFn02PfTVoJ0pjK2rtgRCH5TdzKIY107Kj/qQ2ks9JsW?=
 =?iso-8859-1?Q?6GcpbFW6uTTAODVtQWe8caw575ArJZfsu/LaCpU6f/pt/d1SehEUuZqobj?=
 =?iso-8859-1?Q?hFbXbEnPmiLAgCv00jpyLAlvTHAqtATxZXXmOf9cgh8oZglyk/Gy9W/Ze0?=
 =?iso-8859-1?Q?5ujJQ6mnvBx+fcQyAODqNaGxfIYnp0Qt7cYYc74r5uWE7wKVLWDvcg9Scj?=
 =?iso-8859-1?Q?A/v59W8Y0cWP6XWxETc8Edkcj02dvUEWET1XyPKw/p4c1wHJbb6Nfbp2eF?=
 =?iso-8859-1?Q?0XB7DJmXbCw9Fho/g3RfFe6jnv9lDM7UPXuR/SIGUWmLZ3EiLlt8sKqvXZ?=
 =?iso-8859-1?Q?4AvE+TeS7V4/TMY+jiuf6VkS/+wMSChtG+pTzdJn1bNGX09ukEgqSFXgAf?=
 =?iso-8859-1?Q?St9AmhbJUDzMa+HRKYNAh/+ltuKqKq+zgD8ofBMW+NCmdCM+8T3dnyxYHU?=
 =?iso-8859-1?Q?5e46BaAFOn323JjU1vl4AKBFbpTr7lghbjrq+OlyhS1OEMTsCsURfB3r9I?=
 =?iso-8859-1?Q?l6D5sRY1HGdMr8oejXv4OBbwbpvGcrdnXhyELlQ2HYm93uuueGpWiEY4NG?=
 =?iso-8859-1?Q?JUb7BeT317zj8TpcwDCvEbYTW8qaDgl9vYoGDFMdiRicm2w8YN15QxGo8S?=
 =?iso-8859-1?Q?Og5zRtc2j/qySduheX+D/AV1w=3D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
X-OriginatorOrg: csgroup.eu
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MRZP264MB2988.FRAP264.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-Network-Message-Id: 6484999f-1959-4747-8320-08d9c3d71b9b
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Dec 2021 16:38:09.7416
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 9914def7-b676-4fda-8815-5d49fb3b45c8
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GZHRbB1aDTXT1HxJdy/bGrJCSDMuaIcXXMixkTFZh2jx5D95+dnW4BhRrNxQqMot8ZDOYglbc0jXjxmyKRcbyxa7XncZHBAsA+KpN6oDxBA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MR1P264MB2161
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Livepatching a loaded module involves applying relocations through
apply_relocate_add(), which attempts to write to read-only memory when
CONFIG_STRICT_MODULE_RWX=y.

R_PPC_ADDR16_LO, R_PPC_ADDR16_HI, R_PPC_ADDR16_HA and R_PPC_REL24 are
the types generated by the kpatch-build userspace tool or klp-convert
kernel tree observed applying a relocation to a post-init module.

Use patch_instruction() to patch those relocations.

Commit 8734b41b3efe ("powerpc/module_64: Fix livepatching for
RO modules") did similar change in module_64.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Russell Currey <ruscur@russell.cc>
---
 arch/powerpc/kernel/module_32.c | 44 ++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index a491ad481d85..a0432ef46967 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -18,6 +18,7 @@
 #include <linux/bug.h>
 #include <linux/sort.h>
 #include <asm/setup.h>
+#include <asm/code-patching.h>
 
 /* Count how many different relocations (different symbol, different
    addend) */
@@ -174,15 +175,25 @@ static uint32_t do_plt_call(void *location,
 		entry++;
 	}
 
-	entry->jump[0] = PPC_RAW_LIS(_R12, PPC_HA(val));
-	entry->jump[1] = PPC_RAW_ADDI(_R12, _R12, PPC_LO(val));
-	entry->jump[2] = PPC_RAW_MTCTR(_R12);
-	entry->jump[3] = PPC_RAW_BCTR();
+	if (patch_instruction(&entry->jump[0], ppc_inst(PPC_RAW_LIS(_R12, PPC_HA(val)))))
+		return 0;
+	if (patch_instruction(&entry->jump[1], ppc_inst(PPC_RAW_ADDI(_R12, _R12, PPC_LO(val)))))
+		return 0;
+	if (patch_instruction(&entry->jump[2], ppc_inst(PPC_RAW_MTCTR(_R12))))
+		return 0;
+	if (patch_instruction(&entry->jump[3], ppc_inst(PPC_RAW_BCTR())))
+		return 0;
 
 	pr_debug("Initialized plt for 0x%x at %p\n", val, entry);
 	return (uint32_t)entry;
 }
 
+static int patch_location_16(uint32_t *loc, u16 value)
+{
+	loc = PTR_ALIGN_DOWN(loc, sizeof(u32));
+	return patch_instruction(loc, ppc_inst((*loc & 0xffff0000) | value));
+}
+
 int apply_relocate_add(Elf32_Shdr *sechdrs,
 		       const char *strtab,
 		       unsigned int symindex,
@@ -216,37 +227,42 @@ int apply_relocate_add(Elf32_Shdr *sechdrs,
 
 		case R_PPC_ADDR16_LO:
 			/* Low half of the symbol */
-			*(uint16_t *)location = value;
+			if (patch_location_16(location, PPC_LO(value)))
+				return -EFAULT;
 			break;
 
 		case R_PPC_ADDR16_HI:
 			/* Higher half of the symbol */
-			*(uint16_t *)location = (value >> 16);
+			if (patch_location_16(location, PPC_HI(value)))
+				return -EFAULT;
 			break;
 
 		case R_PPC_ADDR16_HA:
-			/* Sign-adjusted lower 16 bits: PPC ELF ABI says:
-			   (((x >> 16) + ((x & 0x8000) ? 1 : 0))) & 0xFFFF.
-			   This is the same, only sane.
-			 */
-			*(uint16_t *)location = (value + 0x8000) >> 16;
+			if (patch_location_16(location, PPC_HA(value)))
+				return -EFAULT;
 			break;
 
 		case R_PPC_REL24:
 			if ((int)(value - (uint32_t)location) < -0x02000000
-			    || (int)(value - (uint32_t)location) >= 0x02000000)
+			    || (int)(value - (uint32_t)location) >= 0x02000000) {
 				value = do_plt_call(location, value,
 						    sechdrs, module);
+				if (!value)
+					return -EFAULT;
+			}
 
 			/* Only replace bits 2 through 26 */
 			pr_debug("REL24 value = %08X. location = %08X\n",
 			       value, (uint32_t)location);
 			pr_debug("Location before: %08X.\n",
 			       *(uint32_t *)location);
-			*(uint32_t *)location
-				= (*(uint32_t *)location & ~0x03fffffc)
+			value = (*(uint32_t *)location & ~0x03fffffc)
 				| ((value - (uint32_t)location)
 				   & 0x03fffffc);
+
+			if (patch_instruction(location, ppc_inst(value)))
+				return -EFAULT;
+
 			pr_debug("Location after: %08X.\n",
 			       *(uint32_t *)location);
 			pr_debug("ie. jump to %08X+%08X = %08X\n",
-- 
2.33.1