Skip to content

Commit 09bc0c6

Browse files
committed
NFC: external_pointer address_cast earlier
Make it easier to justify our avoidance of capptr_from_client and capptr_reveal in external_pointer by performing address_cast earlier. In particular, with this change, we can see that the pointer (and so its authority, in CHERI) is not passed to any called function other than address_cast and pointer_offset, and so authority is merely propagated and neither exercised nor amplified. Remove the long-disused capptr_reveal_wild, which was added for earlier versions of external_pointer.
1 parent db3ae1c commit 09bc0c6

File tree

3 files changed

+26
-26
lines changed

3 files changed

+26
-26
lines changed

src/snmalloc/ds_core/ptrwrap.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -423,16 +423,6 @@ namespace snmalloc
423423
return p.unsafe_ptr();
424424
}
425425

426-
/**
427-
* Like capptr_reveal, but sometimes we do mean to reveal wild pointers
428-
* (specifically in external_pointer, where we're revealing something
429-
* architecturally derived from a user pointer).
430-
*/
431-
inline SNMALLOC_FAST_PATH void* capptr_reveal_wild(capptr::AllocWild<void> p)
432-
{
433-
return p.unsafe_ptr();
434-
}
435-
436426
/**
437427
* Given a void* from the client, it's fine to call it AllocWild.
438428
* Roughly dual to capptr_reveal().

src/snmalloc/mem/localalloc.h

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -750,22 +750,31 @@ namespace snmalloc
750750
template<Boundary location = Start>
751751
void* external_pointer(void* p)
752752
{
753-
// Note that each case uses `pointer_offset`, so that on
754-
// CHERI it is monotone with respect to the capability.
755-
// Note that the returned pointer could be outside the CHERI
756-
// bounds of `p`, and thus not something that can be followed.
753+
/*
754+
* Note that:
755+
* * each case uses `pointer_offset`, so that on CHERI, our behaviour is
756+
* monotone with respect to the capability `p`.
757+
*
758+
* * the returned pointer could be outside the CHERI bounds of `p`, and
759+
* thus not something that can be followed.
760+
*
761+
* * we don't use capptr_from_client()/capptr_reveal(), to avoid the
762+
* syntactic clutter. By inspection, `p` flows only to address_cast
763+
* and pointer_offset, and so there's no risk that we follow or act
764+
* to amplify the rights carried by `p`.
765+
*/
757766
if constexpr (location == Start)
758767
{
759-
size_t index = index_in_object(p);
768+
size_t index = index_in_object(address_cast(p));
760769
return pointer_offset(p, 0 - index);
761770
}
762771
else if constexpr (location == End)
763772
{
764-
return pointer_offset(p, remaining_bytes(p) - 1);
773+
return pointer_offset(p, remaining_bytes(address_cast(p)) - 1);
765774
}
766775
else
767776
{
768-
return pointer_offset(p, remaining_bytes(p));
777+
return pointer_offset(p, remaining_bytes(address_cast(p)));
769778
}
770779
}
771780

@@ -775,24 +784,25 @@ namespace snmalloc
775784
* auto p = (char*)malloc(size)
776785
* remaining_bytes(p + n) == size - n provided n < size
777786
*/
778-
size_t remaining_bytes(const void* p)
787+
size_t remaining_bytes(address_t p)
779788
{
780789
#ifndef SNMALLOC_PASS_THROUGH
781790
const PagemapEntry& entry =
782-
Config::Backend::template get_metaentry<true>(address_cast(p));
791+
Config::Backend::template get_metaentry<true>(p);
783792

784793
auto sizeclass = entry.get_sizeclass();
785-
return snmalloc::remaining_bytes(sizeclass, address_cast(p));
794+
return snmalloc::remaining_bytes(sizeclass, p);
786795
#else
787-
return pointer_diff(p, reinterpret_cast<void*>(UINTPTR_MAX));
796+
return reinterpret_cast<size_t>(
797+
std::numeric_limits<decltype(p)>::max() - p);
788798
#endif
789799
}
790800

791801
bool check_bounds(const void* p, size_t s)
792802
{
793803
if (SNMALLOC_LIKELY(Config::is_initialised()))
794804
{
795-
return remaining_bytes(p) >= s;
805+
return remaining_bytes(address_cast(p)) >= s;
796806
}
797807
return true;
798808
}
@@ -803,14 +813,14 @@ namespace snmalloc
803813
* auto p = (char*)malloc(size)
804814
* index_in_object(p + n) == n provided n < size
805815
*/
806-
size_t index_in_object(const void* p)
816+
size_t index_in_object(address_t p)
807817
{
808818
#ifndef SNMALLOC_PASS_THROUGH
809819
const PagemapEntry& entry =
810-
Config::Backend::template get_metaentry<true>(address_cast(p));
820+
Config::Backend::template get_metaentry<true>(p);
811821

812822
auto sizeclass = entry.get_sizeclass();
813-
return snmalloc::index_in_object(sizeclass, address_cast(p));
823+
return snmalloc::index_in_object(sizeclass, p);
814824
#else
815825
return reinterpret_cast<size_t>(p);
816826
#endif

src/test/func/memory/memory.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ void test_remaining_bytes()
476476
char* p = (char*)alloc.alloc(size);
477477
for (size_t offset = 0; offset < size; offset++)
478478
{
479-
auto rem = alloc.remaining_bytes(p + offset);
479+
auto rem = alloc.remaining_bytes(address_cast(pointer_offset(p, offset)));
480480
if (rem != (size - offset))
481481
{
482482
printf(

0 commit comments

Comments
 (0)