0

I'm, currently struggling with the correct implementation of a kernel-spinlock in combination with a return statement which should return a value to userspace. I implemented a kernel syscall 'sys_kernel_entropy_is_recording' which should return the value of a kernel-variable 'is_kernel_entropy_recording':

    asmlinkage bool sys_kernel_entropy_is_recording(void)
    {
        spin_lock(&entropy_analysis_lock);
        return is_kernel_entropy_recording;
        spin_unlock(&entropy_analysis_lock);
    }

At this point arise two questions:

Q1: Is this implementation correct at all, meaning will the correct value of 'is_kernel_entropy_recording' be returned to userspace and afterwards the spinlock be released?

My concerns are:

  • a) is it allowed to return a value from kernelspace to userspace this way at all?
  • b) the return statement is located before the spin_unlock statement, hence will spin_unlock be even called?

Q2: To answer these question myself I disassembled the compiled .o file but determined (at least it looks for me like) the spin_lock/spin_unlock calls are completely ignored by the compiler, as it just moves the value of 'sys_kernel_entropy_is_recording' to eax an calls ret (I'm not sure about line 'callq 0xa5'):

    (gdb) disassemble /m sys_kernel_entropy_is_recording
    Dump of assembler code for function sys_kernel_entropy_is_recording:
    49  {
       0x00000000000000a0 <+0>: callq  0xa5 <sys_kernel_entropy_is_recording+5>
       0x00000000000000a5 <+5>: push   %rbp
       0x00000000000000ad <+13>:    mov    %rsp,%rbp

    50      spin_lock(&entropy_analysis_lock);
    51      return is_kernel_entropy_recording;
    52      spin_unlock(&entropy_analysis_lock);
    53  }
       0x00000000000000b5 <+21>:    movzbl 0x0(%rip),%eax        # 0xbc <sys_kernel_entropy_is_recording+28>
       0x00000000000000bc <+28>:    pop    %rbp
       0x00000000000000bd <+29>:    retq 

Hence I guess the application of spinlock is not correct.. Could someone please give me an advice for an appropriate approach? Thanks a lot in advance!

OliverJL
  • 23
  • 5
  • As for disassembled code, `callq` is actually a function call. In you case, it likely calls `spin_lock` (or, in case `spin_lock` is a macro or an inline function, it calls its inner *function*). You may check that by asking gdb to apply relocations to the disassembled code. As for `spin_unlock` call, the compiler probably finds it as *unreachable*, and simply drops it. – Tsyvarev Oct 17 '17 at 07:15
  • as a side note, have you considered using _atomics_ for locking a single variable – myaut Oct 17 '17 at 07:41
  • Code after a `return` statement is unreachable. I'm surprised you don't get a compiler warning (don't you?) – Kaz Oct 17 '17 at 17:14

1 Answers1

2

It is prohibited to return from syscall with spinlock holded. And, as usual with C code, none instruction is executed after return statement.

Common practice is to save value obtained under lock into local variable, and return value of this variable after unlock:

bool ret;

spin_lock(&entropy_analysis_lock);
ret = is_kernel_entropy_recording;
spin_unlock(&entropy_analysis_lock);

return ret;
Tsyvarev
  • 60,011
  • 17
  • 110
  • 153