RE: [PATCH v5] libceph, ceph: reject oversized mon and MDS data segments

[Viacheslav Dubeyko]

On Fri, 2026-05-08 at 18:18 +0000, Viacheslav Dubeyko wrote:
> On Thu, 2026-05-07 at 23:19 +0400, Dhiraj Mishra wrote:
> > Monitor and MDS messages can be allocated with only front-buffer storage
> > and no data items.  The messenger receive path copies the wire header
> > into the selected ceph_msg and later uses hdr.data_len to decide whether
> > to initialize a data cursor.
> > 
> > If a malicious or compromised peer advertises a non-zero data segment for
> > one of these front-only messages, the receive path can call
> > ceph_msg_data_cursor_init() with length greater than msg->data_length and
> > hit its BUG_ON() checks, crashing the client kernel.
> > 
> > I verified the monitor issue against v7.1-rc1-123-ge75a43c7cec4.  The
> > msgr2 trigger path is present since commit cd1a677cad99 ("libceph,
> > ceph: implement msgr2.1 protocol (crc and secure modes)"), which is
> > contained in v5.11-rc1 and later.  The allocator patterns are older, but
> > I have not tested older msgr1-only kernels.
> > 
> > A concrete monitor trigger is a monitor connection over msgr2 after
> > CEPH_CON_S_OPEN where a FRAME_TAG_MESSAGE contains a monitor reply type
> > handled by mon_alloc_msg(), a valid front_len for that message type and
> > data_len = 1.  CEPH_MSG_MON_MAP is one such example: the message is
> > allocated with ceph_msg_new(), leaving msg->data_length and
> > msg->num_data_items as zero.
> > 
> > The MDS allocation path has the same issue: mds_alloc_msg() allocates the
> > incoming message with ceph_msg_new(type, front_len, GFP_NOFS, false),
> > leaving msg->data_length as zero for a front-only message selected from
> > an attacker-controlled wire header.
> > 
> > Reject monitor and MDS messages whose wire data segment is larger than
> > the data backing allocated for the selected ceph_msg, mirroring the
> > existing OSD reply hardening.
> > 
> > Fixes: 53ded495c6ac ("libceph: define mds_alloc_msg() method")
> > Fixes: cd1a677cad99 ("libceph, ceph: implement msgr2.1 protocol (crc and secure modes)")
> > Signed-off-by: Dhiraj Mishra <mishra.dhiraj95@xxxxxxxxx>
> > ---
> > v5:
> >   - Restructure mon_alloc_msg() control flow per review.
> >   - Return early for unknown types and front-buffer reallocation failure.
> >   - Keep the data_len guard on the final message allocation.
> > 
> > v4:
> >   - Add the same oversized data_len guard to the MDS allocation path.
> > 
> > v3:
> >   - Remove the impossible !req->reply check for generic requests.
> >   - Use pr_warn_ratelimited() for malicious-monitor log spam resistance.
> >   - Avoid adding __func__ to the new mon_client warnings.
> >   - Add a blank line between the front_len and data_len checks.
> > 
> > v2:
> >   - Resend as an inline plain-text patch.
> >   - Add full email address to the From and Signed-off-by identities.
> >   - Add ceph-devel and LKML to the recipient list when sending.
> > 
> >  fs/ceph/mds_client.c  | 16 ++++++++++++++++
> >  net/ceph/mon_client.c | 24 +++++++++++++++++++++++-
> >  2 files changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index ed17e0023705e6d4e387ff7e5bd7fb53efdbaeb7..c84d12384717bbcaad369f353b00ccb8a7f2fcb0 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -6543,13 +6543,19 @@ static int mds_handle_auth_bad_method(struct ceph_connection *con,
> >  static struct ceph_msg *mds_alloc_msg(struct ceph_connection *con,
> >  				struct ceph_msg_header *hdr, int *skip)
> >  {
> > +	struct ceph_mds_session *s;
> > +	struct ceph_client *cl;
> >  	struct ceph_msg *msg;
> >  	int type = (int) le16_to_cpu(hdr->type);
> >  	int front_len = (int) le32_to_cpu(hdr->front_len);
> > +	u32 data_len = le32_to_cpu(hdr->data_len);
> >  
> >  	if (con->in_msg)
> >  		return con->in_msg;
> >  
> > +	s = con->private;
> > +	cl = s->s_mdsc->fsc->client;
> > +
> >  	*skip = 0;
> >  	msg = ceph_msg_new(type, front_len, GFP_NOFS, false);
> >  	if (!msg) {
> > @@ -6558,6 +6564,16 @@ static struct ceph_msg *mds_alloc_msg(struct ceph_connection *con,
> >  		return NULL;
> >  	}
> >  
> > +	if (data_len > msg->data_length) {
> > +		pr_warn_ratelimited_client(cl,
> > +					   "mds%d message data %u > prealloc %zu, skipping\n",
> > +					   s->s_mds, data_len,
> > +					   msg->data_length);
> > +		ceph_msg_put(msg);
> > +		*skip = 1;
> > +		return NULL;
> > +	}
> > +
> >  	return msg;
> >  }
> >  
> > diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c
> > index d2cdc8ee31551eb99ad062f191ac3cf984970130..66c0d6d6b33d37a25141117845b0f804664540ef 100644
> > --- a/net/ceph/mon_client.c
> > +++ b/net/ceph/mon_client.c
> > @@ -712,6 +712,7 @@ static struct ceph_msg *get_generic_reply(struct ceph_connection *con,
> >  	struct ceph_mon_client *monc = con->private;
> >  	struct ceph_mon_generic_request *req;
> >  	u64 tid = le64_to_cpu(hdr->tid);
> > +	u32 data_len = le32_to_cpu(hdr->data_len);
> >  	struct ceph_msg *m;
> >  
> >  	mutex_lock(&monc->mutex);
> > @@ -720,6 +721,11 @@ static struct ceph_msg *get_generic_reply(struct ceph_connection *con,
> >  		dout("get_generic_reply %lld dne\n", tid);
> >  		*skip = 1;
> >  		m = NULL;
> > +	} else if (data_len > req->reply->data_length) {
> > +		pr_warn_ratelimited("mon generic reply tid %llu data %u > preallocated %zu, skipping\n",
> > +				    tid, data_len, req->reply->data_length);
> > +		*skip = 1;
> > +		m = NULL;
> >  	} else {
> >  		dout("get_generic_reply %lld got %p\n", tid, req->reply);
> >  		*skip = 0;
> > @@ -1499,6 +1505,7 @@ static struct ceph_msg *mon_alloc_msg(struct ceph_connection *con,
> >  	struct ceph_mon_client *monc = con->private;
> >  	int type = le16_to_cpu(hdr->type);
> >  	int front_len = le32_to_cpu(hdr->front_len);
> > +	u32 data_len = le32_to_cpu(hdr->data_len);
> >  	struct ceph_msg *m = NULL;
> >  
> >  	*skip = 0;
> > @@ -1536,13 +1543,28 @@ static struct ceph_msg *mon_alloc_msg(struct ceph_connection *con,
> >  	if (!m) {
> >  		pr_info("alloc_msg unknown type %d\n", type);
> >  		*skip = 1;
> > -	} else if (front_len > m->front_alloc_len) {
> > +		return m;
> > +	}
> > +
> > +	if (front_len > m->front_alloc_len) {
> >  		pr_warn("mon_alloc_msg front %d > prealloc %d (%u#%llu)\n",
> >  			front_len, m->front_alloc_len,
> >  			(unsigned int)con->peer_name.type,
> >  			le64_to_cpu(con->peer_name.num));
> >  		ceph_msg_put(m);
> >  		m = ceph_msg_new(type, front_len, GFP_NOFS, false);
> > +		if (!m)
> > +			return m;
> > +	}
> > +
> > +	if (data_len > m->data_length) {
> > +		pr_warn_ratelimited("mon message data %u > prealloc %zu (%u#%llu), skipping\n",
> > +				    data_len, m->data_length,
> > +				    (unsigned int)con->peer_name.type,
> > +				    le64_to_cpu(con->peer_name.num));
> > +		ceph_msg_put(m);
> > +		m = NULL;
> > +		*skip = 1;
> >  	}
> >  
> >  	return m;
> 
> Looks good.
> 
> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
> 
> Let me run xfstests suite for the patch. I'll be back with the results ASAP.
> 
> 

The xfstests run was successful. I don't see any issues with the patch.

Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>

Thanks,
Slava.