[PATCH v5] x86: Disallow GOT memory access beyond its GOT slot

[Jan Beulich]

On 13.02.2025 09:25, H.J. Lu wrote:
> On Thu, Feb 13, 2025 at 4:11 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2025 03:16, H.J. Lu wrote:
>>> On Wed, Feb 12, 2025 at 7:22 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 11.02.2025 22:47, H.J. Lu wrote:
>>>>> @@ -12725,6 +12739,51 @@ imm_size (unsigned int n)
>>>>>    return size;
>>>>>  }
>>>>>
>>>>> +/* Since GOT slot size is 32 bits for i386 and 64 bits for x86-64,
>>>>> +   disallow GOT memory access beyond its GOT slot.  */
>>>>> +
>>>>> +static bool
>>>>> +check_GOT_memory (enum bfd_reloc_code_real reloc_type, unsigned int n)
>>>>> +{
>>>>> +  bool qword;
>>>>> +
>>>>> +  qword = false;
>>>>> +  if (reloc_type == BFD_RELOC_32_PCREL)
>>>>> +    {
>>>>> +      if (!GOT_symbol || GOT_symbol != i.op[n].disps->X_op_symbol)
>>>>> +     return true;
>>>>> +      qword = true;
>>>>> +    }
>>>>> +  else if (reloc_type != BFD_RELOC_386_GOT32)
>>>>> +    return true;
>>>>
>>>> What about BFD_RELOC_X86_64_GOT32? The 64-bit ABI simply refers to the
>>>> 32-bit one for many reloc types, including R_X86_64_GOT32.
>>>
>>> It can be added later.
>>
>> Wow, I'm impressed by how you try to really have entirely unpredictable
>> behavior in gas.
>>
>>>>> +  /* Disallow AMX TILE configuration load and store instructions.  */
>>>>> +  if (is_cpu (&i.tm, CpuAMX_TILE))
>>>>> +    return false;
>>>>> +
>>>>> +  /* Disallow instructions with 6-byte and 10-byte memory access.  */
>>>>> +  if (i.tm.operand_types[n].bitfield.fword
>>>>> +      || i.tm.operand_types[n].bitfield.tbyte)
>>>>> +    return false;
>>>>
>>>> Coming back to my concern regarding .insn: Can you explain to me how this
>>>> (as an example) isn't going to affect .insn (using Intel syntax), when
>>>> s_insn() has
>>>>
>>>>       for (j = 0; j < i.operands; ++j)
>>>>         i.tm.operand_types[j] = i.types[j];
>>>
>>> A testcase?
>>
>> Well, no. You're to prove (or at least make plausible) that your patch
>> (which shouldn't be committed anyway, considering my objection to the
>> overall idea) is correct (also applies to the similar other reply you
>> sent). Plus you can't really expect me to put yet more time in something
>> I think should never have been proposed in the first place, for not
>> really offering any benefit. Any .insn with a @gotpcrel(%rip) operand
>> and an "fword ptr" or "tbyte ptr" size specifier ought to do. I.e.
>> without having tried it, perhaps:
>>
>>         .insn 0x89, tbyte ptr sym@gotpcrel[rip], rax
> 
> [hjl@gnu-zen4-1 tmp]$ gcc -c x.s
> x.s: Assembler messages:
> x.s:2: Error: invalid GOT memory operand
> [hjl@gnu-zen4-1 tmp]$

Yet as you agreed with earlier, .insn should remain unaffected by this kind
of change.

Jan