[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28d64f00-e408-9fc2-9506-63c1d8b08b9c@linux.alibaba.com>
Date: Wed, 12 Oct 2022 15:53:13 +0800
From: JeffleXu <jefflexu@...ux.alibaba.com>
To: Jia Zhu <zhujia.zj@...edance.com>, dhowells@...hat.com,
xiang@...nel.org
Cc: linux-cachefs@...hat.com, linux-erofs@...ts.ozlabs.org,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
yinxin.x@...edance.com
Subject: Re: [PATCH 3/5] cachefiles: resend an open request if the read
request's object is closed
On 10/11/22 9:15 PM, Jia Zhu wrote:
> @@ -254,12 +282,18 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
> * request distribution fair.
> */
> xa_lock(&cache->reqs);
> - req = xas_find_marked(&xas, UINT_MAX, CACHEFILES_REQ_NEW);
> - if (!req && cache->req_id_next > 0) {
> - xas_set(&xas, 0);
> - req = xas_find_marked(&xas, cache->req_id_next - 1, CACHEFILES_REQ_NEW);
> +retry:
> + xas_for_each_marked(&xas, req, xa_max, CACHEFILES_REQ_NEW) {
> + if (cachefiles_ondemand_skip_req(req))
> + continue;
> + break;
> }
> if (!req) {
> + if (cache->req_id_next > 0 && xa_max == ULONG_MAX) {
> + xas_set(&xas, 0);
> + xa_max = cache->req_id_next - 1;
> + goto retry;
> + }
I would suggest abstracting the "xas_for_each_marked(...,
CACHEFILES_REQ_NEW)" part into a helper function to avoid the "goto retry".
> @@ -392,8 +434,16 @@ static int cachefiles_ondemand_send_req(struct cachefiles_object *object,
> wake_up_all(&cache->daemon_pollwq);
> wait_for_completion(&req->done);
> ret = req->error;
> + kfree(req);
> + return ret;
> out:
> kfree(req);
> + /* Reset the object to close state in error handling path.
> + * If error occurs after creating the anonymous fd,
> + * cachefiles_ondemand_fd_release() will set object to close.
> + */
> + if (opcode == CACHEFILES_OP_OPEN)
> + cachefiles_ondemand_set_object_close(req->object);
This may cause use-after-free since @req has been freed.
--
Thanks,
Jingbo
Powered by blists - more mailing lists