On Wed, 25 Feb 2026, at 23:27, Mukesh R wrote:
> On 2/21/26 08:43, Ard Biesheuvel wrote:
>> Just spotted this code in v7.0-rc
>>
>> On Wed, 10 Sep 2025, at 02:10, Mukesh Rathor wrote:
>> ...
>>
>>> +static asmlinkage void __noreturn hv_crash_c_entry(void)
>>
>> 'asmlinkage' means that the function may be called from another compilation
>> unit written in assembler, but it doesn't actually evaluate to anything in
>> most cases. Combining it with 'static' makes no sense whatsoever.
>
> 'static' means scope is limited to the file. Common in cases where function
> pointers are used, like here in this file way below.
>
> Like the comment says:
> "This is the C entry point from the asm glue code after...."
>
> IOW, called from assembly function (asm == assembly).
>
I wasn't asking you to explain what 'static' means. I was explaining to you
that asmlinkage means 'external linkage' whereas 'static' means the opposite,
and so combining them makes no sense.
>>
>>> +{
>>> + struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
>>> +
>>> + /* first thing, restore kernel gdt */
>>> + native_load_gdt(&ctxt->gdtr);
>>> +
>>> + asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
>>> + asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
>>> +
>>
>> This code is truly very broken. You cannot enter a C function without a
>> stack, and assign RSP half way down the function. Especially after
>> allocating local variables and/or calling other functions - it may happen to
>> work in most cases, but it is very fragile. (Other architectures have the
>> concept of 'naked' functions for this purpose but x86 does not)
>
> Local variable refers to static bss struct. IOW,
>
> asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
>
> same as:
> asm volatile("movq %0, %%rsp" : : "m"(&hv_crash_ctxt.rsp));
>
>
No, it is *not* the same. In practice, the compiler might perform this
substitution, but there is no guarantee that this happens.
>> IOW, this whole function should be written in asm.
>>> + asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
>>> + asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
>>> + asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
>>> + asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
>>> +
>>> + native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
>>> + asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
>>> +
>>> + asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
>>> + asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
>>> + asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
>>> +
>>> + native_load_idt(&ctxt->idtr);
>>> + native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
>>> + native_wrmsrq(MSR_EFER, ctxt->efer);
>>> +
>>> + /* restore the original kernel CS now via far return */
>>> + asm volatile("movzwq %0, %%rax\n\t"
>>> + "pushq %%rax\n\t"
>>> + "pushq $1f\n\t"
>>> + "lretq\n\t"
>>> + "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
>>> +
>>> + /* We are in asmlinkage without stack frame,
>>
>> You just switched to __KERNEL_CS via the stack.
>
> compiler doesn't know that.
>
So? But does it means to 'be in asmlinkage' in your interpretation? Did you
check what 'asmlinkage' actually evaluates to?
I am not asking you to justify why this broken code works in practice, I am
asking you to fix it.
>>> hence make a C function
>>> + * call which will buy stack frame to restore the tss or clear PT
>>> entry.
>>> + */
>>
>> Where does one buy a stack frame?
>
> A stack market :). Callee will create stack frame now that rsp is
> setup.
>
This code is beyond broken. Please propose fixes rather than try to argue why
carrying broken code like this is acceptable.
