arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames

The dynamic SCS patching code pretends to parse the DWARF augmentation
data in the CIE (header) frame, and handle accordingly when processing
the individual FDE frames based on this CIE frame. However, the boolean
variable is defined inside the loop, and so the parsed value is ignored.

The same applies to the code alignment field, which is also read from
the header but then discarded.

This was never spotted before because Clang is the only compiler that
supports dynamic SCS patching (which is essentially an Android feature),
and the unwind tables it produces are highly uniform, and match the
de facto defaults.

So instead of testing for the 'z' flag in the augmentation data field,
require a fixed augmentation data string of 'zR', and simplify the rest
of the code accordingly.

Also introduce some error codes to specify why the patching failed, and
log it to the kernel console on failure when this happens when loading a
module. (Doing so for vmlinux is infeasible, as the patching is done
extremely early in the boot.)

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Tested-by: Sami Tolvanen <samitolvanen@google.com>
Link: https://lore.kernel.org/r/20241106185513.3096442-6-ardb+git@google.com
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
index 2e010ea76be2f5553bc9827b2e5022341fe0745e..934e9217cd74279e31a44e9708c0df9c81b00cc4 100644 (file)
--- a/arch/arm64/include/asm/scs.h
+++ b/arch/arm64/include/asm/scs.h
@@ -46,6 +46,13 @@ static inline void dynamic_scs_init(void)
 static inline void dynamic_scs_init(void) {}
 #endif
 
+enum {
+       EDYNSCS_INVALID_CIE_HEADER              = 1,
+       EDYNSCS_INVALID_CIE_SDATA_SIZE          = 2,
+       EDYNSCS_INVALID_FDE_AUGM_DATA_SIZE      = 3,
+       EDYNSCS_INVALID_CFA_OPCODE              = 4,
+};
+
 int __pi_scs_patch(const u8 eh_frame[], int size);
 asmlinkage void __pi_scs_patch_vmlinux(void);
 

diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 36b25af563243083da1ac1c1624129c7f6d25ad1..06bb680bfe975cf84a09da80bc2b293340191217 100644 (file)
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -462,14 +462,20 @@ int module_finalize(const Elf_Ehdr *hdr,
                    struct module *me)
 {
        const Elf_Shdr *s;
+       int ret;
+
        s = find_section(hdr, sechdrs, ".altinstructions");
        if (s)
                apply_alternatives_module((void *)s->sh_addr, s->sh_size);
 
        if (scs_is_dynamic()) {
                s = find_section(hdr, sechdrs, ".init.eh_frame");
-               if (s)
-                       __pi_scs_patch((void *)s->sh_addr, s->sh_size);
+               if (s) {
+                       ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size);
+                       if (ret)
+                               pr_err("module %s: error occurred during dynamic SCS patching (%d)\n",
+                                      me->name, ret);
+               }
        }
 
        return module_init_ftrace_plt(hdr, sechdrs, me);

diff --git a/arch/arm64/kernel/pi/patch-scs.c b/arch/arm64/kernel/pi/patch-scs.c
index 49d8b40e61bc050d7510fe961394d31ab317b97e..cec8f0a52bbc761a6f01a650122bba950484fc8f 100644 (file)
--- a/arch/arm64/kernel/pi/patch-scs.c
+++ b/arch/arm64/kernel/pi/patch-scs.c
@@ -120,7 +120,11 @@ struct eh_frame {
        union {
                struct { // CIE
                        u8      version;
-                       u8      augmentation_string[];
+                       u8      augmentation_string[3];
+                       u8      code_alignment_factor;
+                       u8      data_alignment_factor;
+                       u8      return_address_register;
+                       u8      augmentation_data_size;
                };
 
                struct { // FDE
@@ -132,25 +136,21 @@ struct eh_frame {
 };
 
 static int scs_handle_fde_frame(const struct eh_frame *frame,
-                               bool fde_has_augmentation_data,
                                int code_alignment_factor,
                                bool dry_run)
 {
        int size = frame->size - offsetof(struct eh_frame, opcodes) + 4;
        u64 loc = (u64)offset_to_ptr(&frame->initial_loc);
        const u8 *opcode = frame->opcodes;
+       int l;
 
-       if (fde_has_augmentation_data) {
-               int l;
+       // assume single byte uleb128_t for augmentation data size
+       if (*opcode & BIT(7))
+               return EDYNSCS_INVALID_FDE_AUGM_DATA_SIZE;
 
-               // assume single byte uleb128_t
-               if (WARN_ON(*opcode & BIT(7)))
-                       return -ENOEXEC;
-
-               l = *opcode++;
-               opcode += l;
-               size -= l + 1;
-       }
+       l = *opcode++;
+       opcode += l;
+       size -= l + 1;
 
        /*
         * Starting from 'loc', apply the CFA opcodes that advance the location
@@ -201,7 +201,7 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,
                        break;
 
                default:
-                       return -ENOEXEC;
+                       return EDYNSCS_INVALID_CFA_OPCODE;
                }
        }
        return 0;
@@ -209,12 +209,11 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,
 
 int scs_patch(const u8 eh_frame[], int size)
 {
+       int code_alignment_factor = 1;
        const u8 *p = eh_frame;
 
        while (size > 4) {
                const struct eh_frame *frame = (const void *)p;
-               bool fde_has_augmentation_data = true;
-               int code_alignment_factor = 1;
                int ret;
 
                if (frame->size == 0 ||
@@ -223,28 +222,36 @@ int scs_patch(const u8 eh_frame[], int size)
                        break;
 
                if (frame->cie_id_or_pointer == 0) {
-                       const u8 *p = frame->augmentation_string;
-
-                       /* a 'z' in the augmentation string must come first */
-                       fde_has_augmentation_data = *p == 'z';
+                       /*
+                        * Require presence of augmentation data (z) with a
+                        * specifier for the size of the FDE initial_loc and
+                        * range fields (R), and nothing else.
+                        */
+                       if (strcmp(frame->augmentation_string, "zR"))
+                               return EDYNSCS_INVALID_CIE_HEADER;
 
                        /*
                         * The code alignment factor is a uleb128 encoded field
                         * but given that the only sensible values are 1 or 4,
-                        * there is no point in decoding the whole thing.
+                        * there is no point in decoding the whole thing.  Also
+                        * sanity check the size of the data alignment factor
+                        * field, and the values of the return address register
+                        * and augmentation data size fields.
                         */
-                       p += strlen(p) + 1;
-                       if (!WARN_ON(*p & BIT(7)))
-                               code_alignment_factor = *p;
+                       if ((frame->code_alignment_factor & BIT(7)) ||
+                           (frame->data_alignment_factor & BIT(7)) ||
+                           frame->return_address_register != 30 ||
+                           frame->augmentation_data_size != 1)
+                               return EDYNSCS_INVALID_CIE_HEADER;
+
+                       code_alignment_factor = frame->code_alignment_factor;
                } else {
-                       ret = scs_handle_fde_frame(frame,
-                                                  fde_has_augmentation_data,
-                                                  code_alignment_factor,
+                       ret = scs_handle_fde_frame(frame, code_alignment_factor,
                                                   true);
                        if (ret)
                                return ret;
-                       scs_handle_fde_frame(frame, fde_has_augmentation_data,
-                                            code_alignment_factor, false);
+                       scs_handle_fde_frame(frame, code_alignment_factor,
+                                            false);
                }
 
                p += sizeof(frame->size) + frame->size;