[PATCH] x86: Check invalid immediate for rdmsr and wrmsrns

[Jan Beulich]

On 08.04.2026 11:55, H.J. Lu wrote:
> On Wed, Apr 8, 2026 at 5:29 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> 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
> 
> The problem is %cs:.
> 
>> 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
> 
> You guessed wrong:

Well, okay, yet ...

> [hjl@gnu-tgl-3 tmp]$ cat x.s
> .text
> foo:
> wrmsrns $y,%rax
> wrmsrns $y,%eax
> [hjl@gnu-tgl-3 tmp]$ gcc -c x.s
> x.s: Assembler messages:
> x.s:3: Error: operand type mismatch for `wrmsrns'
> x.s:4: Error: operand type mismatch for `wrmsrns'

... can you also explain the difference in behavior for segment register
vs GPR use? Does it maybe merely happen to crash for the former but not
the latter in the specific build you work with? Where what fix wants
making depends on properly understanding what is actually going wrong.
Otherwise we just stack hacks on one another without really sorting the
root cause. If you don't want to take the time, I can, but only later in
the week.

Jan