[PATCH] x86: Check invalid immediate for rdmsr and wrmsrns
Jan Beulich
jbeulich@suse.com
Wed Apr 8 09:29:40 GMT 2026
On 08.04.2026 09:27, H.J. Lu wrote:
> On Wed, Apr 8, 2026 at 2:33 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.04.2026 02:22, H.J. Lu wrote:
>>> Verify immediate operand before processing immediate for rdmsr and
>>> wrmsrns.
>>>
>>> PR gas/34028
>>> * config/tc-i386.c (i386_assemble): Verify immediate operand
>>> before processing immediate for rdmsr and wrmsrns.
>>> * testsuite/gas/i386/msr_imm-inval.l: Updated.
>>> * testsuite/gas/i386/x86-64-msr_imm-inval.l: Likewise.
>>> * testsuite/gas/i386/msr_imm-inval.s: Add new rdmsr and wrmsrns
>>> tests.
>>> * testsuite/gas/i386/x86-64-msr_imm-inval.s: Likewise.
>>
>> The new insn forms you add don't make clear what exactly you're after:
>> The %cs operands make them invalid no matter what immediate is used. I
>> don't mind you adding "bogus register" forms, but that's not related
>> to the purpose of the patch. $y, otoh, looks like an entirely valid
>> operand to me: y can be an absolute symbol defined elsewhere. So what
>> exactly is it that you're trying to (a) prevent and (b) test?
>>
>> As an aside, adding further insn forms to the 32-bit testcase looks
>> pretty meaningless to me. The forms with operands aren't supported
>> outside of 64-bit mode anyway, no matter what exactly the operands
>> are.
>
> Assembler crashed at:
>
> if (is_cpu(current_templates.start, CpuUSER_MSR)
> || t->mnem_off == MN_rdmsr
> || t->mnem_off == MN_wrmsrns)
> {
> for (j = 0; j < i.imm_operands; j++)
> i.types[j] = smallest_imm_type (i.op[j].imms->X_add_number);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> }
>
> My patch prevents segfault. The new testcases caused segfault in
> both 32-bit and 64-bit modes.
I understand that (from Alan's report), albeit I still don't quite
understand why. Yes, we're still ahead of match_template(), but if
i.imm_operands is non-zero the first so many operands should be
immediates, shouldn't they? If not, I would think the operand
swapping immediately ahead of the if() you alter may need adjustment
instead. (Thinking of it, other insns covered there may similarly be
affected, which would even more so call for a correction there.)
Plus the testcase additions don't make clear which operand it is
that the problem was with, as for both operands you use a form which
isn't otherwise tested. I'm (now) guessing it's the %cs one, but it
could as well be the $f. I'm (now) further guessing that you may not
even need to resort to a segment register. A GPR may do as well,
making the testcase less special.
Jan
More information about the Binutils
mailing list