[Devel] [PATCH VZ9 1/2] fs/fuse/kio: corrupted cs_accel state variable
Pavel Tikhomirov
ptikhomirov at virtuozzo.com
Thu Oct 23 10:17:04 MSK 2025
and rh9-5.14.0-427.92.1.vz9.88.2
On 10/23/25 15:15, Pavel Tikhomirov wrote:
> 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