Project

General

Profile

Actions

Bug #12982

closed

ambiguous instructions require an explicit suffix

Added by Andy Fiddaman 10 months ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

As of version 2.35, the GNU assembler has gained a new class of warnings about instructions where the size of the operand is ambiguous and cannot be inferred. They look like:

../../i86pc/ml/locore.s:187: Warning: no instruction mnemonic suffix given and no register operands; using default for `bts'

There are a few places in the kernel where this occurs around cmp and bt[s] instructions; they need updating with an explicit suffix

In some cases, the existing assumed operand size may also not be correct, resulting in real bugs.


Files

12982_wsdiff.out (2.44 MB) 12982_wsdiff.out Andy Fiddaman, 2020-07-28 12:17 PM
Actions #1

Updated by Electric Monk 10 months ago

  • Gerrit CR set to 814
Actions #2

Updated by Andy Fiddaman 10 months ago

I've attached the full output from wsdiff with the proposed patch (https://code.illumos.org/c/illumos-gate/+/814 ) applied. The relevant changes are:

In the sn1 and s10 brands, the following changes to the handler test in the various callbacks (this is for sn1, s10 is the same):

278c278
<     sn1_brand_syscall32_callback+0x19: 41 83 3f 00        cmpl   $0x0,(%r15)
---
>     sn1_brand_syscall32_callback+0x19: 49 83 3f 00        cmpq   $0x0,(%r15)
318c318
<     sn1_brand_syscall_callback+0x19:   41 83 3f 00        cmpl   $0x0,(%r15)
---
>     sn1_brand_syscall_callback+0x19:   49 83 3f 00        cmpq   $0x0,(%r15)
358c358
<     sn1_brand_sysenter_callback+0x19:  41 83 3f 00        cmpl   $0x0,(%r15)
---
>     sn1_brand_sysenter_callback+0x19:  49 83 3f 00        cmpq   $0x0,(%r15)
398c398
<     sn1_brand_int91_callback+0x19:     41 83 3f 00        cmpl   $0x0,(%r15)
---
>     sn1_brand_int91_callback+0x19:     49 83 3f 00        cmpq   $0x0,(%r15)

This is code to check if the brand user-space handler pointer is NULL. This is a 64-bit pointer so the new cmpq is appropriate.

In the kernel (amd64/unix), the following changes at the start of each trap:

<     tr_dbgtrap+0xa:                83 7c 24 58 00     cmpl   $0x0,0x58(%rsp)
<     tr_dbgtrap+0xf:                74 02              je     +0x2    <tr_dbgtrap+0x13>
<     tr_dbgtrap+0x11:               cd 08              int    $0x8
<     tr_dbgtrap+0x13:               48 c7 44 24 58 01  movq   $0x1,0x58(%rsp)
---
>     tr_dbgtrap+0xa:                48 83 7c 24 58 00  cmpq   $0x0,0x58(%rsp)
>     tr_dbgtrap+0x10:               74 02              je     +0x2    <tr_dbgtrap+0x14>
>     tr_dbgtrap+0x12:               cd 08              int    $0x8
>     tr_dbgtrap+0x14:               48 c7 44 24 58 01  movq   $0x1,0x58(%rsp)

This is a check against KPTI_FLAG being 0. KPTI_FLAG is only used to store 0 or 1 but as it's a 64-bit type cmpq is appropriate.

usr/src/uts/i86pc/ml/offsets.in
265:  kf_tr_flag      KPTI_FLAG

usr/src/uts/i86pc/sys/machcpuvar.h
120:  uint64_t        kf_tr_flag;

In boot/cdboot - now using orb in place of the assembler's selected orl. This is the same change that was made in FreeBSD here when adding explicit suffixes.

Actions #3

Updated by Andy Fiddaman 10 months ago

I tested building gate with this patch and using GNU binutils 2.34 and 2.35.
I booted to these bits with KPTI both enabled and not.

I have, however, not tested the s10 or sn1 zone brands which are affected by this change.

Actions #4

Updated by Electric Monk 10 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit abe1e6b305e672d0eedeb6b52003acac658c8371

commit  abe1e6b305e672d0eedeb6b52003acac658c8371
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2020-07-28T21:53:05.000Z

    12982 ambiguous instructions require an explicit suffix
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: John Levon <john.levon@joyent.com>
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF