On Mon, Jan 11, 2021 at 3:07 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > > On 1/7/21 9:25 PM, KP Singh wrote: > > On Thu, Jan 7, 2021 at 8:15 PM Andrii Nakryiko > > <andrii.nakryiko@xxxxxxxxx> wrote: > >> On Thu, Jan 7, 2021 at 11:07 AM Andrii Nakryiko > >> <andrii.nakryiko@xxxxxxxxx> wrote: > >>> On Thu, Jan 7, 2021 at 9:37 AM KP Singh <kpsingh@xxxxxxxxxx> wrote: > >>>> > >>>> The verifier allows ARG_PTR_TO_BTF_ID helper arguments to be NULL, so > >>>> helper implementations need to check this before dereferencing them. > >>>> This was already fixed for the socket storage helpers but not for task > >>>> and inode. > >>>> > >>>> The issue can be reproduced by attaching an LSM program to > >>>> inode_rename hook (called when moving files) which tries to get the > >>>> inode of the new file without checking for its nullness and then trying > >>>> to move an existing file to a new path: > >>>> > >>>> mv existing_file new_file_does_not_exist > >>> > >>> Seems like it's simple to write a selftest for this then? > > > > Sure, I will send in a separate patch for selftest and also for the typo. > > If it's small or trivial to add a selftest for the fix, I'd suggest to add it > as part of this series for 'ease of logistics' as it would otherwise be a bit > odd to i) either have a stand-alone patch against bpf tree with just a selftest > or ii) having to wait until bpf syncs into bpf-next and then send one against > bpf-next where for the latter there's risk that it gets forgotten in meantime > as it might take a while. Sounds good, I completely missed the fact the fix would take some time to sync from bpf to bpf-next and until then the selftest will keep crashing. I updated the selftest and will send in a v2 and while we are at it, will also fix the typo Andrii found. - KP > > Thanks, > Daniel
