Propose new KVM driver based on KVMi subsystem #844

mtarral · 2019-12-05T14:21:46Z

This PR proposes a brand new KVM driver, based on the experimental KVMi subsystem currently being developed by BitDefender.

The driver's code has been splitted:

kvm_legacy.c: legacy kvm driver (GDB and qemu memaccess patch)
kvm.c the new driver based in KVMi
kvm_common.c: common functions share by both drivers.

This PR also changes some of the examples to include a new kvmi_socket optional parameter.

cc @tklengyel , @adlazar, @mdontu

Fixes #827 , #778

Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

This will enable querying all the registers in one step.

Implement a new KVM driver specific MSR translation. This reverts commit f75a73c.

This is still quite experimental. To be more specific, I think kvm_set_vcpureg is likely to be broken. The reason for this is that the kernel only supports setting all the registers at once and for us to be able to set only a single register we need to retrieve the old values first. Of course, both of these operations need to be done without the vcpu continuing execution while the operations are happening. Maybe we should explicitly pause the vcpu before the operations.

Apparently I had broken the kvm.h header when picking chunks for the previous commit. Sorry for that.

kvmi_uninit was called twice leading to a double free bug if unix socket connection errored. This fixes the problem by manually setting the kvmi pointer to NULL as kvmi_uninit checks for that.

This replaces the hard coded /var/run/testing.sock path with a configurable option.

…igurable socket path.

This commit removes all the other access methods from the kvm driver and only leaves the new kvmi based one. Now `--enable-kvm` implies kvmi.

Kernel API ---------- Noteworthy changes - different values for KVMI_EVENT_* (we have KVMI_EVENT_X and KVMI_EVENT_X_FLAG) - kvmi_event_page_fault renamed to kvmi_event_pf + a new member (shadow_gs) - KVMI_PAUSE_ALL_VCPUS command replaced KVMI_PAUSE_VCPU - kvmi_event_reply has a new member (the event id) - kvmi_get_guest_info_reply extended the vcpu_count member to u32 New - two new VM events: KVMI_EVENT_CREATE_VCPU and KVMI_EVENT_UNHOOK Library API ----------- Noteworthy changes - the structures exchanged during handshake changed - kvmi_domain_close() has a second parameter (to shutdown the socket) - the events received while waiting a command' reply are saved for later processing (kvmi_pop_event) - the event callback has been removed; the events should be accesed with kvmi_wait_event()+kvmi_pop_event() New - a handshake callback can be supplied to kvmi_init_* functions - a logging callback can be set (kvmi_set_log_cb) to receive debug/info/warning/error messages from libkvmi - reads and writes have a 15 seconds timeout - all commands (send+recv) and event replies (send) are synchronized with a mutex Signed-off-by: Mihai Donțu <mdontu@bitdefender.com> Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com> Signed-off-by: Nicușor Cîțu <ncitu@bitdefender.com> Signed-off-by: Mircea Cîrjaliu <mcirjaliu@bitdefender.com>

The libkvmi API (RFC v5) changed two functions used by this driver: - kvmi_init_unix_socket() - kvmi_domain_close() Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

In order to silence: warning: implicit declaration of function ‘vasprintf’ [-Wimplicit-function-declaration] Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

Wenzel · 2020-07-28T15:24:22Z

@tklengyel can you trigger the jenkins also ?
thanks

tklengyel · 2020-07-28T18:30:27Z

@drakvuf-jenkins retest this please

.travis.yml

libvmi/driver/kvm/kvm.h

libvmi/driver/kvm/kvm_events.c

tklengyel · 2020-07-28T23:15:23Z

This is looking good to me, I only spotted some minor things that really aren't blockers. Lots of kudos @mtarral for getting this done! 🥇 🥳

Wenzel · 2020-07-29T09:29:08Z

@tklengyel thanks for you review, I've just fixed the minor issues you have found.
Are we waiting for @smaresca's approval also ?

I'm excited to bring this new driver to the LibVMI community !

smaresca · 2020-07-29T11:11:26Z

@tklengyel thanks for you review, I've just fixed the minor issues you have found.
Are we waiting for @smaresca's approval also ?

@Wenzel Thank you for addressing my license and header-related concerns. There are a few remaining Sandia references in libvmi/driver/kvm files though that need attention.

Wenzel · 2020-07-29T11:26:57Z

According to @tklengyel's comment: #844 (comment)
I thought I needed to remove the Sandia license only in new code/new files.
(therefore I should also remove the license in kvm_common.c and kvm_legacy.c)
is that what you meant @smaresca ?

or did you mean that since the files have changed so much, the Sandia license wasn't applying anymore ?

libvmi/driver/kvm/README.md

smaresca · 2020-07-29T11:52:05Z

According to @tklengyel's comment: #844 (comment)
I thought I needed to remove the Sandia license only in new code/new files.
(therefore I should also remove the license in kvm_common.c and kvm_legacy.c)
is that what you meant @smaresca ?

@Wenzel Ah, disregard my comment about the other Sandia lines. The notices I cited do indeed need to stay in place.

README.rst

smaresca · 2020-07-30T03:25:56Z

Echoing Tamas: I'm very happy to see this come to fruition. Thanks to all Mathieu especially but also to the substantial set of reviewers/contributors we've had on this one.

Wenzel · 2020-07-30T11:15:27Z

Thanks @smaresca :) 👌
Shall we merge then ? no more blockers ?

adlazar and others added 30 commits August 24, 2019 21:04

libvmi/driver/kvm: integrate libkvmi

10e3561

Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

remove improperly escaped emoji

17ca686

Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

libvmi/driver/kvm: cleanup some libkvmi related comments

