On 5/15/24 4:45 PM, libaokun@xxxxxxxxxxxxxxx wrote: > From: Baokun Li <libaokun1@xxxxxxxxxx> > > Even with CACHEFILES_DEAD set, we can still read the requests, so in the > following concurrency the request may be used after it has been freed: > > mount | daemon_thread1 | daemon_thread2 > ------------------------------------------------------------ > cachefiles_ondemand_init_object > cachefiles_ondemand_send_req > REQ_A = kzalloc(sizeof(*req) + data_len) > wait_for_completion(&REQ_A->done) > cachefiles_daemon_read > cachefiles_ondemand_daemon_read > // close dev fd > cachefiles_flush_reqs > complete(&REQ_A->done) > kfree(REQ_A) > xa_lock(&cache->reqs); > cachefiles_ondemand_select_req > req->msg.opcode != CACHEFILES_OP_READ > // req use-after-free !!! > xa_unlock(&cache->reqs); > xa_destroy(&cache->reqs) > > Hence remove requests from cache->reqs when flushing them to avoid > accessing freed requests. > > Fixes: c8383054506c ("cachefiles: notify the user daemon when looking up cookie") > Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx> > Reviewed-by: Jia Zhu <zhujia.zj@xxxxxxxxxxxxx> Reviewed-by: Jingbo Xu <jefflexu@xxxxxxxxxxxxxxxxx> > --- > fs/cachefiles/daemon.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c > index 6465e2574230..ccb7b707ea4b 100644 > --- a/fs/cachefiles/daemon.c > +++ b/fs/cachefiles/daemon.c > @@ -159,6 +159,7 @@ static void cachefiles_flush_reqs(struct cachefiles_cache *cache) > xa_for_each(xa, index, req) { > req->error = -EIO; > complete(&req->done); > + __xa_erase(xa, index); > } > xa_unlock(xa); > -- Thanks, Jingbo