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

[H.J. Lu]

On Wed, Apr 8, 2026 at 6:08 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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

They are different things.  They aren't interchangeable in most cases.

> 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.
>

Please go ahead.  BTW, can you assign

https://sourceware.org/bugzilla/show_bug.cgi?id=34028

to yourself?

Thanks.

-- 
H.J.