cachefiles: add spin_lock for cachefiles_ondemand_info

The following concurrency may cause a read request to fail to be completed
and result in a hung:

           t1             |             t2
---------------------------------------------------------
                            cachefiles_ondemand_copen
                              req = xa_erase(&cache->reqs, id)
// Anon fd is maliciously closed.
cachefiles_ondemand_fd_release
  xa_lock(&cache->reqs)
  cachefiles_ondemand_set_object_close(object)
  xa_unlock(&cache->reqs)
                              cachefiles_ondemand_set_object_open
                              // No one will ever close it again.
cachefiles_ondemand_daemon_read
  cachefiles_ondemand_select_req
  // Get a read req but its fd is already closed.
  // The daemon can't issue a cread ioctl with an closed fd, then hung.

So add spin_lock for cachefiles_ondemand_info to protect ondemand_id and
state, thus we can avoid the above problem in cachefiles_ondemand_copen()
by using ondemand_id to determine if fd has been closed.

Fixes: c838305450 ("cachefiles: notify the user daemon when looking up cookie")
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Link: https://lore.kernel.org/r/20240522114308.2402121-8-libaokun@huaweicloud.com
Acked-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
This commit is contained in:
Baokun Li 2024-05-22 19:43:03 +08:00 committed by Christian Brauner
parent a26dc49df3
commit 0a79004083
No known key found for this signature in database
GPG Key ID: 91C61BC06578DCA2
2 changed files with 35 additions and 1 deletions

View File

@ -55,6 +55,7 @@ struct cachefiles_ondemand_info {
int ondemand_id;
enum cachefiles_object_state state;
struct cachefiles_object *object;
spinlock_t lock;
};
/*

View File

@ -16,13 +16,16 @@ static int cachefiles_ondemand_fd_release(struct inode *inode,
struct cachefiles_object *object = file->private_data;
struct cachefiles_cache *cache = object->volume->cache;
struct cachefiles_ondemand_info *info = object->ondemand;
int object_id = info->ondemand_id;
int object_id;
struct cachefiles_req *req;
XA_STATE(xas, &cache->reqs, 0);
xa_lock(&cache->reqs);
spin_lock(&info->lock);
object_id = info->ondemand_id;
info->ondemand_id = CACHEFILES_ONDEMAND_ID_CLOSED;
cachefiles_ondemand_set_object_close(object);
spin_unlock(&info->lock);
/* Only flush CACHEFILES_REQ_NEW marked req to avoid race with daemon_read */
xas_for_each_marked(&xas, req, ULONG_MAX, CACHEFILES_REQ_NEW) {
@ -127,6 +130,7 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args)
{
struct cachefiles_req *req;
struct fscache_cookie *cookie;
struct cachefiles_ondemand_info *info;
char *pid, *psize;
unsigned long id;
long size;
@ -185,6 +189,33 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args)
goto out;
}
info = req->object->ondemand;
spin_lock(&info->lock);
/*
* The anonymous fd was closed before copen ? Fail the request.
*
* t1 | t2
* ---------------------------------------------------------
* cachefiles_ondemand_copen
* req = xa_erase(&cache->reqs, id)
* // Anon fd is maliciously closed.
* cachefiles_ondemand_fd_release
* xa_lock(&cache->reqs)
* cachefiles_ondemand_set_object_close(object)
* xa_unlock(&cache->reqs)
* cachefiles_ondemand_set_object_open
* // No one will ever close it again.
* cachefiles_ondemand_daemon_read
* cachefiles_ondemand_select_req
*
* Get a read req but its fd is already closed. The daemon can't
* issue a cread ioctl with an closed fd, then hung.
*/
if (info->ondemand_id == CACHEFILES_ONDEMAND_ID_CLOSED) {
spin_unlock(&info->lock);
req->error = -EBADFD;
goto out;
}
cookie = req->object->cookie;
cookie->object_size = size;
if (size)
@ -194,6 +225,7 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args)
trace_cachefiles_ondemand_copen(req->object, id, size);
cachefiles_ondemand_set_object_open(req->object);
spin_unlock(&info->lock);
wake_up_all(&cache->daemon_pollwq);
out:
@ -596,6 +628,7 @@ int cachefiles_ondemand_init_obj_info(struct cachefiles_object *object,
return -ENOMEM;
object->ondemand->object = object;
spin_lock_init(&object->ondemand->lock);
INIT_WORK(&object->ondemand->ondemand_work, ondemand_object_worker);
return 0;
}