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

[H.J. Lu]

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:

[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'
[hjl@gnu-tgl-3 tmp]$

> even need to resort to a segment register. A GPR may do as well,
> making the testcase less special.
>
> Jan



-- 
H.J.