Send inn-workers mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."
Today's Topics:
1. Capability to integer casts on CheriBSD (Julien ?LIE)
2. Re: Capability to integer casts on CheriBSD (Richard Kettlewell)
----------------------------------------------------------------------
Message: 1
Date: Sun, 29 Oct 2023 21:36:52 +0100
From: Julien ?LIE <[email protected]>
To: "[email protected]" <[email protected]>
Subject: Capability to integer casts on CheriBSD
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8; format=flowed
Hi all,
I've just tried a build on CheriBSD, using Clang 13.
It is said to implement memory protection and software
compartmentalization features enabled by CHERI-extended CPUs.
Interestingly, one of the test for checking the validity of a header
field body in the test suite core dumps, because of an access to a char
before the start of a string.
It is easily fixed, but what strikes me most is that the error had still
not popped up on any other systems, nor was found by static analysers.
(I bet it would be interesting to run an INN instance for a while on
CheriBSD to see it does not core dumps at other places.)
--- a/lib/headers.c
+++ b/lib/headers.c
@@ -47,6 +47,7 @@ bool
IsValidHeaderBody(const char *p)
{
bool emptycontentline = true;
+ const char *start = p;
/* Not NULL and not empty. */
if (p == NULL || *p == '\0')
@@ -73,7 +74,7 @@ IsValidHeaderBody(const char *p)
* re-initialize emptycontentline to true. */
emptycontentline = true;
continue;
- } else if (p[-1] == '\r') {
+ } else if (p > start && p[-1] == '\r') {
/* Case of CR not followed by LF (handled at the previous
* if statement). */
return false;
That said, the reason I am writing is for two Clang warnings I am unsure
they are genuine errors to fix (and also how to fix them if they should).
In lib/mmap.c, with inn__msync_page(void *p, size_t length, int flags):
mmap.c:29:33: error: cast from capability type 'void *' to
non-capability, non-address type 'size_t' (aka 'unsigned long') is most
likely an error [-Werror,-Wcapability-to-integer-cast]
char *start = (char *) ((size_t) p & mask);
^~~~~~~~~~
mmap.c:29:23: error: cast from provenance-free integer type to pointer
type will give pointer that can not be dereferenced
[-Werror,-Wcheri-capability-misuse]
char *start = (char *) ((size_t) p & mask);
^
mmap.c:30:32: error: cast from capability type 'void *' to
non-capability, non-address type 'size_t' (aka 'unsigned long') is most
likely an error [-Werror,-Wcapability-to-integer-cast]
char *end = (char *) (((size_t) p + length + pagesize) & mask);
^~~~~~~~~~
mmap.c:30:21: error: cast from provenance-free integer type to pointer
type will give pointer that can not be dereferenced
[-Werror,-Wcheri-capability-misuse]
char *end = (char *) (((size_t) p + length + pagesize) & mask);
^
Same thing for CNFS:
cnfs/cnfs.c:1007:23: error: cast from capability type 'void *' to
non-capability, non-address type 'size_t' (aka 'unsigned long') is most
likely an error [-Werror,-Wcapability-to-integer-cast]
start = (char *) ((size_t) p & ~(size_t) (pagesize - 1));
^~~~~~~~~~
cnfs/cnfs.c:1007:13: error: cast from provenance-free integer type to
pointer type will give pointer that can not be dereferenced
[-Werror,-Wcheri-capability-misuse]
start = (char *) ((size_t) p & ~(size_t) (pagesize - 1));
^
cnfs/cnfs.c:1008:21: error: cast from capability type 'char *' to
non-capability, non-address type 'size_t' (aka 'unsigned long') is most
likely an error [-Werror,-Wcapability-to-integer-cast]
end = (char *) ((size_t) ((char *) p + length + pagesize)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cnfs/cnfs.c:1008:11: error: cast from provenance-free integer type to
pointer type will give pointer that can not be dereferenced
[-Werror,-Wcheri-capability-misuse]
end = (char *) ((size_t) ((char *) p + length + pagesize)
^
And a few other parts of the code, like this one:
icd.c:490:16: error: cast from capability type 'char *' to
non-capability, non-address type 'unsigned long' is most likely an error
[-Werror,-Wcapability-to-integer-cast]
syslog(L_FATAL, "%s msync failed %s 0x%lx %d %m", LogName,
ICDactpath,
(unsigned long) ICDactpointer, ICDactsize);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Should something really be fixed, or may I just silent these warnings
with -Wno-* because of false positives?
--
Julien ?LIE
??Rien n'est plus aga?ant que de ne pas se rappeler ce dont on ne
parvient pas ? se souvenir et rien n'est plus ?nervant que de se
souvenir de ce qu'on voudrait parvenir ? oublier.?? (Pierre Dac)
------------------------------
Message: 2
Date: Mon, 30 Oct 2023 09:49:49 +0000
From: Richard Kettlewell <[email protected]>
To: [email protected]
Subject: Re: Capability to integer casts on CheriBSD
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8; format=flowed
On 29/10/2023 20:36, Julien ?LIE wrote:
> Hi all,
>
> I've just tried a build on CheriBSD, using Clang 13.
> It is said to implement memory protection and software
> compartmentalization features enabled by CHERI-extended CPUs.
>
> Interestingly, one of the test for checking the validity of a header
> field body in the test suite core dumps, because of an access to a char
> before the start of a string.
> It is easily fixed, but what strikes me most is that the error had still
> not popped up on any other systems, nor was found by static analysers.
>
> (I bet it would be interesting to run an INN instance for a while on
> CheriBSD to see it does not core dumps at other places.)
>
>
> --- a/lib/headers.c
> +++ b/lib/headers.c
> @@ -47,6 +47,7 @@ bool
> ?IsValidHeaderBody(const char *p)
> ?{
> ???? bool emptycontentline = true;
> +??? const char *start = p;
>
> ???? /* Not NULL and not empty. */
> ???? if (p == NULL || *p == '\0')
> @@ -73,7 +74,7 @@ IsValidHeaderBody(const char *p)
> ????????????? * re-initialize emptycontentline to true. */
> ???????????? emptycontentline = true;
> ???????????? continue;
> -??????? } else if (p[-1] == '\r') {
> +??????? } else if (p > start && p[-1] == '\r') {
> ???????????? /* Case of CR not followed by LF (handled at the previous
> ????????????? * if statement). */
> ???????????? return false;
>
>
>
>
> That said, the reason I am writing is for two Clang warnings I am unsure
> they are genuine errors to fix (and also how to fix them if they should).
>
>
> In lib/mmap.c, with inn__msync_page(void *p, size_t length, int flags):
>
> mmap.c:29:33: error: cast from capability type 'void *' to
> non-capability, non-address type 'size_t' (aka 'unsigned long') is most
> likely an error [-Werror,-Wcapability-to-integer-cast]
> ??????? char *start = (char *) ((size_t) p & mask);
> ??????????????????????????????? ^~~~~~~~~~
> mmap.c:29:23: error: cast from provenance-free integer type to pointer
> type will give pointer that can not be dereferenced
> [-Werror,-Wcheri-capability-misuse]
> ??????? char *start = (char *) ((size_t) p & mask);
> ????????????????????? ^
> mmap.c:30:32: error: cast from capability type 'void *' to
> non-capability, non-address type 'size_t' (aka 'unsigned long') is most
> likely an error [-Werror,-Wcapability-to-integer-cast]
> ??????? char *end = (char *) (((size_t) p + length + pagesize) & mask);
> ?????????????????????????????? ^~~~~~~~~~
> mmap.c:30:21: error: cast from provenance-free integer type to pointer
> type will give pointer that can not be dereferenced
> [-Werror,-Wcheri-capability-misuse]
> ??????? char *end = (char *) (((size_t) p + length + pagesize) & mask);
> ??????????????????? ^
I don't know much about Cheri but AIUI pointers have extra metadata to
enforce run-time bounds checking, so synthesizing pointers from integers
like this does seem rather problematic - there's nowhere for the bound
information to come from.
The idea of the code in question seems to be to convert region expressed
by a pointer and length into the slightly wider region containing it
consisting of whole pages.
An alternative approach that does not synthesize any pointers (but still
relies on a pointer-to-integer conversion):
size_t page_mask = pagesize - 1;
// Offset of p from start of first page
size_t start_offset = (size_t)p & page_mask;
// Start of first page
char *start = p - start_offset;
// Offset of (p+length) from start of last page, or 0
// if (p+length) is exactly on a page boundary
size_t end_offset = (start_offset + length) & page_mask;
// Offset _backwards_ of (p+length) from end of last page
if(end_offset > 0)
end_offset = page_mask - end_offset;
// Total length of pages
size_t total_length = start+offset + length + end_offset;
return msync(start, total_length, flags);
Totally untested - the calculation should be out into a distinct
function so it can be unit-tested properly.
> And a few other parts of the code, like this one:
>
>
> icd.c:490:16: error: cast from capability type 'char *' to
> non-capability, non-address type 'unsigned long' is most likely an error
> [-Werror,-Wcapability-to-integer-cast]
> ??????? syslog(L_FATAL, "%s msync failed %s 0x%lx %d %m", LogName,
> ICDactpath,
> ?????????????? (unsigned long) ICDactpointer, ICDactsize);
> ?????????????? ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
That one should be %p. It would already be broken on a platform with
32-bit long but 64-bit (or longer) pointers.
ttfn/rjk
------------------------------
Subject: Digest Footer
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
------------------------------
End of inn-workers Digest, Vol 154, Issue 2
*******************************************
