[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Make XEN_FW_EFI_MEM_INFO easier to use
- To: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Thu, 25 Aug 2022 09:59:56 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=Y1GnXrSlW4jdUJvT7zuXPcyu4grolSnLUsThau2VI3E=; b=bnZZNwMz7wpmNKVAx5WcDUWNolGDwF5p6FLs2c7jiraCO15KJc2pLlBnFpg8UiGL4k3JDTRmgQIswfcig0tCUeWD/H9+tqI/sR6uhMGuIexNh6EFJ+7cygI6nRvZE5gVKPhg9ZGGsbBsBRsWDs7koHgkbzQ2RmIjjW0XGA8w3v+LBhGEb8MRc2uHQo+3UBL+rh8czclINSzWWgZ4wgAyxUgEcwGVwsXLm8RfbW2JyNPk8c99FodUgTtqnAl/WzeA3T5oOXh9yjTQwNVcec/yQIPh3wybwUNz5mbdNJoZJyQbRFR5n4/wnIjceFEnuQG0IFGOWrwlh5j1gXLGWt8UOg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FlcEWzvxHL3TDxRPsnd4ChBdM/XDq+QDIrevurVokNZlYVWyjb2gYXfm+PA04fKrGOQ8t5vhbnKN/nr/M3PI/cvXBqKM0fv2GMNLCIM1DWxEzGVMNRBkYssXktP2XEK9/07Fi4OyhN9eCqnm/rvetV9hPha/pIoQO5gj+g6uADQ15h05G9qfQJ1iAVmgGHXuO5HUlgTZ4Al1WKGOz7FhimAwBOOhOF+572/rGH+P9+SGG8Mpt2dXUWNDgOBt78CD8miROu2n6pXvEOQ2DGFWbwCL4alRPsf8znbKGvn/oyDqyibsg89iy3G8/cSRkLMdzE1RlTeMxdzR6o+cQNAX6w==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, Xen developer discussion <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Thu, 25 Aug 2022 08:00:02 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 24.08.2022 23:04, Demi Marie Obenour wrote:
> The XEN_FW_EFI_MEM_INFO platform op has very surprising behavior: it
> only sets info->mem.size if the initial value was *larger* than the size
> of the memory region.
And intentionally so - the caller didn't ask for any bigger region,
after all.
> This is not particularly useful and cost me most
> of a day of debugging. It also has some integer overflow problems,
> though as the data comes from dom0 or the firmware (both of which are
> trusted) these are not security issues.
I'm afraid we're trusting the firmware in this regard elsewhere as
well. So if there was a need to change that, I guess it would need
changing everywhere, not just here. But we trust the E820 map as
well, when on non-EFI platforms, so I don't see why we would need
to change that. In any event such would want to be a separate
change imo.
> Fix both of these problems by unconditionally setting the memory region
> size
If you were to report a larger ending address, why would you not also
report a smaller starting address?
But before you go that route - I don't think we can change the API
now that it has been in use this way for many years. If a "give me
the full enclosing range" variant is wanted, it will need to be
fully separate.
Jan
> and by computing it in a way that is immune to integer overflow.
> The new code is slightly longer, but it is much easier to understand and
> use.
|
