sdt_task: Avoid use-after-free after deleting task_storage

[EricccTaiwan]

In scx_task_free(), after releasing the allocator index, invoke bpf_task_storage_delete() to remove the task's entry from the BPF map.

Also zero-initialize mval->tid so that no stale identifier remains in local storage.

Edit: Remove redundant mval field updates after deletion.

[EricccTaiwan]

Hi @etsal , PTAL thanks!

[etsal]

This looks ok but I will give it a closer look, thank you!

[etsal]

I think if we delete then we should not be fixing up the fields of mval, this is a use-after-free. I suspect the only reason this works fine and passes verification is because of RCU preventing the use-after-free from having side effects. Can you adjust to only keep the delete call?

I am also wondering whether this has performance implications. If the kernel caches task_struct allocations and reuses deallocated task struct instances often, then this patch will add a BPF map allocation on the process initialization path. This is in addition to slowing down exit().

In the end, it all depends on how much reuse there is in the kernel of task_struct. @htejun, what do you think?

[EricccTaiwan]

Can you adjust to only keep the delete call?

I think it's best to wait for @htejun 's feedback before adjusting it.
TBH, I hadn't considered the implications in that depth. Appreciate the insight and explanation.
TBD depending on further discussion. 👍

[htejun]

Yeah, no need to update mval if the elem is being deleted.

[EricccTaiwan]

Yeah, no need to update mval if the elem is being deleted.

Ok, I only kept the bpf_task_storage_delete() call. PTAL!