[Devel] [PATCH VZ9 1/2] fs/fuse/kio: corrupted cs_accel state variable
Pavel Tikhomirov
ptikhomirov at virtuozzo.com
Thu Oct 23 10:15:27 MSK 2025
Comited to rh10-6.12.0-55.13.1.2.12.vz10
On 10/20/25 16:34, Alexey Kuznetsov wrote:
> Variable parent_N shared between states was put to data private
> to one particular state and overwritten while state switches.
>
> The core reason for this sad mistake was aliasing between anonymous
> union member and the same member looking like it is common.
> It was obvious this is a way to trouble, but desire to minimize
> patch size was overwhelming. Fix this.
>
> Affects: #VSTOR-109945
>
> Signed-off-by: Alexey Kuznetsov <kuznet at virtuozzo.com>
> ---
> fs/fuse/kio/pcs/pcs_cluster.c | 8 ++++----
> fs/fuse/kio/pcs/pcs_cs.c | 18 +++++++++---------
> fs/fuse/kio/pcs/pcs_cs_accel.c | 12 ++++++------
> fs/fuse/kio/pcs/pcs_map.c | 10 +++++-----
> fs/fuse/kio/pcs/pcs_req.h | 12 ++----------
> 5 files changed, 26 insertions(+), 34 deletions(-)
>
> diff --git a/fs/fuse/kio/pcs/pcs_cluster.c b/fs/fuse/kio/pcs/pcs_cluster.c
> index 710087c..c475d04 100644
> --- a/fs/fuse/kio/pcs/pcs_cluster.c
> +++ b/fs/fuse/kio/pcs/pcs_cluster.c
> @@ -245,8 +245,8 @@ static void fiemap_process_one(struct fiemap_iterator *fiter)
> fiter->apireq.size = sreq->iochunk.size = DENTRY_CHUNK_SIZE(di) - sreq->iochunk.offset;
> sreq->iochunk.csl = NULL;
> sreq->iochunk.banned_cs.val = 0;
> - sreq->iochunk.msg.destructor = NULL;
> - sreq->iochunk.msg.rpc = NULL;
> + sreq->iochunk.ir.msg.destructor = NULL;
> + sreq->iochunk.ir.msg.rpc = NULL;
>
> pcs_sreq_attach(sreq, &fiter->ireq);
> sreq->complete_cb = pcs_sreq_complete;
> @@ -403,8 +403,8 @@ static noinline void __pcs_cc_process_ireq_rw(struct pcs_int_request *ireq)
> sreq->iochunk.size = len;
> sreq->iochunk.csl = NULL;
> sreq->iochunk.banned_cs.val = 0;
> - sreq->iochunk.msg.destructor = NULL;
> - sreq->iochunk.msg.rpc = NULL;
> + sreq->iochunk.ir.msg.destructor = NULL;
> + sreq->iochunk.ir.msg.rpc = NULL;
>
> pcs_sreq_attach(sreq, ireq);
> sreq->complete_cb = pcs_sreq_complete;
> diff --git a/fs/fuse/kio/pcs/pcs_cs.c b/fs/fuse/kio/pcs/pcs_cs.c
> index 8c1fdce..53bd520 100644
> --- a/fs/fuse/kio/pcs/pcs_cs.c
> +++ b/fs/fuse/kio/pcs/pcs_cs.c
> @@ -381,7 +381,7 @@ static void cs_response_done(struct pcs_msg *msg)
> pcs_map_verify_sync_state(ireq->dentry, ireq, msg);
> } else {
> FUSE_KTRACE(ireq->cc->fc, XID_FMT " IO error %d %lu, ireq:%p : %llu:%u+%u",
> - XID_ARGS(ireq->iochunk.hbuf.hdr.xid), msg->error.value,
> + XID_ARGS(ireq->iochunk.ir.hbuf.hdr.xid), msg->error.value,
> msg->error.remote ? (unsigned long)msg->error.offender.val : 0UL,
> ireq, (unsigned long long)ireq->iochunk.chunk,
> (unsigned)ireq->iochunk.offset,
> @@ -488,9 +488,10 @@ static void cs_get_data(struct pcs_msg *msg, int offset, struct iov_iter *it, un
> struct pcs_int_request *ireq = ireq_from_msg(msg);
>
> if (offset < sizeof(struct pcs_cs_iohdr)) {
> - ireq->iochunk.hbuf_kv.iov_base = &ireq->iochunk.hbuf;
> - ireq->iochunk.hbuf_kv.iov_len = sizeof(struct pcs_cs_iohdr);
> - iov_iter_kvec(it, direction, &ireq->iochunk.hbuf_kv, 1, sizeof(struct pcs_cs_iohdr));
> + ireq->iochunk.ir.hbuf_kv.iov_base = &ireq->iochunk.ir.hbuf;
> + ireq->iochunk.ir.hbuf_kv.iov_len = sizeof(struct pcs_cs_iohdr);
> + iov_iter_kvec(it, direction, &ireq->iochunk.ir.hbuf_kv, 1,
> + sizeof(struct pcs_cs_iohdr));
> iov_iter_advance(it, offset);
> TRACE("return msg:%p->size:%d off:%d it_len:%ld\n", msg, msg->size, offset, iov_iter_count(it));
>
> @@ -624,8 +624,8 @@ static void ireq_complete_fo(struct pcs_int_request * ireq)
> }
> FUSE_TRACE_COMMIT(fc->ktrace);
> }
> - ireq->iochunk.msg.destructor = NULL;
> - ireq->iochunk.msg.rpc = NULL;
> + ireq->iochunk.ir.msg.destructor = NULL;
> + ireq->iochunk.ir.msg.rpc = NULL;
> ireq_complete(ireq);
> }
>
> @@ -653,7 +653,7 @@ static void complete_fo_request(struct pcs_int_request * sreq)
>
> static void do_cs_submit(struct pcs_cs *cs, struct pcs_int_request *ireq)
> {
> - struct pcs_msg *msg = &ireq->iochunk.msg;
> + struct pcs_msg *msg = &ireq->iochunk.ir.msg;
> struct pcs_cs_iohdr *ioh;
> struct pcs_cs_list *csl = ireq->iochunk.csl;
> struct pcs_map_entry *map = ireq->iochunk.map; /* ireq keeps reference to map */
> @@ -665,7 +665,7 @@ static void do_cs_submit(struct pcs_cs *cs, struct pcs_int_request *ireq)
> msg->private = cs;
> msg->private2 = ireq;
>
> - ioh = &ireq->iochunk.hbuf;
> + ioh = &ireq->iochunk.ir.hbuf;
> ioh->hdr.len = pcs_cs_msg_size(sizeof(struct pcs_cs_iohdr),
> storage_version);
> aligned_msg = pcs_cs_use_aligned_io(storage_version);
> @@ -741,7 +741,7 @@ static void do_cs_submit(struct pcs_cs *cs, struct pcs_int_request *ireq)
>
> DTRACE(XID_FMT " About to send msg:%p, ireq:%p, cmd:%u,"
> " id:"CUID_FMT" v:"VER_FMT" - %llu:%u+%u\n",
> - XID_ARGS(ireq->iochunk.hbuf.hdr.xid), msg, ireq,
> + XID_ARGS(ireq->iochunk.ir.hbuf.hdr.xid), msg, ireq,
> ireq->iochunk.cmd, CUID_ARGS(ioh->uid),
> VER_ARGS(ioh->map_version),
> (unsigned long long)ireq->iochunk.chunk,
> diff --git a/fs/fuse/kio/pcs/pcs_cs_accel.c b/fs/fuse/kio/pcs/pcs_cs_accel.c
> index 036ca17..ef30ac8 100644
> --- a/fs/fuse/kio/pcs/pcs_cs_accel.c
> +++ b/fs/fuse/kio/pcs/pcs_cs_accel.c
> @@ -510,8 +510,8 @@ static void __pcs_csa_final_completion(struct pcs_aio_req *areq)
> /* Prepare ireq for restart in slow path */
> ireq->flags |= IREQ_F_NO_ACCEL|IREQ_F_ACCELERROR;
> ireq->flags &= ~IREQ_F_ONCE;
> - ireq->iochunk.msg.destructor = NULL;
> - ireq->iochunk.msg.rpc = NULL;
> + ireq->iochunk.ir.msg.destructor = NULL;
> + ireq->iochunk.ir.msg.rpc = NULL;
> }
>
> out:
> @@ -954,8 +954,8 @@ int pcs_csa_cs_submit(struct pcs_cs * cs, struct pcs_int_request * ireq)
> return 1;
> rcu_read_lock();
> /* Clear state which could be rewritten by csa_submit */
> - ireq->iochunk.msg.destructor = NULL;
> - ireq->iochunk.msg.rpc = NULL;
> + ireq->iochunk.ir.msg.destructor = NULL;
> + ireq->iochunk.ir.msg.rpc = NULL;
> ireq->flags |= IREQ_F_NO_ACCEL;
> }
> }
> @@ -1041,8 +1041,8 @@ static void ireq_clear_acr(struct pcs_int_request * ireq)
> ireq->iochunk.acr.awr[i].bvec_copy = NULL;
> }
> }
> - ireq->iochunk.msg.destructor = NULL;
> - ireq->iochunk.msg.rpc = NULL;
> + ireq->iochunk.ir.msg.destructor = NULL;
> + ireq->iochunk.ir.msg.rpc = NULL;
> ireq->flags |= IREQ_F_NO_ACCEL;
> }
>
> diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
> index 62b083b..2d95dc2 100644
> --- a/fs/fuse/kio/pcs/pcs_map.c
> +++ b/fs/fuse/kio/pcs/pcs_map.c
> @@ -1371,7 +1371,7 @@ void map_notify_iochunk_error(struct pcs_int_request * sreq)
> if (!m || (m->state & PCS_MAP_DEAD))
> return;
>
> - map_notify_error(m, sreq, &sreq->iochunk.hbuf.map_version, sreq->iochunk.csl);
> + map_notify_error(m, sreq, &sreq->iochunk.ir.hbuf.map_version, sreq->iochunk.csl);
> }
>
> static void map_replicating(struct pcs_int_request *ireq)
> @@ -1770,7 +1770,8 @@ void map_notify_soft_error(struct pcs_int_request *ireq)
> if (pcs_if_error(&ireq->error))
> map_notify_iochunk_error(ireq);
>
> - if (map_version_compare(&ireq->iochunk.hbuf.map_version, &ireq->iochunk.map->version) < 0)
> + if (map_version_compare(&ireq->iochunk.ir.hbuf.map_version,
> + &ireq->iochunk.map->version) < 0)
> ireq->flags &= ~IREQ_F_ONCE;
>
> pcs_deaccount_ireq(ireq, &err);
> @@ -1973,8 +1973,8 @@ struct pcs_int_request *
> sreq->iochunk.csl = NULL;
> sreq->iochunk.banned_cs.val = 0;
> sreq->complete_cb = ireq->complete_cb;
> - sreq->iochunk.msg.destructor = NULL;
> - sreq->iochunk.msg.rpc = NULL;
> + sreq->iochunk.ir.msg.destructor = NULL;
> + sreq->iochunk.ir.msg.rpc = NULL;
> pcs_sreq_attach(sreq, ireq->completion_data.parent);
>
> ireq->iochunk.size -= iochunk;
> @@ -2472,7 +2472,7 @@ void map_submit(struct pcs_map_entry * m, struct pcs_int_request *ireq)
>
> spin_lock(&m->lock);
> if (ireq->type == PCS_IREQ_IOCHUNK && !(ireq->flags & IREQ_F_MAPPED))
> - ireq->iochunk.hbuf.map_version = m->version;
> + ireq->iochunk.ir.hbuf.map_version = m->version;
>
> if (!(m->state & (1 << direction)) || (m->state & PCS_MAP_DEAD) ||
> map_chk_stale(m)) {
> diff --git a/fs/fuse/kio/pcs/pcs_req.h b/fs/fuse/kio/pcs/pcs_req.h
> index 94fcfdb..61d3575 100644
> --- a/fs/fuse/kio/pcs/pcs_req.h
> +++ b/fs/fuse/kio/pcs/pcs_req.h
> @@ -93,7 +93,6 @@ struct pcs_iochunk_req {
> * A little ugly
> */
> struct kvec hbuf_kv;
> - struct pcs_int_request *parent_N;
> };
>
> struct pcs_fo_req
> @@ -183,15 +182,8 @@ struct pcs_int_request
> u64 offset;
> struct pcs_cs_list *csl;
> PCS_NODE_ID_T banned_cs;
> + struct pcs_int_request *parent_N;
> union {
> - struct {
> - struct pcs_msg msg;
> - struct pcs_cs_iohdr hbuf; /* Buffer for header.
> - * A little ugly
> - */
> - struct kvec hbuf_kv;
> - struct pcs_int_request *parent_N;
> - };
> struct pcs_iochunk_req ir;
> struct pcs_fo_req fo;
> struct pcs_aio_req ar;
> @@ -383,7 +375,7 @@ static inline int pcs_sreq_detach(struct pcs_int_request * sreq)
>
> static inline struct pcs_int_request *ireq_from_msg(struct pcs_msg *msg)
> {
> - return container_of(msg, struct pcs_int_request, iochunk.msg);
> + return container_of(msg, struct pcs_int_request, iochunk.ir.msg);
> }
>
> static inline void ireq_process(struct pcs_int_request *ireq)
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
More information about the Devel
mailing list