Project

General

Profile

Actions

Bug #10470

closed

array over-read in has_saved_fp()

Added by John Levon over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
2019-02-27
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

The analysis for OS-7517 (#10469) revealed -- albeit in a very strange way -- that has_saved_fp reads beyond the end of save_fp_pushes and save_fp_movs; here is the code for has_saved_fp:

static boolean_t
has_saved_fp(dis_handle_t *dhp, uint8_t *ins, int size)
{
        int             i, j;
        uint32_t        n;
        boolean_t       found_push = B_FALSE;
        ssize_t         sz = 0;

        for (i = 0; i < size; i += sz) {
                if ((sz = instr_size(dhp, ins, i, size)) < 1)
                        return (B_FALSE);

                if (found_push == B_FALSE) {
                        if (sz != 1) continue; 
                        n = INSTR1(ins, i);
                        for (j = 0; j <= NUM_FP_PUSHES; j++)
                                if (save_fp_pushes[j] == n) {
                                        found_push = B_TRUE;
                                        break;
                                }
                } else {
                        if (sz != 3)
                                continue;
                        n = INSTR3(ins, i);
                        for (j = 0; j <= NUM_FP_MOVS; j++)
                                if (save_fp_movs[j] == n)
                                        return (B_TRUE);
                }
        }

        return (B_FALSE);
}

And here are the definitions of save_fp_pushes and save_fp_movs:

static const uint8_t save_fp_pushes[] = {
        0x55,   /* pushq %rbp */
        0xcc    /* int $0x3 */
};
#define NUM_FP_PUSHES (sizeof (save_fp_pushes) / sizeof (save_fp_pushes[0]))

static const uint32_t save_fp_movs[] = {
        0x00e58948,     /* movq %rsp,%rbp, encoding 1 */
        0x00ec8b48,     /* movq %rsp,%rbp, encoding 2 */
};
#define NUM_FP_MOVS (sizeof (save_fp_movs) / sizeof (save_fp_movs[0]))

There is a clear off-by-one error in both loops; to dereference at index NUM_FP_PUSHES and NUM_FP_MOVS is to read off the end of the respective arrays.

This bug mirrors Joyent bug OS-7518


Related issues

Related to illumos gate - Feature #9996: use GCC 7 as default primary compilerClosedJohn Levon2018-11-21

Actions
Actions

Also available in: Atom PDF