On Thu, 2024-12-19 at 14:41 -0700, Daniel Xu wrote: [...] > > > I think that if test operates on a key like: > > > > > > valid key 15 > > > v > > > 0000000f <-- written to stack as a single u64 value > > > ^^^^^^^ > > > stack zero marks > > > > > > and is executed (e.g. using __retval annotation), > > > then CI passing for s390 should be enough. > > > > +1, something like that where for big-endian it will be all zero while > > for little endian it would be 0xf (and then make sure that the test > > should *fail* by making sure that 0xf is not a valid index, so NULL > > check is necessary) > > How would it work for LE to be 0xF but BE to be 0x0? > > The prog passes a pointer to the beginning of the u32 to > bpf_map_lookup_elem(). The kernel does a 4 byte read starting from that > address. On both BE and LE all 4 bytes will be interpreted. So set bits > cannot just go away. > > Am I missing something? Ok, thinking a bit more, the best test I can come up with is: u8 vals[8]; vals[0] = 0; ... vals[6] = 0; vals[7] = 0xf; p = bpf_map_lookup_elem(... vals ...); *p = 42; For LE vals as u32 should be 0x0f; For BE vals as u32 should be 0xf000_0000. Hence, it is not safe to remove null check for this program. What would verifier think about the value of such key? As far as I understand, there would be stack zero for for vals[0-6] and u8 stack spill for vals[7]. You were going to add a check for the spill size, which should help here. So, a negative test like above that checks that verifier complains that 'p' should be checked for nullness first? If anyone has better test in mind, please speak-up. [...]
