On Tue, 2024-10-08 at 17:14 +0800, Hou Tao wrote: > From: Hou Tao <houtao1@xxxxxxxxxx> > > The patch basically does the following three things to enable dynptr key > for bpf map: > > 1) Only allow PTR_TO_STACK typed register for dynptr key > The main reason is that bpf_dynptr can only be defined in the stack, so > for dynptr key only PTR_TO_STACK typed register is allowed. bpf_dynptr > could also be represented by CONST_PTR_TO_DYNPTR typed register (e.g., > in callback func or subprog), but it is not supported now. > > 2) Only allow fixed-offset for PTR_TO_STACK register > Variable-offset for PTR_TO_STACK typed register is disallowed, because > it is impossible to check whether or not the stack access is aligned > with BPF_REG_SIZE and is matched with the location of dynptr or > non-dynptr part in the map key. > > 3) Check the layout of the stack content is matched with the btf_record > Firstly check the start offset of the stack access is aligned with > BPF_REG_SIZE, then check the offset and the size of dynptr/non-dynptr > parts in the stack content is consistent with the btf_record of the map > key. > > Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx> > --- The logic of this patch looks correct, however I find it cumbersome. The only place where access to dynptr key is allowed is 'case ARG_PTR_TO_MAP_KEY' in check_func_arg(), a lot of places are modified to facilitate this. It seems that logic would be easier to follow if there would be a dedicated function to check dynptr key constraints, called only for the 'case ARG_PTR_TO_MAP_KEY'. This would als make 'struct dynptr_key_state' unnecessary as this state would be tracked inside such function. Wdyt? [...]
