Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hv: Fix issue in size used for memset in create_vm #5108

Closed
wants to merge 1 commit into from

Conversation

sainath14
Copy link

arch_vm member of struct acrn_vm is page aligned. memset used in create_vm
subtracts only 8 bytes, sizeof(spinlock_t) from the size of acrn_vm and uses
the vm->arch_vm as the destination address. To do it right, it should subtract
4096 bytes. This would result in writing memory beyond the acrn_vm struct.

This patch fixes the issue by using offsetof compiler macro and subtracts the
right amount of size corresponding to the beginning of arch_vm member in
struct acrn_vm.

Tracked-On: #5107
Signed-off-by: Sainath Grandhi sainath.grandhi@intel.com

@ghost
Copy link

ghost commented Jul 29, 2020

No new violations to the coding guideline detected.
No New Name Conflict

No New Function Declaration/Definition Mismatch

arch_vm member of struct acrn_vm is page aligned. memset used in create_vm
subtracts only 8 bytes, sizeof(spinlock_t) from the size of acrn_vm and uses
the vm->arch_vm as the destination address. To do it right, it should subtract
4096 bytes. This would result in writing memory beyond the acrn_vm struct.

This patch fixes the issue by using offsetof compiler macro and subtracts the
right amount of size corresponding to the beginning of arch_vm member in
struct acrn_vm.

Tracked-On: projectacrn#5107
Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.com>
* subtracting 4K from the total size of acrn_vm.
* Using offset_of macro to avoid hardcoding 4k here.
*/
(void)memset((void *)&vm->arch_vm, 0U, (sizeof(struct acrn_vm) - offsetof(struct acrn_vm, arch_vm)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. What we want to reset is from arch_vm to the end of vm structure.
Will " (size_t *) (vm+1) - (size_t) &vm->arch_vm" be easier to reader?

You decide.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the current definition of struct acrn_vm may waste ~1 page of memory. We need a cleaner solution.

Copy link
Contributor

@dongyaozu dongyaozu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a cleaner solution to avoid space waste

@wenlingz
Copy link
Contributor

wenlingz commented Aug 5, 2020

@sainath14 please rebase your code, thanks

@sainath14
Copy link
Author

This is not needed after #5128
I will close this PR.

@sainath14 sainath14 closed this Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants