conn_fd is leaked in the error path.

Also, 0.0.0.0 is not a valid address. Here it really means, "bind to a wildcard address". Probably 127.0.0.1 is a better example.

Above, the syntax is documented as tcp:0.0.0.0:1234, but here it's tcp=0.0.0.0:1234. Which one is right?

uart_tcp_softc would be a better name in my opinion. ("softc" is an abbreviation for "software context", very common in BSD code.)

Discussing in person, since other backends return errors, we should do the same here too.

You probably want to set the SO_REUSEADDR socket option. Otherwise, if I use the TCP console and reboot the VM, the previous connection will linger and I hit this error.

Also, just use warn() here, instead of warnx(). Then strerror(errno) is automatically appended to the warning. That is, just write: warn("bind()");.

This won't unlock sc->mtx.

Isn't it a race condition when setting is_socket after adding the mevent handler? If the mevent fires very fast, is_socket might be unset. It's very unlikely to hit the race condition but we should avoid it.

Have you addressed this? We should lock sc->mtx on uart_tcp_disconnect, shouldn't we?

I'm unfamiliar with capsicum. Do we have to make sure to limit the rights of bind_fd before this mevent fires?

One reason is that we don't use recursive lock so the caller should take the responsibility to lock it. Also, the caller is always a (read, write) operation, we should have already lock it in the caller.

See comment:

• When connected based protocal disconnected, we use this handler to clean it
• Notice that this function is a helper function thus the caller should
• take the responsibilty to lock the softc

In our condition, we have to let the fd received by accept syscall, e.g.(fd = accept(bind_fd)) have read permission. However, we cannot modify the permission after get into capsicum mode. So the only way to achieve it is to let bind_fd have read permission. And when doing accept system call, fd will inherit all capability from bind_fd.

You are right. Thanks for your kindly remind!

sc->tty.rfd is the mevent fd. mevent_delete_close() schedules the mevent fd to be closed, so are we double-closing the descriptor here?

Indentation here should be by four spaces.

This can be set in uart_tcp_backend(), no?

I set in here as a remind that if we add some protocol (like udp) needs socket, we should set is_socket ourself. This prevent dig into tcp code when adding new protocol. But I think you are right, it is better to put it into tcp_backend

0.0.0.0 comment not addressed, and this reads as if we can access the 800 by 600 pixel console via TCP -- maybe "accessed via VNC at .... and a serial console that can be accessed via TCP at ..."?