On 04.08.2025 18:57, Stewart Hildebrand wrote:
> On 8/4/25 03:57, Jan Beulich wrote:
>> On 01.08.2025 22:24, Stewart Hildebrand wrote:
>>> @@ -839,6 +839,9 @@ void sched_destroy_vcpu(struct vcpu *v)
>>> {
>>> struct sched_unit *unit = v->sched_unit;
>>>
>>> + if ( !unit )
>>> + return;
>>> +
>>> kill_timer(&v->periodic_timer);
>>> kill_timer(&v->singleshot_timer);
>>> kill_timer(&v->poll_timer);
>>
>> What if it's the 2nd error path in sched_init_vcpu() that is taken? Then we
>> might take this path (just out of context here)
>>
>> if ( unit->vcpu_list == v )
>> {
>> rcu_read_lock(&sched_res_rculock);
>>
>> sched_remove_unit(vcpu_scheduler(v), unit);
>> sched_free_udata(vcpu_scheduler(v), unit->priv);
>>
>> and at least Credit1's hook doesn't look to be safe against being passed
>> NULL.
>> (Not to speak of the risk of unit->priv being used elsewhere while cleaning
>> up.)
>
>
> Are you referring to this error path in sched_init_vcpu?
No, given the context I thought it was clear that I was referring to
static void cf_check
csched_free_udata(const struct scheduler *ops, void *priv)
{
struct csched_unit *svc = priv;
BUG_ON( !list_empty(&svc->runq_elem) );
(i.e. particularly this BUG_ON()).
Jan
