[syscalls][vmo][op_range] add rights checking for different ops
Test for the WRITE right for most of the ops, except the simple cache
clean ops which cant modify data and thus only need read. This mirrors
what arm64 already enforces for user vs supervisor permissions of the
different cache ops. All but a pure invalidate is a read operation,
permission wise.
ZX-967 #done
Change-Id: If45f774b5a2de478572e1bacbfc13baf0a4da667
diff --git a/docs/syscalls/vmo_op_range.md b/docs/syscalls/vmo_op_range.md
index 116c4fb..9295777 100644
--- a/docs/syscalls/vmo_op_range.md
+++ b/docs/syscalls/vmo_op_range.md
@@ -29,20 +29,26 @@
**ZX_VMO_OP_COMMIT** - Commit *size* bytes worth of pages starting at byte *offset* for the VMO.
More information can be found in the [vm object documentation](../objects/vm_object.md).
+Requires the *ZX_RIGHT_WRITE* right.
**ZX_VMO_OP_DECOMMIT** - Release a range of pages previously commited to the VMO from *offset* to *offset*+*size*.
+Requires the *ZX_RIGHT_WRITE* right.
**ZX_VMO_OP_LOCK** - Presently unsupported.
**ZX_VMO_OP_UNLOCK** - Presently unsupported.
**ZX_VMO_OP_CACHE_SYNC** - Performs a cache sync operation.
+Requires the *ZX_RIGHT_READ* right.
**ZX_VMO_OP_CACHE_INVALIDATE** - Performs a cache invalidation operation.
+Requires the *ZX_RIGHT_WRITE* right.
**ZX_VMO_OP_CACHE_CLEAN** - Performs a cache clean operation.
+Requires the *ZX_RIGHT_READ* right.
**ZX_VMO_OP_CACHE_CLEAN_INVALIDATE** - Performs cache clean and invalidate operations together.
+Requires the *ZX_RIGHT_READ* right.
## RETURN VALUE
@@ -60,6 +66,8 @@
**ZX_ERR_WRONG_TYPE** *handle* is not a VMO handle.
+**ZX_ERR_ACCESS_DENIED** *handle* does not have sufficient rights to perform the operation.
+
**ZX_ERR_INVALID_ARGS** *out* is an invalid pointer, *op* is not a valid
operation, or *size* is zero and *op* is a cache operation.
diff --git a/kernel/object/include/object/vm_object_dispatcher.h b/kernel/object/include/object/vm_object_dispatcher.h
index 66c6803..9b1c30f 100644
--- a/kernel/object/include/object/vm_object_dispatcher.h
+++ b/kernel/object/include/object/vm_object_dispatcher.h
@@ -42,7 +42,7 @@
zx_status_t SetSize(uint64_t);
zx_status_t GetSize(uint64_t* size);
zx_status_t RangeOp(uint32_t op, uint64_t offset, uint64_t size, user_inout_ptr<void> buffer,
- size_t buffer_size);
+ size_t buffer_size, zx_rights_t rights);
zx_status_t Clone(
uint32_t options, uint64_t offset, uint64_t size, bool copy_name,
fbl::RefPtr<VmObject>* clone_vmo);
diff --git a/kernel/object/vm_object_dispatcher.cpp b/kernel/object/vm_object_dispatcher.cpp
index 034460b..6b7e60e 100644
--- a/kernel/object/vm_object_dispatcher.cpp
+++ b/kernel/object/vm_object_dispatcher.cpp
@@ -96,35 +96,58 @@
}
zx_status_t VmObjectDispatcher::RangeOp(uint32_t op, uint64_t offset, uint64_t size,
- user_inout_ptr<void> buffer, size_t buffer_size) {
+ user_inout_ptr<void> buffer, size_t buffer_size,
+ zx_rights_t rights) {
canary_.Assert();
LTRACEF("op %u offset %#" PRIx64 " size %#" PRIx64
- " buffer %p buffer_size %zu\n",
- op, offset, size, buffer.get(), buffer_size);
+ " buffer %p buffer_size %zu rights %#x\n",
+ op, offset, size, buffer.get(), buffer_size, rights);
switch (op) {
case ZX_VMO_OP_COMMIT: {
+ if ((rights & ZX_RIGHT_WRITE) == 0) {
+ return ZX_ERR_ACCESS_DENIED;
+ }
// TODO: handle partial commits
auto status = vmo_->CommitRange(offset, size, nullptr);
return status;
}
case ZX_VMO_OP_DECOMMIT: {
+ if ((rights & ZX_RIGHT_WRITE) == 0) {
+ return ZX_ERR_ACCESS_DENIED;
+ }
// TODO: handle partial decommits
auto status = vmo_->DecommitRange(offset, size, nullptr);
return status;
}
case ZX_VMO_OP_LOCK:
case ZX_VMO_OP_UNLOCK:
- // TODO: handle
+ // TODO: handle or remove
return ZX_ERR_NOT_SUPPORTED;
+
case ZX_VMO_OP_CACHE_SYNC:
+ if ((rights & ZX_RIGHT_READ) == 0) {
+ return ZX_ERR_ACCESS_DENIED;
+ }
return vmo_->SyncCache(offset, size);
case ZX_VMO_OP_CACHE_INVALIDATE:
+ // A straight invalidate op requires the write right since
+ // it may drop dirty cache lines, thus modifying the contents
+ // of the VMO.
+ if ((rights & ZX_RIGHT_WRITE) == 0) {
+ return ZX_ERR_ACCESS_DENIED;
+ }
return vmo_->InvalidateCache(offset, size);
case ZX_VMO_OP_CACHE_CLEAN:
+ if ((rights & ZX_RIGHT_READ) == 0) {
+ return ZX_ERR_ACCESS_DENIED;
+ }
return vmo_->CleanCache(offset, size);
case ZX_VMO_OP_CACHE_CLEAN_INVALIDATE:
+ if ((rights & ZX_RIGHT_READ) == 0) {
+ return ZX_ERR_ACCESS_DENIED;
+ }
return vmo_->CleanInvalidateCache(offset, size);
default:
return ZX_ERR_INVALID_ARGS;
diff --git a/kernel/syscalls/vmo.cpp b/kernel/syscalls/vmo.cpp
index b029f86..2f86658 100644
--- a/kernel/syscalls/vmo.cpp
+++ b/kernel/syscalls/vmo.cpp
@@ -184,13 +184,15 @@
auto up = ProcessDispatcher::GetCurrent();
// lookup the dispatcher from handle
- // TODO(ZX-967): test rights on the handle
+ // save the rights and pass down into the dispatcher for further testing
fbl::RefPtr<VmObjectDispatcher> vmo;
- zx_status_t status = up->GetDispatcher(handle, &vmo);
- if (status != ZX_OK)
+ zx_rights_t rights;
+ zx_status_t status = up->GetDispatcherAndRights(handle, &vmo, &rights);
+ if (status != ZX_OK) {
return status;
+ }
- return vmo->RangeOp(op, offset, size, _buffer, buffer_size);
+ return vmo->RangeOp(op, offset, size, _buffer, buffer_size, rights);
}
zx_status_t sys_vmo_set_cache_policy(zx_handle_t handle, uint32_t cache_policy) {
