On 7/1/20 11:15 AM, KP Singh wrote:
On Wed, Jul 1, 2020 at 1:41 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
On Wed, Jul 01, 2020 at 01:26:44AM +0200, Daniel Borkmann wrote:
On 6/30/20 6:33 AM, Alexei Starovoitov wrote:
[...]
+/* list of non-sleepable kernel functions that are otherwise
+ * available to attach by bpf_lsm or fmod_ret progs.
+ */
+static int check_sleepable_blacklist(unsigned long addr)
+{
+#ifdef CONFIG_BPF_LSM
+ if (addr == (long)bpf_lsm_task_free)
+ return -EINVAL;
+#endif
+#ifdef CONFIG_SECURITY
+ if (addr == (long)security_task_free)
+ return -EINVAL;
+#endif
+ return 0;
+}
Would be nice to have some sort of generic function annotation to describe
that code cannot sleep inside of it, and then filter based on that. Anyway,
is above from manual code inspection?
yep. all manual. I don't think there is a way to automate it.
At least I cannot think of one.
What about others like security_sock_rcv_skb() for example which could be
bh_lock_sock()'ed (or, generally hooks running in softirq context)?
ahh. it's in running in bh at that point? then it should be added to blacklist.
The rough idea I had is to try all lsm_* and security_* hooks with all
debug kernel flags and see which ones will complain. Then add them to blacklist.
Unfortunately I'm completely swamped and cannot promise to do that
in the coming months.
So either we wait for somebody to do due diligence or land it knowing
that blacklist is incomplete and fix it up one by one.
I think the folks who're waiting on sleepable work would prefer the latter.
I'm fine whichever way.
Chiming in since I belong to the folks who are waiting on sleepable BPF patches:
1. Let's obviously add security_sock_rcv_skb to the list.
2. I can help in combing through the LSM hooks (at least the comments)
to look for any other obvious candidates.
3. I think it's okay (for us) for this list to be a WIP and build on it with
proper warnings (in the changelog / comments).
4. To make it easier for figuring out which hooks cannot sleep,
It would be nice if we could:
* Have a helper say, bool bpf_cant_sleep(), available when
DEBUG_ATOMIC_SLEEP is enabled.
* Attach LSM programs to all hooks which call this helper and gather data.
* Let this run on dev machines, run workloads which use the LSM hooks .
4. Finally, once we do the hard work. We can also think of augmenting the
LSM_HOOK macro to have structured access to whether a hook is sleepable
or not (instead of relying on comments).
+1, I think augmenting mid-term would be the best given check_sleepable_blacklist()
is rather a very fragile workaround^hack and it's also a generic lsm/sec hooks issue
(at least for BPF_PROG_TYPE_LSM type & for the sake of documenting it for other LSMs).
Perhaps there are function attributes that could be used and later retrieved via BTF?
Thanks,
Daniel
