From: Yun Lu bpf_wq_init() checks the map's usercnt but not whether its value is still linked. A program retaining a value pointer across deletion can publish and configure a new cb after deletion has already cancelled the element: reader delete / reclamation worker retain value E unlink E, cancel work publish cb, set and start leave read-side section finish E's grace period enter callback cancel_work(), free E access E The callback's trace read-side section is outside that grace period's waiting set. Keeping cb alive does not keep E alive, and the destructor's cancel_work() does not wait for the running callback. After publication and the existing usercnt check, look up the key and require the result to equal cb->value. Either lookup rejects the removed value or deletion finds the published cb. On rejection, cancel only this init's cb and return -ENOENT. Repeat membership checking in the setter and return -EINVAL on mismatch. Another CPU can use the published cb before init completes its checks, so checking only init would leave that configuration window open. Use pointer identity to reject an old value after same-key replacement. Enable these checks only for WQ here; timer follows separately. Fixes: eb48f6cd41a0 ("bpf: wq: add bpf_wq_init") Signed-off-by: Yun Lu --- kernel/bpf/helpers.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 1227debd3e8a..7bcb66c92abd 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1448,6 +1448,11 @@ static int __bpf_async_init(struct bpf_async_kern *async, struct bpf_map *map, u bpf_async_cancel_cb(async, cb); return -EPERM; } + if (type == BPF_ASYNC_TYPE_WQ && + !bpf_map_value_is_linked(map, cb->value)) { + bpf_async_cancel_cb(async, cb); + return -ENOENT; + } return 0; } @@ -1567,6 +1572,9 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback return -EINVAL; if (!atomic64_read(&map->usercnt)) return -EPERM; + if (cb->type == BPF_ASYNC_TYPE_WQ && + !bpf_map_value_is_linked(map, cb->value)) + return -EINVAL; return bpf_async_update_prog_callback(cb, prog, callback_fn); } @@ -3520,7 +3528,8 @@ __bpf_kfunc void bpf_throw(u64 cookie) * * Return: 0 on success; -EINVAL for invalid flags; -EBUSY if already * initialized; -ENOMEM on allocation failure; -EPERM if the map has no user - * reference. + * reference; -ENOENT if the current map lookup does not return this value + * when the membership check is reached. */ __bpf_kfunc int bpf_wq_init(struct bpf_wq *wq, void *p__const_map, unsigned int flags) { @@ -3570,8 +3579,8 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags) * Success does not prevent concurrent cancellation. * * Return: 0 on success; -EINVAL for invalid flags, if @wq was not initialized, - * or if its containing value was already invalidated; -EPERM if the map has - * no user references. + * if its containing value was already invalidated, or if the current map + * lookup does not return that value; -EPERM if the map has no user references. */ __bpf_kfunc int bpf_wq_set_callback(struct bpf_wq *wq, int (callback_fn)(void *map, int *key, void *value), -- 2.43.0