Skip to content

Commit a739b3f

Browse files
IoanCristianCirsteaIoanCristianCirstea
committed
kernel: Extensive use of nullable pointers
Signed-off-by: Ioan-Cristian CÎRSTEA <ioan.cirstea@oxidos.io>
1 parent 3bd3df4 commit a739b3f

8 files changed

Lines changed: 319 additions & 152 deletions

File tree

‎kernel/src/grant.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ use core::slice;
135135

136136
use crate::kernel::Kernel;
137137
use crate::memory_management::pointers::{
138-
ImmutableKernelVirtualPointer, MutableKernelVirtualPointer,
138+
ImmutableKernelNullableVirtualPointer, MutableKernelNullableVirtualPointer,
139139
};
140140
use crate::process::{Error, Process, ProcessCustomGrantIdentifier, ProcessId};
141141
use crate::processbuffer::{ReadOnlyProcessBuffer, ReadWriteProcessBuffer};
@@ -744,10 +744,8 @@ struct SavedUpcall {
744744
/// read-only allow in a process' kernel managed grant space without wasting
745745
/// memory duplicating information such as process ID.
746746
#[repr(C)]
747-
// TODO: Try to wrap SavedAllowRo in an option instead.
748747
struct SavedAllowRo {
749-
ptr: Option<ImmutableKernelVirtualPointer<u8>>,
750-
// TODO: This should be NonZero once the structure is wrapped in an option.
748+
ptr: ImmutableKernelNullableVirtualPointer<u8>,
751749
len: usize,
752750
}
753751

@@ -757,18 +755,19 @@ struct SavedAllowRo {
757755
#[allow(clippy::derivable_impls)]
758756
impl Default for SavedAllowRo {
759757
fn default() -> Self {
760-
Self { ptr: None, len: 0 }
758+
Self {
759+
ptr: ImmutableKernelNullableVirtualPointer::new_null(),
760+
len: 0,
761+
}
761762
}
762763
}
763764

764765
/// A minimal representation of a read-write allow from app, used for storing a
765766
/// read-write allow in a process' kernel managed grant space without wasting
766767
/// memory duplicating information such as process ID.
767768
#[repr(C)]
768-
// TODO: Try to wrap SavedAllowRw in an option instead.
769769
struct SavedAllowRw {
770-
ptr: Option<MutableKernelVirtualPointer<u8>>,
771-
// TODO: This should be NonZero once the structure is wrapped in an option.
770+
ptr: MutableKernelNullableVirtualPointer<u8>,
772771
len: usize,
773772
}
774773

@@ -778,7 +777,10 @@ struct SavedAllowRw {
778777
#[allow(clippy::derivable_impls)]
779778
impl Default for SavedAllowRw {
780779
fn default() -> Self {
781-
Self { ptr: None, len: 0 }
780+
Self {
781+
ptr: MutableKernelNullableVirtualPointer::new_null(),
782+
len: 0,
783+
}
782784
}
783785
}
784786

@@ -905,9 +907,8 @@ pub(crate) fn allow_ro(
905907
//
906908
// The pointer has already been validated to be within application
907909
// memory before storing the values in the saved slice.
908-
let old_allow = unsafe {
909-
ReadOnlyProcessBuffer::new(saved.ptr.take(), saved.len, process.processid())
910-
};
910+
let old_allow =
911+
unsafe { ReadOnlyProcessBuffer::new(saved.ptr, saved.len, process.processid()) };
911912

912913
// Replace old values with current buffer.
913914
let (ptr, len) = buffer.consume();
@@ -954,9 +955,8 @@ pub(crate) fn allow_rw(
954955
//
955956
// The pointer has already been validated to be within application
956957
// memory before storing the values in the saved slice.
957-
let old_allow = unsafe {
958-
ReadWriteProcessBuffer::new(saved.ptr.take(), saved.len, process.processid())
959-
};
958+
let old_allow =
959+
unsafe { ReadWriteProcessBuffer::new(saved.ptr, saved.len, process.processid()) };
960960

961961
// Replace old values with current buffer.
962962
let (ptr, len) = buffer.consume();

‎kernel/src/kernel.rs‎

Lines changed: 70 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ use crate::memory_management::memory_managers::{
2727
use crate::memory_management::pages::Page4KiB;
2828
use crate::memory_management::permissions::Permissions;
2929
use crate::memory_management::pointers::{
30-
ImmutableUserVirtualPointer, KernelVirtualPointer, MutableUserVirtualPointer, PhysicalPointer,
31-
UserVirtualPointer,
30+
ImmutableUserVirtualPointer, KernelNullableVirtualPointer, KernelVirtualPointer,
31+
MutableUserVirtualPointer, PhysicalPointer, UserNullableVirtualPointer, UserVirtualPointer,
3232
};
3333
use crate::memory_management::regions::{
3434
KernelMappedAllocatedRegion, KernelMappedProtectedAllocatedRegion,
@@ -246,6 +246,33 @@ impl Kernel {
246246
)
247247
}
248248

249+
pub(crate) fn translate_user_protected_virtual_nullable_pointer_byte<
250+
const IS_MUTABLE: bool,
251+
U: AlwaysAligned,
252+
>(
253+
&self,
254+
process: &dyn process::Process,
255+
user_virtual_pointer: UserNullableVirtualPointer<IS_MUTABLE, U>,
256+
) -> Result<
257+
KernelNullableVirtualPointer<IS_MUTABLE, U>,
258+
UserNullableVirtualPointer<IS_MUTABLE, U>,
259+
> {
260+
match user_virtual_pointer {
261+
UserNullableVirtualPointer::Null => Ok(KernelNullableVirtualPointer::Null),
262+
UserNullableVirtualPointer::NonNull(non_null_user_virtual_pointer) => self
263+
.translate_user_protected_virtual_pointer_byte(
264+
process,
265+
non_null_user_virtual_pointer,
266+
)
267+
.map(|non_null_kernel_virtual_pointer| {
268+
KernelNullableVirtualPointer::NonNull(non_null_kernel_virtual_pointer)
269+
})
270+
.map_err(|non_null_user_virtual_pointer| {
271+
UserNullableVirtualPointer::NonNull(non_null_user_virtual_pointer)
272+
}),
273+
}
274+
}
275+
249276
pub(crate) fn internal_translate_user_allocated_virtual_pointer_byte<
250277
const IS_MUTABLE: bool,
251278
U: AlwaysAligned,
@@ -290,20 +317,24 @@ impl Kernel {
290317
>(
291318
&self,
292319
process: &dyn process::Process,
293-
kernel_virtual_pointer: Option<KernelVirtualPointer<IS_MUTABLE, U>>,
320+
kernel_virtual_pointer: KernelNullableVirtualPointer<IS_MUTABLE, U>,
294321
) -> Result<
295-
Option<UserVirtualPointer<IS_MUTABLE, U>>,
296-
Option<KernelVirtualPointer<IS_MUTABLE, U>>,
322+
UserNullableVirtualPointer<IS_MUTABLE, U>,
323+
KernelNullableVirtualPointer<IS_MUTABLE, U>,
297324
> {
298325
match kernel_virtual_pointer {
299-
None => Ok(None),
300-
Some(non_null_kernel_virtual_pointer) => self
326+
KernelNullableVirtualPointer::Null => Ok(UserNullableVirtualPointer::Null),
327+
KernelNullableVirtualPointer::NonNull(non_null_kernel_virtual_pointer) => self
301328
.translate_kernel_allocated_to_user_protected_byte(
302329
process,
303330
non_null_kernel_virtual_pointer,
304331
)
305-
.map(|user_virtual_pointer| Some(user_virtual_pointer))
306-
.map_err(|non_null_kernel_virtual_pointer| Some(non_null_kernel_virtual_pointer)),
332+
.map(|user_virtual_pointer| {
333+
UserNullableVirtualPointer::NonNull(user_virtual_pointer)
334+
})
335+
.map_err(|non_null_kernel_virtual_pointer| {
336+
KernelNullableVirtualPointer::NonNull(non_null_kernel_virtual_pointer)
337+
}),
307338
}
308339
}
309340

@@ -1393,17 +1424,16 @@ impl Kernel {
13931424
allow_pointer,
13941425
allow_size,
13951426
} => {
1396-
let kernel_allow_pointer = match allow_pointer {
1397-
None => None,
1398-
Some(non_null_allow_pointer) => {
1399-
// PANIC: TODO: don't panic
1400-
let non_null_kernel_allow_pointer = self
1401-
.translate_user_protected_virtual_pointer_byte(
1402-
process,
1403-
non_null_allow_pointer,
1404-
).unwrap();
1405-
Some(non_null_kernel_allow_pointer)
1427+
let kernel_allow_pointer = match self.translate_user_protected_virtual_nullable_pointer_byte(
1428+
process,
1429+
allow_pointer,
1430+
) {
1431+
Err(allow_pointer) => {
1432+
let syscall_return = SyscallReturn::AllowReadWriteFailure(ErrorCode::INVAL, allow_pointer, allow_size);
1433+
process.set_syscall_return_value(syscall_return);
1434+
return;
14061435
}
1436+
Ok(kernel_allow_pointer) => kernel_allow_pointer,
14071437
};
14081438

14091439
let res = match driver {
@@ -1543,7 +1573,7 @@ impl Kernel {
15431573
process.processid(),
15441574
driver_number,
15451575
subdriver_number,
1546-
allow_pointer.map_or(0, |pointer| pointer.get_address().get()),
1576+
allow_pointer.get_address(),
15471577
allow_size,
15481578
res
15491579
);
@@ -1556,17 +1586,16 @@ impl Kernel {
15561586
allow_pointer,
15571587
allow_size,
15581588
} => {
1559-
let kernel_allow_pointer = match allow_pointer {
1560-
None => None,
1561-
Some(non_null_allow_pointer) => {
1562-
// PANIC: TODO: don't panic
1563-
let non_null_kernel_allow_pointer = self
1564-
.translate_user_protected_virtual_pointer_byte(
1565-
process,
1566-
non_null_allow_pointer,
1567-
).unwrap();
1568-
Some(non_null_kernel_allow_pointer)
1589+
let kernel_allow_pointer = match self.translate_user_protected_virtual_nullable_pointer_byte(
1590+
process,
1591+
allow_pointer,
1592+
) {
1593+
Err(allow_pointer) => {
1594+
let syscall_return = SyscallReturn::AllowReadWriteFailure(ErrorCode::INVAL, allow_pointer, allow_size);
1595+
process.set_syscall_return_value(syscall_return);
1596+
return;
15691597
}
1598+
Ok(kernel_allow_pointer) => kernel_allow_pointer,
15701599
};
15711600

15721601
let res = match driver {
@@ -1646,7 +1675,7 @@ impl Kernel {
16461675
process.processid(),
16471676
driver_number,
16481677
subdriver_number,
1649-
allow_pointer.map_or(0, |pointer| pointer.get_address().get()),
1678+
allow_pointer.get_address(),
16501679
allow_size,
16511680
res
16521681
);
@@ -1659,17 +1688,16 @@ impl Kernel {
16591688
allow_pointer,
16601689
allow_size,
16611690
} => {
1662-
let kernel_allow_pointer = match allow_pointer {
1663-
None => None,
1664-
Some(non_null_allow_pointer) => {
1665-
// PANIC: TODO: don't panic
1666-
let non_null_kernel_allow_pointer = self
1667-
.translate_user_protected_virtual_pointer_byte(
1668-
process,
1669-
non_null_allow_pointer,
1670-
).unwrap();
1671-
Some(non_null_kernel_allow_pointer)
1691+
let kernel_allow_pointer = match self.translate_user_protected_virtual_nullable_pointer_byte(
1692+
process,
1693+
allow_pointer,
1694+
) {
1695+
Err(allow_pointer) => {
1696+
let syscall_return = SyscallReturn::AllowReadOnlyFailure(ErrorCode::INVAL, allow_pointer, allow_size);
1697+
process.set_syscall_return_value(syscall_return);
1698+
return;
16721699
}
1700+
Ok(kernel_allow_pointer) => kernel_allow_pointer,
16731701
};
16741702

16751703
let res = match driver {
@@ -1809,7 +1837,7 @@ impl Kernel {
18091837
process.processid(),
18101838
driver_number,
18111839
subdriver_number,
1812-
allow_pointer.map_or(0, |pointer| pointer.get_address().get()),
1840+
allow_pointer.get_address(),
18131841
allow_size,
18141842
res
18151843
);

0 commit comments

Comments
 (0)