diff --git a/cmd/socktest/main.c b/cmd/socktest/main.c index ed513ad..d50b819 100644 --- a/cmd/socktest/main.c +++ b/cmd/socktest/main.c @@ -12,6 +12,8 @@ #include "types.h" #include "user.h" #include "socket.h" +#include "stat.h" +#include "fcntl.h" static int failures; @@ -54,6 +56,12 @@ streq(const char *a, const char *b) // Fill a sockaddr_un and return the length to pass to bind/connect: the // family plus the path plus its NUL, which is what musl computes and what the // kernel side is written to expect. +// Every bind() here is preceded by an unlink(), which is not tidiness: a bound +// socket is a *file* now, and it outlives the process that bound it. Rebinding +// a path that already has a node is EADDRINUSE, exactly as on Linux, so every +// real server removes the stale one first. Before this change the name lived +// only in the kernel's socket table and vanished when the process did, so a +// test could bind the same path on every run and never notice. static int mkaddr(struct sockaddr_un *sa, const char *path) { @@ -72,7 +80,10 @@ static void test_roundtrip(void) { struct sockaddr_un sa; - int len = mkaddr(&sa, "/tmp/socktest"); + int len; + + unlink("/tmp/socktest"); // a stale node from an earlier run + len = mkaddr(&sa, "/tmp/socktest"); int srv, pid, cli, n; char buf[64]; @@ -140,7 +151,10 @@ static void test_errors(void) { struct sockaddr_un sa; - int len = mkaddr(&sa, "/tmp/nobody-listening"); + int len; + + unlink("/tmp/nobody-listening"); // a stale node from an earlier run + len = mkaddr(&sa, "/tmp/nobody-listening"); int s; puts_("error paths:\n"); @@ -181,7 +195,10 @@ static void test_eof(void) { struct sockaddr_un sa; - int len = mkaddr(&sa, "/tmp/socktest-eof"); + int len; + + unlink("/tmp/socktest-eof"); // a stale node from an earlier run + len = mkaddr(&sa, "/tmp/socktest-eof"); int srv, cli, pid, n; char buf[16]; @@ -743,6 +760,76 @@ test_fdpass_nodata(void) close(p[0]); close(sv[0]); close(sv[1]); } +// --------------------------------------------------------------------------- +// 15. A bound socket is a file. +// +// docs/plan/foundations.md item 3, in its own words: bind() a socket, then +// stat() and `ls -l` the path and see an `s`, then unlink() it. +// +// The reason it matters is not tidiness. A Wayland client finds its compositor +// by bind()ing a real path and libwayland locks the file beside it; a name +// table in the kernel can express neither. These checks are what a name table +// cannot pass -- every one of them asks the *filesystem* about the socket, +// not the socket layer. +// --------------------------------------------------------------------------- +static void +test_fsnames(void) +{ + struct sockaddr_un sa; + struct stat st; + int len, sv, c; + + puts_("a bound socket is a file:\n"); + + unlink("/tmp/socktest-fs"); // from a previous run + + sv = socket(AF_UNIX, SOCK_STREAM, 0); + len = mkaddr(&sa, "/tmp/socktest-fs"); + check(sv >= 0, "socket()"); + check(bind(sv, (struct sockaddr *)&sa, len) == 0, "bind creates the path"); + check(stat("/tmp/socktest-fs", &st) == 0, "and it can be stat()ed"); + check(st.type == T_SOCK, "as a socket, not a regular file"); + + // Binding the same path twice is EADDRINUSE, and it has to come from the + // directory -- a socket file left by a crashed server must refuse a + // rebind for the same reason a running one does. + { + int s2 = socket(AF_UNIX, SOCK_STREAM, 0); + check(s2 >= 0 && bind(s2, (struct sockaddr *)&sa, len) < 0, + "a second bind to the same path is refused"); + if (s2 >= 0) close(s2); + } + + // Connecting to a path that is not a socket must be refused rather than + // matched by name. A name table could not tell the difference. + { + struct sockaddr_un sb; + int lb = mkaddr(&sb, "/tmp/socktest-notasock"); + int fd = open("/tmp/socktest-notasock", O_CREATE | O_RDWR); + if (fd >= 0) close(fd); + c = socket(AF_UNIX, SOCK_STREAM, 0); + check(c >= 0 && connect(c, (struct sockaddr *)&sb, lb) < 0, + "connecting to a regular file is refused"); + if (c >= 0) close(c); + unlink("/tmp/socktest-notasock"); + } + + check(unlink("/tmp/socktest-fs") == 0, "unlink removes it"); + check(stat("/tmp/socktest-fs", &st) < 0, "and it is gone from the filesystem"); + + // The path is free again. On Linux the old socket keeps working for + // anyone already connected; it is simply no longer findable by name. + { + int s3 = socket(AF_UNIX, SOCK_STREAM, 0); + check(s3 >= 0 && bind(s3, (struct sockaddr *)&sa, len) == 0, + "and the path is free to bind again"); + if (s3 >= 0) close(s3); + unlink("/tmp/socktest-fs"); + } + + close(sv); +} + int main(void) { @@ -761,6 +848,7 @@ main(void) test_fdpass_orphaned(); test_fdpass_truncated(); test_fdpass_nodata(); + test_fsnames(); if (failures == 0) puts_("socktest: ALL CHECKS PASSED\n"); diff --git a/docs/architecture/networking.md b/docs/architecture/networking.md index 1e5b2a2..31f28fa 100644 --- a/docs/architecture/networking.md +++ b/docs/architecture/networking.md @@ -170,11 +170,41 @@ two-object locking needs an ordering rule every path obeys or it deadlocks. Worse, the wakeup races are subtle — a reader testing `peer_closed` under its own lock while the closer sets it under the peer's has no interlock at all. -**Names live in a table here, not in the filesystem.** Binding creates no -directory entry. The table gives the behaviour that matters — a client finds a -server by a name agreed in advance — and what is lost is `ls -l` showing an `s`, -and `unlink()` of a socket path working. Both want a VFS change rather than a -socket change. +**A bound socket is a file.** `bind()` creates a `T_SOCK` node at the path, and +`connect()` resolves the path and looks for the socket bound to **that inode**. + +Matching on the inode rather than on the path *string* is the part that +matters. The name used to live only in this table, so two spellings of one path +missed each other, a relative path never matched an absolute one, and there was +nothing to `stat()` or `unlink()`. An inode is the thing a path resolves *to* — +so `connect()` can walk the path however it likes and still arrive at the same +socket, which is precisely what a string comparison could never do. + +``` +# ls -l /tmp +srwxr-xr-x 1 root root 0 2026-09-22 08:04 socktest +``` + +The node is a **marker, not the binding**. It holds no data, and the live +association is in this table. So a socket file left behind by a crash is what +Linux leaves behind: a dead file that `connect()` refuses until something binds +to it again. Conversely, `unlink()`ing a live socket's path does not disturb +anyone already connected — the socket simply stops being findable. + +`EADDRINUSE` comes from the *directory*, not from a scan of this table, and +that is deliberate: a stale socket file has to refuse a rebind for the same +reason a running server's does, or two servers both believe they own it. The +consequence is the one every real server already handles — `unlink()` before +`bind()`, which `cmd/socktest` now does too. + +The creation lives in `sys_bind()` rather than here, because that is the layer +with both: `kernel/net/unix.c` knows about sockets and has no business calling +`create()`. + +**`flock()` is still missing.** libwayland locks `wayland-0.lock` beside the +socket so that two compositors cannot claim one display. The lock file can be +created and opened now that paths are real; the advisory lock itself is a +separate mechanism with no other caller yet. ### Descriptor passing diff --git a/docs/plan/1.0.md b/docs/plan/1.0.md index dfaa9af..87bfebf 100644 --- a/docs/plan/1.0.md +++ b/docs/plan/1.0.md @@ -436,7 +436,14 @@ Nothing above needs these; everything below stops without them. System V key, which is the whole reason it exists on Linux. `cmd/memfdtest`: seventeen checks, ending with a memfd sent over a socket to a receiver that closes every inherited copy first. -- [ ] **Item 3 — `AF_UNIX` sockets in the filesystem namespace.** +- [x] ~~**Item 3 — `AF_UNIX` sockets in the filesystem namespace.**~~ `bind()` + creates a `T_SOCK` node; `connect()` resolves the path and matches the + socket bound to *that inode*, not to a string. `ls -l` shows + `srwxr-xr-x`, `stat()` reports `S_IFSOCK`, `unlink()` removes it, and a + second bind to a live path is `EADDRINUSE` *from the directory* — which + is what makes a socket file left by a crashed server refuse a rebind, as + it must. `cmd/socktest` section 15. `flock()` is still missing and is + recorded as such. ### Gate 2 — Init as a role — **done** diff --git a/docs/plan/foundations.md b/docs/plan/foundations.md index bf97da1..02d3157 100644 --- a/docs/plan/foundations.md +++ b/docs/plan/foundations.md @@ -97,12 +97,18 @@ carries `shmid` and a `no_free` flag for pages that are not ours to release). The general answer — a page cache, and writable `MAP_SHARED` of a real file — stays owed. It is a larger project and nothing on this plan needs it. -### 3. `AF_UNIX` sockets in the filesystem namespace +### 3. `AF_UNIX` sockets in the filesystem namespace — **done** Clients find a Wayland compositor by `bind()`ing `$XDG_RUNTIME_DIR/wayland-0` as a real path, and libwayland `flock()`s the lock file beside it. A name table can express neither. +`bind()` creates a `T_SOCK` node and `connect()` resolves the path and matches +the socket bound to **that inode** — not to a string, which is what made two +spellings of one path miss each other. `ls -l` shows `srwxr-xr-x`, `stat()` +reports `S_IFSOCK`, and `unlink()` removes it. Asserted by `cmd/socktest`, +section 15. `flock()` is still not implemented; see below. + `kernel/net/unix.c`'s own header comment already predicted this: > *"What is lost is `ls -l /tmp/schcom` showing an `s`, and `unlink()` of a socket @@ -159,3 +165,7 @@ These belong in `bootcheck` as a new switch, following the `-Sock` model. - **`SO_*` coverage is the two options named**, not the full set. - **No credential passing** (`SCM_CREDENTIALS`) — only descriptors. Nothing on this plan needs it, and a compositor that wants to know who connected will. +- **No `flock()`.** libwayland locks `wayland-0.lock` beside the socket to stop + two compositors claiming one display. The socket itself is now a real path, + so the lock file can be created and opened; what is missing is the advisory + lock, which is a separate mechanism and has no other caller yet. diff --git a/include/defs.h b/include/defs.h index 92ea608..166e4ac 100644 --- a/include/defs.h +++ b/include/defs.h @@ -164,6 +164,10 @@ uint tree_expand(struct inode *ip, uint off); void tree_search(struct inode* dp, char* name, uint* poff, struct dirent* de,uint add); void build_dir_tree(struct inode* dp,uint64 pinum); struct inode* divesymlink(struct inode *); +// A socket node for AF_UNIX bind(2), and unlink by path for the failure that +// follows it. See the notes on each in kernel/sys/sys_fs.c. +struct inode* create_sock(char *path); +int unlink_path(char *path); void readsb(int dev, struct superblock *sb); // pci.c -- bus enumeration. Drivers that need to *find* their hardware use @@ -469,10 +473,10 @@ int sockwrite(struct socket*, char*, int); // The same two, carrying descriptors. See kernel/net/unix.c. int sockread_fds(struct socket*, char*, int, struct file**, int*); int sockwrite_fds(struct socket*, char*, int, struct file**, int); -int sockbind(struct socket*, const char *path); +int sockbind(struct socket*, const char *path, uint dev, uint64 ino); int socklisten(struct socket*, int backlog); struct socket* sockaccept(struct socket*); -int sockconnect(struct socket*, const char *path); +int sockconnect(struct socket*, uint dev, uint64 ino); int sockreadable(struct socket*); int sock_hup(struct socket*); int sock_shutdown(struct socket*, int how); diff --git a/include/schfs.h b/include/schfs.h index 21097ce..47fb00a 100644 --- a/include/schfs.h +++ b/include/schfs.h @@ -293,6 +293,11 @@ _Static_assert(__builtin_offsetof(struct schfs_uberblock, next_objectid) == 72, #define SCHFS_T_DIR 2 #define SCHFS_T_DEV 3 #define SCHFS_T_SYMLINK 4 +// A bound AF_UNIX socket. A *compat* addition by the rule above: a driver that +// does not know this value maps it to SCHFS_T_FILE and sees a zero-length +// regular file, which is harmless -- nothing goes stale and nothing becomes +// unreadable, so it needs no feature flag. +#define SCHFS_T_SOCK 5 struct schfs_inode { uint64_t size; diff --git a/include/socket.h b/include/socket.h index 3b2e6e2..d12eb26 100644 --- a/include/socket.h +++ b/include/socket.h @@ -309,7 +309,21 @@ struct socket { // wants a struct socket. void *inet; - char name[UNIX_PATH_MAX]; // the bound path, for a listener + char name[UNIX_PATH_MAX]; // the bound path, for a listener and getsockname + // The filesystem node bind() created, by identity rather than by name. + // + // Matching on the *path string* was what made these sockets invisible to the + // filesystem: two spellings of one path missed each other, a relative path + // never matched an absolute one, and there was nothing to stat() or + // unlink(). An inode is the thing a path resolves *to*, so connect() can + // walk the path however it likes and still arrive at the same socket. + // + // bind_dev 0 means "no node": a socket bound before this existed, or one + // whose file has been unlinked. The socket stays bound and stays usable by + // anyone already connected; it is simply no longer findable, which is + // exactly what Linux does. + uint bind_dev; + uint64 bind_ino; // A connected socket's peer. Writing to a socket appends to the *peer's* // buffer, which is what makes one buffer per socket enough for a diff --git a/include/stat.h b/include/stat.h index 9152d0c..2f2f42e 100644 --- a/include/stat.h +++ b/include/stat.h @@ -6,6 +6,13 @@ #define T_DEV 3 // Device #define T_SYMLINK 4 // Divesymlink #define T_EXTENT 5 // bmap & itrunc +// A bound AF_UNIX socket. The node is a *marker*: it is what makes a socket +// findable by path, stat()able and unlink()able, and it holds no data. The +// live binding lives in kernel/net/unix.c and is matched by (dev, inum), not +// by this node's contents -- so a socket file left behind by a crash is +// exactly what Linux leaves behind, a dead file that connect() refuses until +// something binds to it again. +#define T_SOCK 6 struct stat { int32 type; // File type (T_DIR, T_FILE, or T_DEV) diff --git a/kernel/fs/schfs/kmount.c b/kernel/fs/schfs/kmount.c index 9880bae..91f36e5 100644 --- a/kernel/fs/schfs/kmount.c +++ b/kernel/fs/schfs/kmount.c @@ -1203,6 +1203,7 @@ kp_entry(uint64_t ino, uint16_t type, const char *name, unsigned namelen, kp_puts(c->buf, c->cap, c->len, type == SCHFS_T_DIR ? "dir " : type == SCHFS_T_SYMLINK ? "symlink " : + type == SCHFS_T_SOCK ? "socket " : type == SCHFS_T_DEV ? "dev " : "file "); for (unsigned i = 0; i < namelen && *c->len < c->cap - 1; i++) c->buf[(*c->len)++] = name[i]; diff --git a/kernel/fs/schfs/vfs.c b/kernel/fs/schfs/vfs.c index 084f3f5..18b68e3 100644 --- a/kernel/fs/schfs/vfs.c +++ b/kernel/fs/schfs/vfs.c @@ -53,6 +53,7 @@ type_to_vfs(uint16_t t) case SCHFS_T_DIR: return T_DIR; case SCHFS_T_DEV: return T_DEV; case SCHFS_T_SYMLINK: return T_SYMLINK; + case SCHFS_T_SOCK: return T_SOCK; default: return T_FILE; } } @@ -64,6 +65,7 @@ type_from_vfs(short t) case T_DIR: return SCHFS_T_DIR; case T_DEV: return SCHFS_T_DEV; case T_SYMLINK: return SCHFS_T_SYMLINK; + case T_SOCK: return SCHFS_T_SOCK; default: return SCHFS_T_FILE; } } @@ -434,6 +436,7 @@ gd_emit(struct gd_ctx *g, uint64 ino, unsigned short type, *(uint8 *)(k + 18) = type == SCHFS_T_DIR ? 4 /* DT_DIR */ : type == SCHFS_T_DEV ? 2 /* DT_CHR */ : type == SCHFS_T_SYMLINK ? 10 /* DT_LNK */ + : type == SCHFS_T_SOCK ? 12 /* DT_SOCK */ : 8; /* DT_REG */ memmove(k + 19, name, namelen); memset(k + 19 + namelen, 0, reclen - 19 - namelen); diff --git a/kernel/net/unix.c b/kernel/net/unix.c index 413c481..3da7d59 100644 --- a/kernel/net/unix.c +++ b/kernel/net/unix.c @@ -133,12 +133,12 @@ alloc_locked(int type) // Find a socket listening on this name. Caller holds stab.lock. static struct socket * -find_listener(const char *name) +find_listener_ino(uint dev, uint64 ino) { struct socket *s; for(s = stab.sock; s < stab.sock + NSOCK; s++) - if(s->state == SS_LISTENING && strncmp(s->name, name, UNIX_PATH_MAX) == 0) + if(s->state == SS_LISTENING && s->bind_dev == dev && s->bind_ino == ino) return s; return 0; } @@ -656,11 +656,22 @@ sockpair(struct socket **pa, struct socket **pb) return 0; } +// bind(2) for AF_UNIX. The caller has already created the filesystem node and +// hands over its identity; see sys_bind() in kernel/sys/sys_net.c for why the +// creation lives there and not here. +// +// The name used to live only in this table, so `bind()` created nothing -- +// there was no file to stat(), no `s` in `ls -l`, and nothing to unlink. A +// Wayland client finds its compositor by bind()ing a real path, and +// libwayland locks the file beside it; a name table can express neither. +// +// **The node is a marker, not the binding.** It holds no data. What makes a +// socket findable is that connect() resolves the path to an inode and looks +// for the socket bound to *that inode*, so any spelling of the path arrives +// at the same place -- which a string comparison could never manage. int -sockbind(struct socket *s, const char *path) +sockbind(struct socket *s, const char *path, uint dev, uint64 ino) { - struct socket *other; - if(path[0] == '\0') return -EINVAL; @@ -669,14 +680,9 @@ sockbind(struct socket *s, const char *path) release(&stab.lock); return -EINVAL; } - for(other = stab.sock; other < stab.sock + NSOCK; other++){ - if(other != s && other->state >= SS_BOUND && - strncmp(other->name, path, UNIX_PATH_MAX) == 0){ - release(&stab.lock); - return -EADDRINUSE; - } - } safestrcpy(s->name, path, UNIX_PATH_MAX); + s->bind_dev = dev; + s->bind_ino = ino; s->state = SS_BOUND; release(&stab.lock); return 0; @@ -753,8 +759,11 @@ sockaccept(struct socket *s) return c; } +// connect(2) for AF_UNIX. The listener is the socket bound to the inode the +// path resolved to -- the caller did the resolving, for the same reason +// sockbind() does not create the node. int -sockconnect(struct socket *s, const char *path) +sockconnect(struct socket *s, uint dev, uint64 ino) { struct socket *srv, *peer; @@ -763,7 +772,7 @@ sockconnect(struct socket *s, const char *path) release(&stab.lock); return -EINVAL; } - srv = find_listener(path); + srv = find_listener_ino(dev, ino); if(srv == 0){ release(&stab.lock); return -ECONNREFUSED; diff --git a/kernel/sys/sys_fs.c b/kernel/sys/sys_fs.c index bc3f711..2ffe86a 100644 --- a/kernel/sys/sys_fs.c +++ b/kernel/sys/sys_fs.c @@ -52,6 +52,7 @@ struct linux_stat { #define LINUX_S_IFCHR 0020000 #define LINUX_S_IFREG 0100000 #define LINUX_S_IFLNK 0120000 +#define LINUX_S_IFSOCK 0140000 // Linux's dev_t: 12 bits of major and 8 of minor packed into the low 32, with // the overflow of each above that. Spelled out in full rather than as @@ -87,6 +88,7 @@ stati_linux(struct inode *ip, struct linux_stat *st) case T_DIR: type_bits = LINUX_S_IFDIR; perm_bits = 0755; break; case T_DEV: type_bits = LINUX_S_IFCHR; perm_bits = 0666; break; case T_SYMLINK: type_bits = LINUX_S_IFLNK; perm_bits = 0777; break; + case T_SOCK: type_bits = LINUX_S_IFSOCK; perm_bits = 0755; break; default: type_bits = LINUX_S_IFREG; perm_bits = (ip->rwmode & 2) ? 0755 : 0555; break; } st->st_mode = type_bits | perm_bits; @@ -812,29 +814,61 @@ bad_link: // is the standard shape -- and against -1/EPERM every such test took the // complaining branch. zsh saving its history is one of them, and it went on // reporting a failure after the history had in fact been written. -addr_t sys_unlink(void) +// A socket node, for bind(2). +// +// create() is static because everything that creates a file is in this file; +// AF_UNIX bind is the one exception, and it gets a named door rather than +// create() being exported -- the difference matters, because `create` with an +// arbitrary type is a much bigger thing to hand out than "make me a socket". +// +// Returns a locked inode, like create() does. +struct inode * +create_sock(char *path) +{ + return create(path, T_SOCK, 0, 0); +} + +// The body of unlink(2), by path. Factored out so that a failed bind can take +// its own node back out without going through the syscall entry, and so there +// is one implementation of what unlink means rather than two. +// +// The caller brackets this with begin_op()/end_op(), the same as sys_unlink. +int +unlink_path(char *path) { struct inode *ip, *dp; - char name[DIRSIZ], *path; + char name[DIRSIZ]; uint off; int err = EPERM; - if (argstr(0, &path) < 0) return -EFAULT; - begin_op(); - if (!(dp = nameiparent(path, name))) { end_op(); return -ENOENT; } + + if (!(dp = nameiparent(path, name))) return -ENOENT; ilock(dp); - // "." and ".." are refused by name, which is what POSIX asks for. - if (namecmp(name, ".") == 0 || namecmp(name, "..") == 0) { err = EINVAL; goto bad_unlink; } - if (!(ip = dirlookup(dp, name, &off))) { err = ENOENT; goto bad_unlink; } + if (namecmp(name, ".") == 0 || namecmp(name, "..") == 0) { err = EINVAL; goto bad; } + if (!(ip = dirlookup(dp, name, &off))) { err = ENOENT; goto bad; } ilock(ip); if (ip->nlink < 1) panic("unlink: nlink < 1"); - if (ip->type == T_DIR && dirempty(ip) != 1) { iunlockput(ip); err = ENOTEMPTY; goto bad_unlink; } - if (dirunlink(dp, name, off) < 0) { iunlockput(ip); err = EIO; goto bad_unlink; } + if (ip->type == T_DIR && dirempty(ip) != 1) { iunlockput(ip); err = ENOTEMPTY; goto bad; } + if (dirunlink(dp, name, off) < 0) { iunlockput(ip); err = EIO; goto bad; } if (ip->type == T_DIR) { dp->nlink--; iupdate(dp); } iunlockput(dp); ip->nlink--; iupdate(ip); iunlockput(ip); - end_op(); return 0; -bad_unlink: iunlockput(dp); end_op(); return -err; + return 0; +bad: + iunlockput(dp); + return -err; } +addr_t sys_unlink(void) +{ + char *path; + int r; + if (argstr(0, &path) < 0) return -EFAULT; + begin_op(); + r = unlink_path(path); + end_op(); + return r; +} + + addr_t sys_exec(void) { char *path, *argv[MAXARG], *envp[MAXENV]; diff --git a/kernel/sys/sys_net.c b/kernel/sys/sys_net.c index 5b0772b..f84a0d0 100644 --- a/kernel/sys/sys_net.c +++ b/kernel/sys/sys_net.c @@ -19,6 +19,7 @@ #include "fs.h" #include "spinlock.h" #include "sleeplock.h" +#include "stat.h" #include "file.h" #include "posix.h" #include "syscall.h" @@ -241,7 +242,46 @@ SYSCALL_DEFINE(bind) if ((r = fetch_sun_path(uaddr, ulen, path)) < 0) return r; - return sockbind(s, path); + + // The node lives in the filesystem now, and it is made here rather than in + // sockbind() because this is the layer that has both -- kernel/net/unix.c + // knows about sockets and has no business calling create(). + // + // EADDRINUSE comes from the directory, not from a scan of the socket + // table: the name being taken is a property of the filesystem, and a + // socket file left behind by a crashed server has to refuse a rebind for + // the same reason a running one does. Otherwise two servers both believe + // they own it. + { + struct inode *ip; + uint dev; + uint64 ino; + + begin_op(); + if ((ip = namei(path)) != 0) { + iput(ip); + end_op(); + return -EADDRINUSE; + } + if ((ip = create_sock(path)) == 0) { + end_op(); + return -ENOENT; // the directory does not exist + } + dev = ip->dev; + ino = ip->inum; + iunlockput(ip); + end_op(); + + r = sockbind(s, path, dev, ino); + if (r < 0) { + // Take the node back out, so a failed bind leaves nothing behind + // for the next one to trip over. + begin_op(); + unlink_path(path); + end_op(); + } + return r; + } } SYSCALL_DEFINE(listen) @@ -277,7 +317,27 @@ SYSCALL_DEFINE(connect) if ((r = fetch_sun_path(uaddr, ulen, path)) < 0) return r; - return sockconnect(s, path); + + // Resolve the path here, for the same reason bind() creates here. A path + // that names nothing, or names something that is not a socket, is + // ECONNREFUSED -- which is what Linux says and what a client can act on. + { + struct inode *ip; + uint dev; + uint64 ino; + short type; + + if ((ip = namei(path)) == 0) + return -ECONNREFUSED; + ilock(ip); + dev = ip->dev; + ino = ip->inum; + type = ip->type; + iunlockput(ip); + if (type != T_SOCK) + return -ECONNREFUSED; + return sockconnect(s, dev, ino); + } } // accept(2) and accept4(2) share everything but the flags word. diff --git a/lib/glue.c b/lib/glue.c index 2243cbf..fe5cf34 100644 --- a/lib/glue.c +++ b/lib/glue.c @@ -4,12 +4,20 @@ #include "types.h" #include "user.h" -extern void _xv6_exit(void); +// The raw syscall. It takes the status in %rdi, and both callers below must +// pass one: this declaration used to be `void _xv6_exit(void)`, so the status +// was whatever the caller happened to leave in that register. Every program +// that ended with a bare exit() reported a random number, which is why +// `if socktest; then` could not work for any of them. +extern void _xv6_exit(int status); +// exit(void) in this library means exit(0). A program that wants to report +// something returns it from main -- lib/start.S passes that through now -- or +// calls exit_group(n). void exit(void) { - _xv6_exit(); + _xv6_exit(0); for (;;); } @@ -84,7 +92,9 @@ memcmp(const void *a, const void *b, unsigned long n) void __stack_chk_fail(void) { - _xv6_exit(); + // A smashed stack is not a successful run. 134 is what a shell reports for + // SIGABRT, which is the nearest true thing this can say. + _xv6_exit(134); for (;;); } diff --git a/lib/start.S b/lib/start.S index 6bf33c4..f9dd4c8 100644 --- a/lib/start.S +++ b/lib/start.S @@ -39,6 +39,18 @@ _start: andq $-16, %rsp callq main /* main(%rdi = argc, %rsi = argv) */ - callq exit /* exit() is noreturn, but be certain */ + + /* main's return value is the exit status, and it used to be thrown away. + * + * sys_exit() reads the status from %rdi, and nothing put it there -- so + * every first-party program exited with whatever happened to be in that + * register, and `if socktest; then` could not work for any of them. The + * test suites all end in `return failures ? 1 : 0` and none of it reached + * the shell. + * + * %eax, not %rax: the status is the low eight bits, and a `return 0` from + * a C function leaves only %eax defined. */ + movl %eax, %edi + callq exit_group /* noreturn, but be certain */ 1: hlt jmp 1b