Re: [PATCH v2 1/2] xen/sched: core: skip missing vcpu slots in sched_move_domain()
Juergen Gross <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <[email protected]> |
On 19.08.26 07:15, Furkan Caliskan wrote:
> sched_move_domain() derives the number of units to rebuild from
> d->max_vcpus, which is fixed at domain creation and never rolled
> back if vcpu_create() fails partway through building a domain. So
> d->vcpu[i] can be NULL for some i even though max_vcpus still
> counts it - this happens if sched_alloc_udata() returns NULL.
>
> The per-unit loop doesn't check for this: it sets
> unit->vcpu_list = d->vcpu[unit_id] (NULL) and hands that broken
> unit straight to the destination scheduler's alloc_udata(),
> which assumes vcpu_list is always valid and crashes Xen when
> it is not.
>
> Reproduced by building a domain in a non-default cpupool where
> vcpu creation fails partway through, then destroying it.
> domain_kill() moves the domain back to the default cpupool via
> sched_move_domain() before actually destroying it, crashing
> inside the destination scheduler's alloc_udata() (seen in
> Credit2's csched2_alloc_udata() -> is_idle_unit() -> NULL deref).
>
> Before building a unit in sched_move_domain(), check whether all
> vpcu slots belonging to that unit are populated. If any of its
> vpcus is missing:
> - For a dying domain, skip the unit allocation.
> - For an active domain, abort the move and return -EINVAL to
> prevent running with dropped vCPUs.
>
> Fixes: 70fadc41635b ("xen/cpupool: support moving domain between cpupools with different granularity")
> Signed-off-by: Furkan Caliskan <[email protected]>
Sorry for realizing this only now, but I think this problem should be
solved completely differently.
Today there are multiple places in the hypervisor where vcpus are being
setup via vcpu_create(). For vcpu-ids other than 0 this is always done
in a loop with ascending ids, up to d->max_vcpus. Whenever vcpu_create()
is failing, the loop is terminated. The only special case is the idle-domain,
which will never have your problem.
So the easy fix would be to:
- have only one function vcpus_create() creating all the vcpus for a domain
and use this function everywhere instead of said loop
- in case a single vcpu can't be created by vcpus_create(), d->max_vcpus
should be reset to the id of the vcpu which couldn't be created
This will avoid any potential NULL derefs elsewhere, as vcpu loops will
just be ending before hitting a NULL pointer.
In case you are not feeling comfortable writing this patch, just speak up
and I will do it.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
(application/pgp-keys, 3.6 KB)
-----BEGIN PGP PUBLIC KEY BLOCK----- xsBNBFOMcBYBCACgGjqjoGvbEouQZw/ToiBg9W98AlM2QHV+iNHsEs7kxWhKMjri oyspZKOBycWxw3ie3j9uvg9EOB3aN4xiTv4qbnGiTr3oJhkB1gsb6ToJQZ8uxGq2 kaV2KL9650I1SJvedYm8Of8Zd621lSmoKOwlNClALZNew72NjJLEzTalU1OdT7/i 1TXkH09XSSI8mEQ/ouNcMvIJNwQpd369y9bfIhWUiVXEK7MlRgUG6MvIj6Y3Am/B BLUVbDa4+gmzDC9ezlZkTZG2t14zWPvxXP3FAp2pkW0xqG7/377qptDmrk42GlSK N4z76ELnLxussxc7I2hx18NUcbP8+uty4bMxABEBAAHNHEp1ZXJnZW4gR3Jvc3Mg PGpnQHBmdXBmLm5ldD7CwHkEEwECACMFAlOMcBYCGwMHCwkIBwMCAQYVCAIJCgsE FgIDAQIeAQIXgAAKCRCw3p3WKL8TL0KdB/93FcIZ3GCNwFU0u3EjNbNjmXBKDY4F UGNQH2lvWAUy+dnyThpwdtF/jQ6j9RwE8VP0+NXcYpGJDWlNb9/JmYqLiX2Q3Tye vpB0CA3dbBQp0OW0fgCetToGIQrg0MbD1C/sEOv8Mr4NAfbauXjZlvTj30H2jO0u +6WGM6nHwbh2l5O8ZiHkH32iaSTfN7Eu5RnNVUJbvoPHZ8SlM4KWm8rG+lIkGurq qu5gu8q8ZMKdsdGC4bBxdQKDKHEFExLJK/nRPFmAuGlId1E3fe10v5QL+qHI3EIP tyfE7i9Hz6rVwi7lWKgh7pe0ZvatAudZ+JNIlBKptb64FaiIOAWDCx1SzR9KdWVy Z2VuIEdyb3NzIDxqZ3Jvc3NAc3VzZS5jb20+wsB5BBMBAgAjBQJTjHCvAhsDBwsJ CAcDAgEGFQgCCQoLBBYCAwECHgECF4AACgkQsN6d1ii/Ey/HmQf/RtI7kv5A2PS4 RF7HoZhPVPogNVbC4YA6lW7DrWf0teC0RR3MzXfy6pJ+7KLgkqMlrAbN/8Dvjoz7 8X+5vhH/rDLa9BuZQlhFmvcGtCF8eR0T1v0nC/nuAFVGy+67q2DH8As3KPu0344T BDpAvr2uYM4tSqxK4DURx5INz4ZZ0WNFHcqsfvlGJALDeE0LhITTd9jLzdDad1pQ SToCnLl6SBJZjDOX9QQcyUigZFtCXFst4dlsvddrxyqT1f17+2cFSdu7+ynLmXBK 7abQ3rwJY8SbRO2iRulogc5vr/RLMMlscDAiDkaFQWLoqHHOdfO9rURssHNN8WkM nQfvUewRz80hSnVlcmdlbiBHcm9zcyA8amdyb3NzQG5vdmVsbC5jb20+wsB5BBMB AgAjBQJTjHDXAhsDBwsJCAcDAgEGFQgCCQoLBBYCAwECHgECF4AACgkQsN6d1ii/ Ey8PUQf/ehmgCI9jB9hlgexLvgOtf7PJnFOXgMLdBQgBlVPO3/D9R8LtF9DBAFPN hlrsfIG/SqICoRCqUcJ96Pn3P7UUinFG/I0ECGF4EvTE1jnDkfJZr6jrbjgyoZHi w/4BNwSTL9rWASyLgqlA8u1mf+c2yUwcGhgkRAd1gOwungxcwzwqgljf0N51N5Jf VRHRtyfwq/ge+YEkDGcTU6Y0sPOuj4Dyfm8fJzdfHNQsWq3PnczLVELStJNdapwP OoE+lotufe3AM2vAEYJ9rTz3Cki4JFUsgLkHFqGZarrPGi1eyQcXeluldO3m91NK /1xMI3/+8jbO0tsn1tqSEUGIJi7ox80eSnVlcmdlbiBHcm9zcyA8amdyb3NzQHN1 c2UuZGU+wsB5BBMBAgAjBQJTjHDrAhsDBwsJCAcDAgEGFQgCCQoLBBYCAwECHgEC F4AACgkQsN6d1ii/Ey+LhQf9GL45eU5vOowA2u5N3g3OZUEBmDHVVbqMtzwlmNC4 k9Kx39r5s2vcFl4tXqW7g9/ViXYuiDXb0RfUpZiIUW89siKrkzmQ5dM7wRqzgJpJ wK8Bn2MIxAKArekWpiCKvBOB/Cc+3EXE78XdlxLyOi/NrmSGRIov0karw2RzMNOu 5D+jLRZQd1Sv27AR+IP3I8U4aqnhLpwhK7MEy9oCILlgZ1QZe49kpcumcZKORmzB TNh30FVKK1EvmV2xAKDoaEOgQB4iFQLhJCdP1I5aSgM5IVFdn7v5YgEYuJYx37Io N1EblHI//x/e2AaIHpzK5h88NEawQsaNRpNSrcfbFmAg987ATQRTjHAWAQgAyzH6 AOODMBjgfWE9VeCgsrwH3exNAU32gLq2xvjpWnHIs98ndPUDpnoxWQugJ6MpMncr 0xSwFmHEgnSEjK/PAjppgmyc57BwKII3sV4on+gDVFJR6Y8ZRwgnBC5mVM6JjQ5x Dk8WRXljExRfUX9pNhdE5eBOZJrDRoLUmmjDtKzWaDhIg/+1Hzz93X4fCQkNVbVF LELU9bMaLPBG/x5q4iYZ2k2ex6d47YE1ZFdMm6YBYMOljGkZKwYde5ldM9mo45mm we0icXKLkpEdIXKTZeKDO+Hdv1aqFuAcccTg9RXDQjmwhC3yEmrmcfl0+rPghO0I v3OOImwTEe4co3c1mwARAQABwsBfBBgBAgAJBQJTjHAWAhsMAAoJELDendYovxMv Q/gH/1ha96vm4P/L+bQpJwrZ/dneZcmEwTbe8YFsw2V/Buv6Z4Mysln3nQK5ZadD 534CF7TDVft7fC4tU4PONxF5D+/tvgkPfDAfF77zy2AH1vJzQ1fOU8lYFpZXTXIH b+559UqvIB8AdgR3SAJGHHt4RKA0F7f5ipYBBrC6cyXJyyoprT10EMvU8VGiwXvT yJz3fjoYsdFzpWPlJEBRMedCot60g5dmbdrZ5DWClAr0yau47zpWj3enf1tLWaqc suylWsviuGjKGw7KHQd3bxALOknAp4dN3QwBYCKuZ7AddY9yjynVaD5X7nF9nO5B jR/i1DG86lem3iBDXzXsZDn8R3/CwO0EGAEIACAWIQSFEmdy6PYElKXQl/ew3p3W KL8TLwUCWt3w0AIbAgCBCRCw3p3WKL8TL3YgBBkWCAAdFiEEUy2wekH2OPMeOLge gFxhu0/YY74FAlrd8NAACgkQgFxhu0/YY75NiwD/fQf/RXpyv9ZX4n8UJrKDq422 bcwkujisT6jix2mOOwYBAKiip9+mAD6W5NPXdhk1XraECcIspcf2ff5kCAlG0DIN aTUH/RIwNWzXDG58yQoLdD/UPcFgi8GWtNUp0Fhc/GeBxGipXYnvuWxwS+Qs1Qay 7/Nbal/v4/eZZaWs8wl2VtrHTS96/IF6q2o0qMey0dq2AxnZbQIULiEndgR625EF RFg+IbO4ldSkB3trsF2ypYLij4ZObm2casLIP7iB8NKmQ5PndL8Y07TtiQ+Sb/wn g4GgV+BJoKdDWLPCAlCMilwbZ88Ijb+HF/aipc9hsqvW/hnXC2GajJSAY3Qs9Mib 4Hm91jzbAjmp7243pQ4bJMfYHemFFBRaoLC7ayqQjcsttN2ufINlqLFPZPR/i3IX kt+z4drzFUyEjLM1vVvIMjkUoJs= =eeAB -----END PGP PUBLIC KEY BLOCK-----
OpenPGP_signature.asc
(application/pgp-signature, 495 B)
-----BEGIN PGP SIGNATURE----- wsB5BAABCAAjFiEEhRJncuj2BJSl0Jf3sN6d1ii/Ey8FAmqRgvMFAwAAAAAACgkQsN6d1ii/Ey8X FQf/U0/ussTD7PYg8BxOlEksaV1+Z/CgRRog6ItVu8Rw5/FTYEInbtLpOZZ9FMQVZ4G15MYG7gvF I9X4OARBPIPw81zSO7tAUnthS7plxGsy+mnE+7EA8yOPVJ0DNHG/XgoRlgsXBNHM6JoQQfINXCP8 txM9GbHlKtBuBM8TqIFOvqUyDp/MNhWfAeE0qqVNuttYvSBC1DUprwhk3nDB4pkZQ5gmAX089HBb kdYQoCkfyVU2i02FTQcf0pU9OYrRUwafVmSmPsLW9hxp6FyunSoeAaePMElX+hTw4zLBEHFXKoqH SlOhI1utiseuPxgOkKnZItMnuBzDFfiK48tEKf7iAg== =upuY -----END PGP SIGNATURE-----