Re: [PATCH] libceph: fix client panic on MDS message with data but no data items

[Henry Martin]

Hi Alex,

Thanks for looking, but the skip you describe doesn't exist on the
MDS path.

mds_alloc_msg() (fs/ceph/mds_client.c:7384) leaves *skip = 0
unconditionally and never inspects hdr->data_len.  In v1,
read_partial_message() calls prepare_message_data() whenever the
on-wire data_len is non-zero (messenger_v1.c:1229); in v2,
__handle_control() routes any non-zero data_len to
prepare_read_data() (messenger_v2.c:1837).  Neither path compares
data_len to msg->data_length anywhere; I also found no such guard
in {mds,mon,osd}_client.c.

For an MDS message with data_len > 0 the first hit in
ceph_msg_data_cursor_init() is actually BUG_ON(length >
msg->data_length) with data_length == 0, one step before
BUG_ON(!msg->num_data_items); either way the client panics.

The check you have in mind is the OSD-side one, 98c4bfe9d89b2
("libceph: check reply num_data_items in setup_request_data()").
It validates reply items against the OSD request's preallocated
ops - MDS has no preallocated reply path at all, which is exactly
the sibling gap this patch closes.

Happy to move the rejection into mds_alloc_msg() (skip/reject
data_len > 0 there) if you prefer that over messenger-level
guards; I'd keep them since they also cover any future alloc_msg()
that allocates no items.

Regards,
Henry