db9803f

Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

Fix MSR indexes to correspond kernel's msr-index.h

4f55f80

Add missing break statement and MSR_STAR handling to get_kvmi_registers.

967732f

Fix kvmi_get_registers return value handling

f49ca84

Partial implementation of kvm_get_vcpuregs.

e79e98a

This will enable querying all the registers in one step.

Revert previous MSR index fix as it conflicted with Xen driver.

8b99498

Implement a new KVM driver specific MSR translation. This reverts commit f75a73c.

More correct typing for translate_msr_index

fa6f257

Add memory access support for kvmi driver

163e9f6

Fix broken header.

563645f

Apparently I had broken the kvm.h header when picking chunks for the previous commit. Sorry for that.

Fix double free in kvmi_uninit.

f0628fc

kvmi_uninit was called twice leading to a double free bug if unix socket connection errored. This fixes the problem by manually setting the kvmi pointer to NULL as kvmi_uninit checks for that.

Allow passing socket path to init_kvmi.

16877c6

This replaces the hard coded /var/run/testing.sock path with a configurable option.

Comment out Qemu version check and add process-list example with conf…

490c83e

…igurable socket path.

Only kvmi based access in kvm driver.

385aef1

This commit removes all the other access methods from the kvm driver and only leaves the new kvmi based one. Now `--enable-kvm` implies kvmi.

Implement kvm_put_memory using kvmi

e987eeb

driver/kvm: fix the libkvmi calls

24b7be7

The libkvmi API (RFC v5) changed two functions used by this driver: - kvmi_init_unix_socket() - kvmi_domain_close() Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

driver/kvm/libkvmi: add _GNU_SOURCE

1f2e95f

In order to silence: warning: implicit declaration of function ‘vasprintf’ [-Wimplicit-function-declaration] Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>

cmake: fix include dirs for KVM driver

74cf76c

kvm: fix init_data with vmi_init_data_t type

1008413

cmake: compile kvm driver with libkvmi

1e2c3b2

kvm: remove old code in kvm_get_memsize

ba23dd7

examples: update process-list to fill new init_data for kvm

2454407

kvm: implement get_memsize

b2c4085

kvm: add debug output

221ca4c

kvm: read FS_BASE and GS_BASE

718ecd3

kvm: get VCPU count via KVMI instead of libvirt

40e90b2

kvm: replace virDomainSuspend/Resume by kvmi calls

4ce3c58

tklengyel reviewed Jul 28, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

tklengyel reviewed Jul 28, 2020

View reviewed changes

libvmi/driver/kvm/kvm.h Show resolved Hide resolved

tklengyel reviewed Jul 28, 2020

View reviewed changes

libvmi/driver/kvm/kvm_events.c Outdated Show resolved Hide resolved

tklengyel approved these changes Jul 28, 2020

View reviewed changes

Wenzel mentioned this pull request Jul 29, 2020

Kvmi minor fixes KVM-VMI/libvmi#33

Merged

mtarral added 3 commits July 29, 2020 11:06

travis: fix typo

fc7a15e

kvm: remove Sandia notice

cd6ad6e

kvm: refactor kvm.h

778d39c

smaresca reviewed Jul 29, 2020

View reviewed changes

libvmi/driver/kvm/README.md Show resolved Hide resolved

namidairo reviewed Jul 29, 2020

View reviewed changes

README.rst Show resolved Hide resolved

mtarral added 6 commits July 29, 2020 18:05

kvm: update README on VCPU registers implementation

081d797

examples: use retcode value

bc06c31

examples: fix error handling in descriptor-event-example

869d8ee

examples: fix potential memory leak

de64a4c

examples: fix possible uninitialized value when freeing memmap var

ab38de2

examples: fix potential memory leak

d41bf52

smaresca approved these changes Jul 30, 2020

View reviewed changes

tklengyel merged commit aeb8d1d into libvmi:master Jul 30, 2020

Wenzel deleted the kvmi branch August 1, 2020 19:28

This was referenced Aug 1, 2020

libvmi: track commit in master branch KVM-VMI/kvm-vmi#65

Merged

VMIInitData not exported libvmi/python#63

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propose new KVM driver based on KVMi subsystem #844

Propose new KVM driver based on KVMi subsystem #844

mtarral commented Dec 5, 2019

Wenzel commented Jul 28, 2020

tklengyel commented Jul 28, 2020

tklengyel commented Jul 28, 2020 •

edited

Loading

Wenzel commented Jul 29, 2020 •

edited

Loading

smaresca commented Jul 29, 2020 •

edited

Loading

Wenzel commented Jul 29, 2020

smaresca commented Jul 29, 2020 •

edited

Loading

smaresca commented Jul 30, 2020

Wenzel commented Jul 30, 2020

Propose new KVM driver based on KVMi subsystem #844

Propose new KVM driver based on KVMi subsystem #844

Conversation

mtarral commented Dec 5, 2019

Wenzel commented Jul 28, 2020

tklengyel commented Jul 28, 2020

tklengyel commented Jul 28, 2020 • edited Loading

Wenzel commented Jul 29, 2020 • edited Loading

smaresca commented Jul 29, 2020 • edited Loading

Wenzel commented Jul 29, 2020

smaresca commented Jul 29, 2020 • edited Loading

smaresca commented Jul 30, 2020

Wenzel commented Jul 30, 2020

tklengyel commented Jul 28, 2020 •

edited

Loading

Wenzel commented Jul 29, 2020 •

edited

Loading

smaresca commented Jul 29, 2020 •

edited

Loading

smaresca commented Jul 29, 2020 •

edited

Loading