Re: [PATCH v2] libceph: Assign requests to homeless osd if calculated osd exceeds max_osd

On 27.07.26 12:35 PM, Ilya Dryomov wrote:
> On Wed, Jul 1, 2026 at 5:56 PM Raphael Zimmer
> <[email protected]> wrote:
>>
>> A corrupted osdmap received from a Ceph monitor or OSD may contain
>> placement groups with osd indices that don't exist, i.e., that are
>> greater than max_osd or smaller than CEPH_HOMELESS_OSD (-1).
> 
> Hi Raphael,
> 
> The osdmap doesn't contain all placement group mappings -- they are
> produced on demand by CRUSH based off of the crushmap that is embedded
> in the osdmap but doesn't contain any OSD indices.  There are a few
> (usually small or empty) exception tables in the form of pg_temp,
> primary_temp, pg_upmap and pg_upmap_items where the "override" OSD
> indices are stored.  Are those what you have in mind wrt. osdmap
> corruption?
> 

Hi Ilya,

Yes, it's about these. I looked into it again after submitting this
patch and concluded that the problematic part is specifically
primary_temp. In the functions called from ceph_pg_to_up_acting_osds(),
non-existent osds are removed from the osd sets. Therefore, the up set
will not contain any invalid osds afterwards and will also have a valid
primary, as it is always one of the osds in the set. Similarly,
non-existent osds are removed from the acting set. However, the primary
osd is directly assigned from pg->primary_temp.osd in get_temp_osds(),
without checking for its existence. This way, an invalid osd index can
be returned as target osd from calc_target().

Therefore, it would suffice to add the check at this position (in
get_temp_osds()) and set the primary osd to the homeless osd in this
case (or perhaps to another osd from the acting set, but this doesn't
seem like a good solution to me, as the osdmap may be corrupted even
more and other osd mappings are also incorrect).

> If so, I'd suggest adding map->max_osd checks to the corresponding
> decode routines.  Munging ct_res to CALC_TARGET_NEED_RESEND as done in
> this patch doesn't make much sense -- you are asking the OSD client to
> arrange for the OSD request to be resent when there is no valid OSD to
> send it to (and no expectation whatsoever that the resend's invocation
> of calc_target() would produce a different result).
> 

I don't completely agree on this matter. I specifically reassign the
request to the homeless osd here and don't ask for resending it to an
invalid one. This will result in not sending it out. __submit_request()
only sends it if !osd_homeless(osd). The same holds true for
kick_requests(). For linger requests, send_linger() also ultimately
calls __submit_request(), leading to the same behavior. Finally,
recalc_linger_target() only re-links the request to the homeless osd if
calc_target() returns CALC_TARGET_NEED_RESEND.
If applying the solution described above (checking in get_temp_osds()),
this would furthermore only happen once.

Adding map->max_osd checks to the corresponding decode routines would
also be possible. I initially decided not to do this because the
proposed solution needs fewer code changes and still resolves the issue
of a potential out-of-bounds access. If we wanted to add checks to the
decode functions, this should be done in all of them for consistency.
That would mean we need to change multiple functions, including their
signatures, to pass the map->max_osd into them.

Do you still think that checks should be added to the decode functions,
or would a check for either ceph_osd_is_down(pg->primary_temp.osd) or
!ceph_osd_exists(pg->primary_temp.osd) in get_temp_osds() be an
acceptable option too?

I can send a new patch with the preferred solution.

Best regards,
Raphael

> Thanks,
> 
>                 Ilya
> 
>> Subsequently, this may lead to calc_target() returning such an index as
>> target osd for a (linger) request. Because the osd_state, osd_weight,
>> and osd_addr arrays only contain max_osd entries (with indices 0 to
>> max_osd -1), this leads to out-of-bounds accesses when trying to read
>> values from these arrays.
>>
>> This patch fixes the issue by adding a check to calc_target() assigning
>> the request to the homeless osd if the target osd index read from the
>> osdmap falls outside the valid osd index range.
>>
>> Fixes: 63244fa123a7 ("libceph: introduce ceph_osd_request_target, calc_target()")
>> Signed-off-by: Raphael Zimmer <[email protected]>
>> ---
>>  net/ceph/osd_client.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
>> index a4d3cbfd27a3..3d13226a5d1f 100644
>> --- a/net/ceph/osd_client.c
>> +++ b/net/ceph/osd_client.c
>> @@ -1707,6 +1707,12 @@ static enum calc_target_result calc_target(struct ceph_osd_client *osdc,
>>                 }
>>         }
>>
>> +       if (t->osd != CEPH_HOMELESS_OSD && (u32)t->osd >= osdc->osdmap->max_osd) {
>> +               t->osd = CEPH_HOMELESS_OSD;
>> +               ct_res = CALC_TARGET_NEED_RESEND;
>> +               goto out;
>> +       }
>> +
>>         if (unpaused || legacy_change || force_resend || split)
>>                 ct_res = CALC_TARGET_NEED_RESEND;
>>         else
>> --
>> 2.47.3
>>