Re: [PATCH] libceph: Reject osdmaps advertising pools of unsupported types

On 11.06.26 6:07 AM, Viacheslav Dubeyko wrote:
> On Wed, 2026-06-10 at 20:53 -0700, Viacheslav Dubeyko wrote:
>> CC: [email protected]
>>
>> On Mon, 2026-06-08 at 17:13 +0200, Raphael Zimmer wrote:
>>> osdmaps are received over the network and typically contain
>>> information
>>> about existing pools. Information about each pool is decoded in
>>> decode_pool(). A pool is either replicated or erasure coded, which
>>> is
>>> encoded in the type field as an 8-bit value. Currently, any value
>>> in
>>> this field is treated as valid even though only two values have an
>>> actual meaning. If this field contains a value other than
>>> CEPH_POOL_TYPE_REP or CEPH_POOL_TYPE_EC, a BUG() assertion in
>>> ceph_can_shift_osds() can be triggered subsequently. This happens
>>> the
>>> next time calc_target() is called for a request in this pool.
>>>
>>> This patch fixes the issue by adding a check that verifies the pool
>>> type
>>> directly during decoding and rejecting osdmaps that contain invalid
>>> values.
>>>
>>> Fixes: 4f6a7e5ee139 ("ceph: update support for PGID64, PGPOOL3,
>>> OSDENC protocol features")
>>> Signed-off-by: Raphael Zimmer <[email protected]>
>>> ---
>>>  include/linux/ceph/rados.h | 5 +++++
>>>  net/ceph/osdmap.c          | 4 ++++
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/ceph/rados.h
>>> b/include/linux/ceph/rados.h
>>> index 73c3efbec36c..5b5c33845301 100644
>>> --- a/include/linux/ceph/rados.h
>>> +++ b/include/linux/ceph/rados.h
>>> @@ -86,6 +86,11 @@ struct ceph_pg_v1 {
>>>  #define CEPH_POOL_TYPE_RAID4   2 /* never implemented */
>>>  #define CEPH_POOL_TYPE_EC      3
>>>  
>>> +static inline bool ceph_pool_type_is_supported(__u8 type)
>>> +{
>>> +	return type == CEPH_POOL_TYPE_REP || type ==
>>> CEPH_POOL_TYPE_EC;
>>> +}
>>> +
>>>  /*
>>>   * stable_mod func is used to control number of placement groups.
>>>   * similar to straight-up modulo, but produces a stable mapping as
>>> b
>>> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
>>> index f8d5621bc767..e440f6019d58 100644
>>> --- a/net/ceph/osdmap.c
>>> +++ b/net/ceph/osdmap.c
>>> @@ -818,6 +818,10 @@ static int decode_pool(void **p, void *end,
>>> struct ceph_pg_pool_info *pi)
>>>  
>>>  	ceph_decode_need(p, end, 4 + 4 + 4, bad);
>>>  	pi->type = ceph_decode_8(p);
>>> +	if (!ceph_pool_type_is_supported(pi->type)) {
>>> +		pr_warn_ratelimited("got unsupported pool type
>>> %u",
>>> pi->type);
>>> +		return -EINVAL;
>>> +	}
>>>  	pi->size = ceph_decode_8(p);
>>>  	pi->crush_ruleset = ceph_decode_8(p);
>>>  	pi->object_hash = ceph_decode_8(p);
> 
> Makes sense.
> 
> Reviewed-by: Viacheslav Dubeyko <[email protected]>
> 
> Thanks,
> Slava.

Hi,

I noticed that this solution does not cover all trigger paths for this
BUG() assertion. It fixes the problem for full maps. In the case of an
incremental map, the update is performed in-place. This means that by
the time, the "!ceph_pool_type_is_supported(pi->type)" check triggers,
pi->type is already set to the unsupported type in the current osdmap.
Therefore, the BUG() can still be triggered. I suggest also removing the
partially decoded (corrupted) pool from the osdmap again in
__decode_pools() when decode_pool() returns an error. This fixes the
issue for incremental maps and further ensures that no otherwise
corrupted pools remain in the current osdmap after their decoding failed.

I would suggest adding this logic to this patch, as it has not yet been
applied upstream. Therefore, I will send a v2 of this patch.

Best regards,
Raphael