CTSRD-CHERI / cheribsd

FreeBSD adapted for CHERI-RISC-V and Arm Morello.
http://cheribsd.org
Other
164 stars 59 forks source link

CheriABI ping(8): unable to open system.dns service: Invalid argument #83

Closed rwatson closed 8 years ago

rwatson commented 8 years ago

This issue picks up where #79 leaves off in debugging why system.dns fails to initialise when using a CheriABI ping(8).

Changes to ping(8) build:

diff --git a/sbin/ping/Makefile b/sbin/ping/Makefile
index 533f8b8..077dec6 100644
--- a/sbin/ping/Makefile
+++ b/sbin/ping/Makefile
@@ -3,6 +3,9 @@

 .include <src.opts.mk>

+WANT_CHERI=pure
+WANT_DUMP=yes
+
 PROG=  ping
 MAN=   ping.8
 BINOWN=        root

Gives:

root@beri1:/tmp # ./ping localhost
ping: unable to open system.dns service: Invalid argument

Without b04dd8a, the error was instead ping: unable to open system.dns service: Cannot allocate memory, which related to CTSRD-CHERI/llvm#144.

rwatson commented 8 years ago

It looks like this is a problem with fcntl(2), in which the third argument, if an integer, contains surprising garbage:

 19942 ping     CALL  fcntl(0x5,F_GETFL,0xfffebd30)
 19942 ping     RET   fcntl 2
 19942 ping     CALL  mmap(0,0x8000,0x3<PROT_READ|PROT_WRITE>,0x1002<MAP_PRIVATE|MAP_ANON>,0xffffffff,0)
 19942 ping     RET   mmap 5908201472/0x160280000
 19942 ping     CALL  fcntl(0x2,F_GETFL,0xfffebeb0)
 19942 ping     RET   fcntl 2
 19942 ping     CALL  fcntl(0x2,F_GETFL,0xfffebaf0)
 19942 ping     RET   fcntl 2
 19942 ping     CALL  fcntl(0x2,F_DUPFD_CLOEXEC,0xfffebaf0)
 19942 ping     RET   fcntl -1 errno 22 Invalid argument

This only matters for fcntls that have integer arguments, and I guess F_DUPFD_CLOEXEC is the first to arise in ping(8).

Userspace appears to do a reasonable conversion:

#ifndef __CHERI_SANDBOX__
#pragma weak fcntl
int
fcntl(int fd, int cmd, ...)
#else
__weak_reference(_fcntl, fcntl);
#pragma weak _fcntl
int
_fcntl(int fd, int cmd, ...)
#endif
{
        va_list args;
        intptr_t arg;

        va_start(args, cmd);
        switch (cmd) {
        case F_GETLK:
        case F_SETLK:
        case F_SETLKW:
                arg = va_arg(args, intptr_t);
                break;

        case F_GETFD:
        case F_GETFL:
        case F_GETOWN:
                arg = 0;
                break;

        default:
                arg = va_arg(args, long);
                break;
        }
        va_end(args);

        return (((int (*)(int, int, intptr_t))
            __libc_interposing[INTERPOS_fcntl])(fd, cmd, arg));
}

On the off chance it was a disagreement between long and int, I tried changing the default va_args(args, long) into va_args(args, int) but no change in behaviour.

rwatson commented 8 years ago

(on a related note, it's interesting that my addition of arg = 0; for F_GETFL and friends appears not to have led to the kernel seeing 0 as the third argument either, although it no longer crashes.)

rwatson commented 8 years ago

This might be occurring because CheriABI is using CToPtr to convert userspace-originated intptr_ts into kernel intptr_ts. CToPtr will generate NULL if the originating user capability is untagged, which an integer interpretation will generate. Likely, CheriABI needs to use the non-present CToInt instruction, or more realistically for us, (CGetBase + CGetOffset), ignoring the tag, for arguments that will be interpreted as integers. In some ways this is slightly awkward, as the current CToPtr interpretation of user pointers ensured that the kernel got back NULL for untagged user pointers (which is not a perfect result, but an interesting one), but probably better to work than not in a world where we convert to kernel-accessible user pointers from capabilities transparently.

(Although this is likely one of the things happening here, it might not actually explain what we are seeing, which is other negative-linking integer values in ktrace, rather than 0 for all integer interpretations of intptr_t arguments?)

rwatson commented 8 years ago

Tagging @davidchisnall and @michael-roe as this may be another case where an explicit CToInt might be desirable.

rwatson commented 8 years ago

ping(8) now fails with a new error: ping: unable to open system.dns service: Bad address, which appears to relate to a problem in sendmsg(2). I will open a new issue for that to track it separately.