Skip to content

Commit 86142b0

Browse files
author
H. Peter Anvin
committed
assemble: limit-check operand references
Don't do an out-of-range check for the operands, even temporarily. Setting the operand pointer to NULL will help catch errors when accessing non-operands, too. Signed-off-by: H. Peter Anvin <hpa@zytor.com>
1 parent 699684b commit 86142b0

File tree

3 files changed

+73
-53
lines changed

3 files changed

+73
-53
lines changed

asm/assemble.c

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* ----------------------------------------------------------------------- *
22
*
3-
* Copyright 1996-2023 The NASM Authors - All Rights Reserved
3+
* Copyright 1996-2024 The NASM Authors - All Rights Reserved
44
* See the file AUTHORS included with the NASM distribution for
55
* the specific copyright holders.
66
*
@@ -109,9 +109,18 @@ static void add_asp(insn *, int);
109109
static int process_ea(operand *, ea *, int, int, opflags_t,
110110
insn *, enum ea_type, const char **);
111111

112+
/* Get the pointer to an operand if it exits */
113+
static inline struct operand *get_operand(insn *ins, unsigned int n)
114+
{
115+
if (n >= (unsigned int)ins->operands)
116+
return NULL;
117+
else
118+
return &ins->oprs[n];
119+
}
120+
112121
static inline bool absolute_op(const struct operand *o)
113122
{
114-
return o->segment == NO_SEG && o->wrt == NO_SEG &&
123+
return o && o->segment == NO_SEG && o->wrt == NO_SEG &&
115124
!(o->opflags & OPFLAG_RELATIVE);
116125
}
117126

