Re: [PATCH bpf-next 1/3] s390/bpf: Add orig_gpr2 to user_pt_regs

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

Naveen N. Rao wrote:
> Hi Heiko,
> 
> Heiko Carstens wrote:
>> On Wed, Feb 02, 2022 at 12:41:58AM +0100, Ilya Leoshkevich wrote:
>>> user_pt_regs is used by eBPF in order to access userspace registers -
>>> see commit 466698e654e8 ("s390/bpf: correct broken uapi for
>>> BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first
>>> syscall argument from eBPF programs, we need to export orig_gpr2.
>>> 
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>>  arch/s390/include/asm/ptrace.h      | 2 +-
>>>  arch/s390/include/uapi/asm/ptrace.h | 1 +
>>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
>>> index 4ffa8e7f0ed3..c8698e643904 100644
>>> --- a/arch/s390/include/asm/ptrace.h
>>> +++ b/arch/s390/include/asm/ptrace.h
>>> @@ -83,9 +83,9 @@ struct pt_regs {
>>>  			unsigned long args[1];
>>>  			psw_t psw;
>>>  			unsigned long gprs[NUM_GPRS];
>>> +			unsigned long orig_gpr2;
>>>  		};
>>>  	};
>>> -	unsigned long orig_gpr2;
>>>  	union {
>>>  		struct {
>>>  			unsigned int int_code;
>>> diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
>>> index ad64d673b5e6..b3dec603f507 100644
>>> --- a/arch/s390/include/uapi/asm/ptrace.h
>>> +++ b/arch/s390/include/uapi/asm/ptrace.h
>>> @@ -295,6 +295,7 @@ typedef struct {
>>>  	unsigned long args[1];
>>>  	psw_t psw;
>>>  	unsigned long gprs[NUM_GPRS];
>>> +	unsigned long orig_gpr2;
>>>  } user_pt_regs;
>> 
>> Isn't this broken on nearly all architectures? I just checked powerpc,
>> arm64, and riscv. While powerpc seems to mirror pt_regs as user_pt_regs,
>> and therefore exports orig_gpr3, the bpf macros still seem to access the
>> wrong location to access the first syscall parameter(?).
> 
> On powerpc, gpr[3] continues to be valid on syscall entry (so this test 
> passes on powerpc), though orig_gpr3 will remain valid throughout.

Hmm.. we can't use orig_gpr3 since we don't use a syscall wrapper. All 
system calls just receive the parameters directly.

- Naveen