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

dm: virtio: check for paddr_guest2host return value #5453

Merged
merged 1 commit into from Nov 6, 2020

Conversation

tomasbw
Copy link
Contributor

@tomasbw tomasbw commented Nov 2, 2020

paddr_guest2host can return NULL, but code paths in virtio
are not checking the return value.
_vq_record() initializes iov_base pointer using paddr_guest2host()
but there is nothing in the flow that checks for NULL.
Chane _vq_record to return -1 in case the address translation
has failed.

Tracked-On: #5452
Signed-off-by: Tomas Winkler tomas.winkler@intel.com
Acked-by: Wang, Yu1 yu1.wang@intel.com

@ghost
Copy link

ghost commented Nov 2, 2020

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

No New Function Declaration/Definition Mismatch

@wenlingz
Copy link
Contributor

wenlingz commented Nov 3, 2020

OK to verify

@ghost
Copy link

ghost commented Nov 3, 2020

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

No New Function Declaration/Definition Mismatch

@acrnsi-robot
Copy link
Contributor

@tomasbw
RT could not boot up..

@acrnsi-robot
Copy link
Contributor

@tomasbw
boot rt fail ~~ Please check it out
`pm_by_vuart_init idx: 1, path: /dev/ttyS1
pci init hostbridge
pci init passthru
polling 35...
Listening 35...
initiate a connection on a socket error
create socket to connect life-cycle manager failed
pci init virtio-console
tpm: init_vtpm2:Invalid socket path!
/tmp/dm.XbAOPLW 24: Device (PCI0)
Warning 3073 - Multiple types ^ (Device object requires either a _HID or _ADR, but not both)

                                                                                                                                                                              /tmp/dm.XbAOPLW    110:     Processor (CPU0, 0x00, 0x00000000, 0x00) {}
                                                          Warning  3168 -                          ^ Legacy Processor() keyword detected. Use Device() keyword instead.

                                                                                                                                                                       /tmp/dm.XbAOPLW    111:     Processor (CPU1, 0x01, 0x00000000, 0x00) {}
                                                   Warning  3168 -                          ^ Legacy Processor() keyword detected. Use Device() keyword instead.

                                                                                                                                                                acrn_sw_load
                                                                                                                                                                            SW_LOAD: partition blob /usr/share/acrn/bios/OVMF.fd size 2097152 copy to guest 0xffe00000
                                                                           SW_LOAD: build e820 7 entries to addr: 0x7f26800ef008

SW_LOAD: entry[0]: addr 0x0000000000000000, size 0x00000000000a0000, type 0x1
SW_LOAD: entry[1]: addr 0x0000000000100000, size 0x000000003b700000, type 0x1
SW_LOAD: entry[2]: addr 0x000000003b800000, size 0x0000000004004000, type 0x2
SW_LOAD: entry[3]: addr 0x0000000040080000, size 0x0000000000800000, type 0x2
SW_LOAD: entry[4]: addr 0x00000000e0000000, size 0x0000000020000000, type 0x2
SW_LOAD: entry[5]: addr 0x0000000140000000, size 0x0000000000000000, type 0x2
SW_LOAD: entry[6]: addr 0x0000000000000000, size 0x0000000000000000, type 0x0
SW_LOAD: ovmf_entry 0xfffffff0
add_cpu
out instr on NMI port (0x61) not supported
out instr on NMI port (0x61) not supported
out instr on NMI port (0x61) not supported
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
`

@ywan170
Copy link
Contributor

ywan170 commented Nov 4, 2020

@tomasbw please check your patch:

virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed
virtio_vq_init: vq enable failed

paddr_guest2host can return NULL, but code paths in virtio
are not checking the return value.
_vq_record() initializes iov_base pointer using paddr_guest2host()
but there is nothing in the flow that checks for NULL.
Chane _vq_record to return -1 in case the address translation
has failed.

Tracked-On: projectacrn#5452
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Acked-by: Wang, Yu1 <yu1.wang@intel.com>
@tomasbw
Copy link
Contributor Author

tomasbw commented Nov 4, 2020

the issue was fixed, missing return before the error label.

@ghost
Copy link

ghost commented Nov 4, 2020

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

No New Function Declaration/Definition Mismatch

@wenlingz wenlingz merged commit 188ab4f into projectacrn:master Nov 6, 2020
13 checks passed
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