[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] IOMMU: make domctl handler tolerate NULL domain
- To: Juergen Gross <jgross@xxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- Date: Tue, 19 Apr 2022 16:06:19 +0000
- Accept-language: en-GB, en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=K18yJa0mvn0c4nvHjp2+PMA/6b9XrRtukNIp6LQK1tw=; b=h9EyQWjlkFKZQRHzspMsh3bE/0MyR0to4OVAlhMgup4MsG21EhzmwrHV5XZnzm8QPEJ4a7BvFfoWiOIhY9d5JkNLfoOJgfYZ4TuJZvrif12A0RQgd5LnvYLPMQjYzBpdgAr17iTNYk5PcxnS84kLxizpjFRtPwRw8duNz8wtjFuZFhaD4fFs0NlmERO8A219pIVUZN/2su6/Lsvk+z1Cw55XILiOZE9LRN7rJtFviWsoeb7FeufRMcdg5oiUnKh2P8+Mq3+858vYMajMy3CsJkN7/POh8QjUeO0uFETcDpHqlBneQn4wGweLFpnoPUmCU/hr2Zdxva59NtQJICBS1g==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lp05NfTjYpMiUH6j+qBrSQIgr2IiREwQEnH266DKUL8PAmdRB8f++uIsSca219efwWPZxDI3dlFnwE9g2Ov4dv8BbNI3fEtjhHMmBJi4E3q/f7l5d5jBUCVzu2Ry4hyCrpjIeuBHMTyfpZvk5aeMIFsi+G0HYTdDCUSnJmDtUk0A0SyU8KYh/8wgTG7oGCBJ5iSRYKi94U25KikR95HytppzoEuOUV0Y5CaO38FcyzMZKwTrQ9eAaj2sXGaEhVWIPKCBgJJO4Z87d+3T6dpmlr46aGYe5c1l5MScQX6tUop4KY3DkozZ/6irZxFz78Hbit2IlR/Do7X5WYuhvDowRw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Paul Durrant <paul@xxxxxxx>
- Delivery-date: Tue, 19 Apr 2022 16:06:34 +0000
- Ironport-data: A9a23:DFaEvqmPwQ55eMQ1vvWT2X/o5gyMJ0RdPkR7XQ2eYbSJt1+Wr1Gzt xIbWDvTPfyCYjD1KY13OY3kphkAsJbRyoVqGVZt/n1jQSMWpZLJC+rCIxarNUt+DCFioGGLT Sk6QoOdRCzhZiaE/n9BCpC48T8kk/vgqoPUUIYoAAgoLeNfYHpn2EoLd9IR2NYy24DlWlLV4 7senuWEULOb828sWo4rw/rrRCNH5JwebxtB4zTSzdgS1LPvvyF94KA3fMldHFOhKmVgJcaoR v6r8V2M1jixEyHBqD+Suu2TnkUiGtY+NOUV45Zcc/DKbhNq/kTe3kunXRa1hIg+ZzihxrhMJ NtxWZOYZgV5Ban+vcUhWTJmDTpgZYJ685bBPi3q2SCT5xWun3rE5dxLVBtzEahDv+F9DCdJ6 OASLy0LYlabneWqzbmnS+5qwMM+MM3sO4BZsXZlpd3bJa9+HdafHOOXu5kEhl/chegXdRraT +MfZSBic1LrZBpXN01MIJk/gP2plj/0dDgwRFe9+/tnuTePlVYZPL7FOdyLV4KzQdpuuW3fg 1za83jFOkwTDYnKodaC2jf27gPVpgv5V5gVD6aQ7eNxjRuYwWl7IB8cWEa/oPK5olWjQN8ZI EsRkgI+oK53+EG1Q93VWxyjvGXCrhMaQ8BXEeAx9EeK0KW8yx6QG2wsXjNHLts8u6cLqScC0 1aIm5blAGdpubjNFnaFrO/I93W1JDQfKnIEaWkcVwwZ7tL/oYY1yBXSUtJkF63zhdrwcd3t/ w23QOEFr+17paY2O2+TpzgrXxrESkD1czMI
- Ironport-hdrordr: A9a23:0mqUwqrnENkdanKNPTPxAjAaV5tyLNV00zEX/kB9WHVpm5Oj+v xGzc5w6farsl0ssSkb6Ku90KnpewK+yXbsibNhcYtKLzOWwldAS7sSorcKogeQVhEWk9Qw6U 4OSdkYNDSdNzlHZIPBkXGF+rUbsZe6GcKT9IHjJh5WJGkEBZ2IrT0JczpzeXcGJjWucKBJcK Z0kfA3wgZIF052Uu2LQl0+G8TTrdzCk5zrJTQcAQQ81QWIhTS0rJbnDhmxxH4lInNy6IZn1V KAvx3y562lvf3+4ATbzXXv45Nfn8ak4sdfBfaLltMeJlzX+0WVjcVaKv+/VQIO0aWSAWUR4Z 7xStAbToJOAkbqDySISN3WqlDdOXgVmiffIBSj8AbeSITCNU4H4ox69MNkm1LimjQdVJsX6t M140uJ85VQFh/OhyL7+pzBUAxrjFO9pT44nfcUlGE3a/pXVFZ9l/1owKpuKuZIIMs60vFULM B+SMXHoPpGe1KTaH7U+mFp3dy3R3w2WhOLWFILtMCZ2yVf2CkR9TpT+OUP2nMbsJ4tQZhN4O rJdqxuibFVV8cTKaZwHv0IT8e7AnHEBRjMLGWRK1L6E7xvAQOHl7fnpLEuoO26cp0By5U/3J zHTVNDrGY3P1njDMWftac7hSwlgF/NKQgF5vsul6SR4IeMNYYDGRfzO2wGgo+nv+gVBNHdVr K6JI9WasWTWFfTJQ==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHYU9FUwjuogeCS0EWgGeBVFIanqKz3Yk2AgAAA74CAAAPwgA==
- Thread-topic: [PATCH] IOMMU: make domctl handler tolerate NULL domain
On 19/04/2022 16:52, Juergen Gross wrote:
> On 19.04.22 17:48, Andrew Cooper wrote:
>> On 19/04/2022 10:39, Jan Beulich wrote:
>>> Besides the reporter's issue of hitting a NULL deref when
>>> !CONFIG_GDBSX,
>>> XEN_DOMCTL_test_assign_device can legitimately end up having NULL
>>> passed
>>> here, when the domctl was passed DOMID_INVALID.
>>>
>>> Fixes: 71e617a6b8f6 ("use is_iommu_enabled() where appropriate...")
>>> Reported-by: Cheyenne Wills <cheyenne.wills@xxxxxxxxx>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/drivers/passthrough/iommu.c
>>> +++ b/xen/drivers/passthrough/iommu.c
>>> @@ -558,7 +558,7 @@ int iommu_do_domctl(
>>> {
>>> int ret = -ENODEV;
>>> - if ( !is_iommu_enabled(d) )
>>> + if ( !(d ? is_iommu_enabled(d) : iommu_enabled) )
>>> return -EOPNOTSUPP;
>>
>> Having spent the better part of a day debugging this mess, this patch is
>> plain broken.
>>
>> It depends on Juergen's "xen/iommu: cleanup iommu related domctl
>> handling" patch, because otherwise it erroneously fails non-IOMMU
>> subops.
>
> Which is not a real problem, as it is being called after all other
> subops didn't match.
It is a real problem even now, because it is bogus for the host or
domain's IOMMU status to have any alteration to the
XEN_DOMCTL_gdbsx_guestmemio path. The root cause of this bug is the
existing XEN_DOMCTL_gdbsx_guestmemio case being compiled out in the
!GDBSX case.
It would be a more obvious problem if there were calls chained after
iommu_do_domctl() in the arch_domctl() default: blocks, because then it
wouldn't only be compiled-out functionality which hit this check.
~Andrew
|