@@ -537,8 +546,9 @@ static bool jmp_match(int32_t segment, int64_t offset, int bits,
537546
const uint8_t *code = temp->code;
538547
uint8_t c = code[0];
539548
bool is_byte;
549+
const struct operand * const op0 = get_operand(ins, 0);
540550

541-
if (((c & ~1) != 0370) || (ins->oprs[0].type & STRICT))
551+
if (((c & ~1) != 0370) || (op0->type & STRICT))
542552
return false;
543553
if (!optimizing.level || (optimizing.flag & OPTIM_DISABLE_JMP_MATCH))
544554
return false;
@@ -547,14 +557,14 @@ static bool jmp_match(int32_t segment, int64_t offset, int bits,
547557

548558
isize = calcsize(segment, offset, bits, ins, temp);
549559

550-
if (ins->oprs[0].opflags & OPFLAG_UNKNOWN)
560+
if (op0->opflags & OPFLAG_UNKNOWN)
551561
/* Be optimistic in pass 1 */
552562
return true;
553563

554-
if (ins->oprs[0].segment != segment)
564+
if (op0->segment != segment)
555565
return false;
556566

557-
isize = ins->oprs[0].offset - offset - isize; /* isize is delta */
567+
isize = op0->offset - offset - isize; /* isize is delta */
558568
is_byte = (isize >= -128 && isize <= 127); /* is it byte size? */
559569

560570
if (is_byte && c == 0371 && ins->prefixes[PPS_REP] == P_BND) {
@@ -1211,7 +1221,7 @@ static int64_t calcsize(int32_t segment, int64_t offset, int bits,
12111221
uint8_t c;
12121222
int rex_mask = ~0;
12131223
int op1, op2;
1214-
struct operand *opx;
1224+
struct operand *opx, *opy;
12151225
uint8_t opex = 0;
12161226
enum ea_type eat;
12171227
uint8_t hleok = 0;
@@ -1232,8 +1242,9 @@ static int64_t calcsize(int32_t segment, int64_t offset, int bits,
12321242
while (*codes) {
12331243
c = *codes++;
12341244
op1 = (c & 3) + ((opex & 1) << 2);
1245+
opx = get_operand(ins, op1);
12351246
op2 = ((c >> 3) & 3) + ((opex & 2) << 1);
1236-
opx = &ins->oprs[op1];
1247+
opy = NULL;
12371248
opex = 0; /* For the next iteration */
12381249

12391250
switch (c) {
@@ -1310,8 +1321,8 @@ static int64_t calcsize(int32_t segment, int64_t offset, int bits,
13101321
case 0171:
13111322
c = *codes++;
13121323
op2 = (op2 & ~3) | ((c >> 3) & 3);
1313-
opx = &ins->oprs[op2];
1314-
ins->rex |= op_rexflags(opx, REX_R|REX_H|REX_P|REX_W);
1324+
opy = get_operand(ins, op2);
1325+
ins->rex |= op_rexflags(opy, REX_R|REX_H|REX_P|REX_W);
13151326
length++;
13161327
break;
13171328

@@ -1479,14 +1490,15 @@ static int64_t calcsize(int32_t segment, int64_t offset, int bits,
14791490
*! severe programming error as the code could break at
14801491
*! any time for any number of reasons.
14811492
*/
1482-
if (!absolute_op(&ins->oprs[0]))
1493+
/* The bytecode ends in 0, so opx points to operand 0 */
1494+
if (!absolute_op(opx))
14831495
nasm_nonfatal("attempt to reserve non-constant"
14841496
" quantity of BSS space");
1485-
else if (ins->oprs[0].opflags & OPFLAG_FORWARD)
1497+
else if (opx->opflags & OPFLAG_FORWARD)
14861498
nasm_warn(WARN_FORWARD, "forward reference in RESx "
14871499
"can have unpredictable results");
14881500
else
1489-
length += ins->oprs[0].offset * resb_bytes(ins->opcode);
1501+
length += opx->offset * resb_bytes(ins->opcode);
14901502
break;
14911503

14921504
case 0341:
@@ -1546,8 +1558,9 @@ static int64_t calcsize(int32_t segment, int64_t offset, int bits,
15461558
ea ea_data;
15471559
int rfield;
15481560
opflags_t rflags;
1549-
struct operand *opy = &ins->oprs[op2];
1550-
struct operand *op_er_sae;
1561+
const struct operand *op_er_sae;
1562+
1563+
opy = get_operand(ins, op2);
15511564

15521565
ea_data.rex = 0; /* Ensure ea.REX is initially 0 */
15531566

@@ -1558,11 +1571,11 @@ static int64_t calcsize(int32_t segment, int64_t offset, int bits,
15581571
} else {
15591572
rflags = 0;
15601573
rfield = c & 7;
1574+
opx = NULL;
15611575
}
15621576

15631577
/* EVEX.b1 : evex_brerop contains the operand position */
1564-
op_er_sae = (ins->evex_brerop >= 0 ?
1565-
&ins->oprs[ins->evex_brerop] : NULL);
1578+
op_er_sae = ins->evex_brerop;
15661579

15671580
if (op_er_sae && (op_er_sae->decoflags & (ER | SAE))) {
15681581
/* set EVEX.b */
@@ -1888,7 +1901,7 @@ static void gencode(struct out_data *data, insn *ins)
18881901
uint8_t bytes[4];
18891902
int64_t size;
18901903
int op1, op2;
1891-
struct operand *opx;
1904+
struct operand *opx, *opy;
18921905
const uint8_t *codes = data->itemp->code;
18931906
uint8_t opex = 0;
18941907
enum ea_type eat = EA_SCALAR;
@@ -1903,8 +1916,9 @@ static void gencode(struct out_data *data, insn *ins)
19031916
while (*codes) {
19041917
c = *codes++;
19051918
op1 = (c & 3) + ((opex & 1) << 2);
1919+
opx = get_operand(ins, op1);
19061920
op2 = ((c >> 3) & 3) + ((opex & 2) << 1);
1907-
opx = &ins->oprs[op1];
1921+
opy = NULL;
19081922
opex = 0; /* For the next iteration */
19091923

19101924

@@ -2013,11 +2027,11 @@ static void gencode(struct out_data *data, insn *ins)
20132027
const struct operand *opy;
20142028

20152029
c = *codes++;
2016-
opx = &ins->oprs[c >> 3];
2017-
opy = &ins->oprs[c & 7];
2030+
opx = get_operand(ins, op1 = (c >> 3) & 7);
2031+
opy = get_operand(ins, op2 = c & 7);
20182032
if (!absolute_op(opy))
20192033
nasm_nonfatal("non-absolute expression not permitted "
2020-
"as argument %d", c & 7);
2034+
"as argument %d", op2);
20212035
else if (opy->offset & ~mask)
20222036
nasm_warn(ERR_PASS2|WARN_NUMBER_OVERFLOW,
20232037
"is4 argument exceeds bounds");
@@ -2027,7 +2041,7 @@ static void gencode(struct out_data *data, insn *ins)
20272041

20282042
case 0173:
20292043
c = *codes++;
2030-
opx = &ins->oprs[c >> 4];
2044+
opx = get_operand(ins, op1 = c >> 4);
20312045
c &= 15;
20322046
goto emit_is4;
20332047

@@ -2250,7 +2264,8 @@ static void gencode(struct out_data *data, insn *ins)
22502264
int rfield;
22512265
opflags_t rflags;
22522266
uint8_t *p;
2253-
struct operand *opy = &ins->oprs[op2];
2267+
2268+
opy = get_operand(ins, op2);
22542269

22552270
if (c <= 0177) {
22562271
/* pick rfield from operand b (opx) */
@@ -2260,6 +2275,7 @@ static void gencode(struct out_data *data, insn *ins)
22602275
/* rfield is constant */
22612276
rflags = 0;
22622277
rfield = c & 7;
2278+
opx = NULL;
22632279
}
22642280

22652281
if (process_ea(opy, &ea_data, bits,
@@ -2390,16 +2406,10 @@ static enum match_result find_match(const struct itemplate **tempp,
23902406
enum match_result m, merr;
23912407
opflags_t xsizeflags[MAX_OPERANDS];
23922408
bool opsizemissing = false;
2393-
int8_t broadcast = instruction->evex_brerop;
23942409
int i;
23952410

2396-
/* broadcasting uses a different data element size */
2397-
for (i = 0; i < instruction->operands; i++) {
2398-
if (i == broadcast)
2399-
xsizeflags[i] = instruction->oprs[i].decoflags & BRSIZE_MASK;
2400-
else
2401-
xsizeflags[i] = instruction->oprs[i].type & SIZE_MASK;
2402-
}
2411+
for (i = 0; i < instruction->operands; i++)
2412+
xsizeflags[i] = instruction->oprs[i].xsize;
24032413

24042414
merr = MERR_INVALOP;
24052415

@@ -2415,11 +2425,12 @@ static enum match_result find_match(const struct itemplate **tempp,
24152425
/*
24162426
* Missing operand size and a candidate for fuzzy matching...
24172427
*/
2418-
for (i = 0; i < temp->operands; i++)
2419-
if (i == broadcast)
2428+
for (i = 0; i < temp->operands; i++) {
2429+
if (instruction->oprs[i].bcast)
24202430
xsizeflags[i] |= temp->deco[i] & BRSIZE_MASK;
24212431
else
24222432
xsizeflags[i] |= temp->opd[i] & SIZE_MASK;
2433+
}
24232434
opsizemissing = true;
24242435
}
24252436
if (m > merr)
@@ -2433,23 +2444,24 @@ static enum match_result find_match(const struct itemplate **tempp,
24332444
goto done;
24342445

24352446
for (i = 0; i < instruction->operands; i++) {
2447+
struct operand *op = &instruction->oprs[i];
24362448
/*
24372449
* We ignore extrinsic operand sizes on registers, so we should
24382450
* never try to fuzzy-match on them. This also resolves the case
24392451
* when we have e.g. "xmmrm128" in two different positions.
24402452
*/
2441-
if (is_class(REGISTER, instruction->oprs[i].type))
2453+
if (is_class(REGISTER, op->type))
24422454
continue;
24432455

24442456
/* This tests if xsizeflags[i] has more than one bit set */
24452457
if ((xsizeflags[i] & (xsizeflags[i]-1)))
24462458
goto done; /* No luck */
24472459

2448-
if (i == broadcast) {
2449-
instruction->oprs[i].decoflags |= xsizeflags[i];
2450-
instruction->oprs[i].type |= brsize_to_size(xsizeflags[i]);
2460+
if (op->bcast) {
2461+
op->decoflags |= xsizeflags[i];
2462+
op->type |= brsize_to_size(xsizeflags[i]);
24512463
} else {
2452-
instruction->oprs[i].type |= xsizeflags[i]; /* Set the size */
2464+
op->type |= xsizeflags[i]; /* Set the size */
24532465
}
24542466
}
24552467

asm/parser.c

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,15 @@ const expr *next_expr(const expr *e, const expr **next_list)
221221
return e;
222222
}
223223

224-
static inline void init_operand(operand *op)
224+
static inline void init_operand(operand *op, unsigned int opidx)
225225
{
226-
memset(op, 0, sizeof *op);
226+
nasm_zero(*op);
227227

228228
op->basereg = -1;
229229
op->indexreg = -1;
230230
op->segment = NO_SEG;
231231
op->wrt = NO_SEG;
232+
op->opidx = opidx;
232233
}
233234

234235
static int parse_mref(operand *op, const expr *e)
@@ -648,7 +649,7 @@ insn *parse_line(char *buffer, insn *result)
648649
result->eops = NULL; /* must do this, whatever happens */
649650
result->operands = 0; /* must initialize this */
650651
result->evex_rm = 0; /* Ensure EVEX rounding mode is reset */
651-
result->evex_brerop = -1; /* Reset EVEX broadcasting/ER op position */
652+
result->evex_brerop = NULL; /* Reset EVEX broadcasting/ER op position */
652653

653654
/* Ignore blank lines */
654655
if (i == TOKEN_EOS)
@@ -847,6 +848,10 @@ insn *parse_line(char *buffer, insn *result)
847848
*/
848849
far_jmp_ok = result->opcode == I_JMP || result->opcode == I_CALL;
849850

851+
/* Initialize operand structures */
852+
for (opnum = 0; opnum < MAX_OPERANDS; opnum++)
853+
init_operand(&result->oprs[opnum], opnum);
854+
850855
for (opnum = 0; opnum < MAX_OPERANDS; opnum++) {
851856
operand *op = &result->oprs[opnum];
852857
expr *value; /* used most of the time */
@@ -856,8 +861,6 @@ insn *parse_line(char *buffer, insn *result)
856861
int setsize = 0;
857862
decoflags_t brace_flags = 0; /* flags for decorators in braces */
858863

859-
init_operand(op);
860-
861864
i = stdscan(NULL, &tokval);
862865
if (i == TOKEN_EOS)
863866
break; /* end of operands: get out of here */
@@ -1035,7 +1038,7 @@ insn *parse_line(char *buffer, insn *result)
10351038
if (!value)
10361039
goto fail;
10371040

1038-
init_operand(&o2);
1041+
init_operand(&o2, 0);
10391042
if (parse_mref(&o2, value))
10401043
goto fail;
10411044

@@ -1288,16 +1291,18 @@ insn *parse_line(char *buffer, insn *result)
12881291
}
12891292

12901293
/* remember the position of operand having broadcasting/ER mode */
1291-
if (op->decoflags & (BRDCAST_MASK | ER | SAE))
1292-
result->evex_brerop = opnum;
1294+
if (op->decoflags & (BRDCAST_MASK | ER | SAE)) {
1295+
result->evex_brerop = op;
1296+
op->bcast = true;
1297+
op->xsize = op->decoflags & BRSIZE_MASK;
1298+
} else {
1299+
op->bcast = false;
1300+
op->xsize = op->type & SIZE_MASK;
1301+
}
12931302
}
12941303

12951304
result->operands = opnum; /* set operand count */
12961305

1297-
/* clear remaining operands */
1298-
while (opnum < MAX_OPERANDS)
1299-
result->oprs[opnum++].type = 0;
1300-
13011306
return result;
13021307

13031308
fail:

include/nasm.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ enum eval_hint { /* values for `hinttype' */
631631

632632
typedef struct operand { /* operand to an instruction */
633633
opflags_t type; /* type of operand */
634-
int disp_size; /* 0 means default; 16; 32; 64 */
634+
opflags_t xsize; /* size flags used in find_match() */
635635
enum reg_enum basereg;
636636
enum reg_enum indexreg; /* address registers */
637637
int scale; /* index scale */
@@ -643,6 +643,9 @@ typedef struct operand { /* operand to an instruction */
643643
int eaflags; /* special EA flags */
644644
int opflags; /* see OPFLAG_* defines below */
645645
decoflags_t decoflags; /* decorator flags such as {...} */
646+
bool bcast; /* broadcast operand */
647+
uint8_t disp_size; /* 0 means default; 16; 32; 64 */
648+
uint8_t opidx; /* Operand index */
646649
} operand;
647650

648651
#define OPFLAG_FORWARD 1 /* operand is a forward reference */
@@ -766,7 +769,7 @@ typedef struct insn { /* an instruction itself */
766769
/* EVEX.P2: [z,L'L,b,V',aaa] */
767770
enum ttypes evex_tuple; /* Tuple type for compressed Disp8*N */
768771
int evex_rm; /* static rounding mode for AVX512 (EVEX) */
769-
int8_t evex_brerop; /* BR/ER/SAE operand position */
772+
struct operand *evex_brerop; /* BR/ER/SAE operand position */
770773
} insn;
771774

772775
/* Instruction flags type: IF_* flags are defined in insns.h */

0 commit comments

Comments
 (0)