Hi,
On 10/3/22 19:58, Reinette Chatre wrote:
Hi Borys,
On 10/3/2022 10:33 AM, Reinette Chatre wrote:
On 10/3/2022 10:19 AM, Borys wrote:
I've stumbled upon "sgx_validate_offset_length" function in
"arch/x86/kernel/cpu/sgx/ioctl.c" (all of this is based on 6.0-rc7
version), which does not entirely do what it claims. "offset" and
"length" parameters are provided by userspace and as such their
addition can overflow, which may result in this function approving
malicious values. Fortunately this does not result in any exploitable
bugs at the moment (or at least I couldn't find any), but this might
change if "sgx_validate_offset_length" is used in a new context or
current usages are changed, so it might be worth fixing anyway.
Simple overflow check `offset + length < offset` should be enough.>
Could you please elaborate where you see a possibility for overflow?
Together the provided values, offset and length, are already ensured to
not exceed the total size of the enclave in the following check:
sgx_validate_offset_length() {
...
if (offset + length - PAGE_SIZE >= encl->size)
return -EINVAL;
...
}
I think I see what you mean now ... if offset and length are
sufficiently large the above check can still pass but loops
that have the following pattern may have issues:
for (c = 0 ; c < length; c += PAGE_SIZE) {
...
/* do something at <offset> */
}
Are you planning to submit a patch for the check you propose?
Reinette
Sure, I'll try to submit a patch later today.
Borys
