runtime: sandbox working on testnet full client #6323
Conversation
ibhatt-jumptrading
requested review from
nbridge-jump,
ripatel-fd and
akhinvasara-jumptrading
as code owners
src/app/firedancer/topology.c
Outdated
| @@ -735,8 +735,10 @@ fd_topo_initialize( config_t * config ) { | |||
|
|
|||
| fd_topo_tile_t * poh_tile = &topo->tiles[ fd_topo_find_tile( topo, "poh", 0UL ) ]; | |||
| fd_topo_tile_t * pack_tile = &topo->tiles[ fd_topo_find_tile( topo, "pack", 0UL ) ]; | |||
| fd_topo_tile_t * bank_tile = &topo->tiles[ fd_topo_find_tile( topo, "bank", i ) ]; | |||
| fd_topob_tile_uses( topo, poh_tile, busy_obj, FD_SHMEM_JOIN_MODE_READ_WRITE ); | |||
There was a problem hiding this comment.
Remove this one then? Not sure why poh needs read/write
src/discof/ipecho/fd_ipecho_server.c
Outdated
| @@ -310,7 +310,17 @@ void | |||
| fd_ipecho_server_poll( fd_ipecho_server_t * server, | |||
| int * charge_busy, | |||
| int timeout_ms ) { | |||
| int nfds = fd_syscall_poll( server->pollfds, (uint)( server->max_connection_cnt+1UL ), timeout_ms ); | |||
|
|
|||
| /* Will look for first fd==-1 */ | |||
There was a problem hiding this comment.
?? Don't do this lol. Why can't it just use max_connection_cnt ?
There was a problem hiding this comment.
it fails with FD_LOG_ERR if any of the pollfds are ==-1
| (eq (arg 1) "SOCK_STREAM|SOCK_NONBLOCK") | ||
| (eq (arg 2) 0)) | ||
|
|
||
| # FIXME: We can have up to 16 so [0, 17] execept 2 and 4 ?????? |
There was a problem hiding this comment.
Almost all of these can be removed by moving the init functions into privileged_init, they are called once only
| @@ -199,7 +199,7 @@ echo " | |||
| [development] | |||
| sandbox = false | |||
There was a problem hiding this comment.
sandbox = true ?
You can just remove all these options, they are already the defaults
src/discof/ipecho/fd_ipecho_tile.c
Outdated
| fd_topo_tile_t const * tile FD_PARAM_UNUSED ) { | ||
| /* stderr, logfile, and one for each socket() call for up to 16 | ||
| gossip entrypoints (GOSSIP_TILE_ENTRYPOINTS_MAX). */ | ||
| return 18UL; |
There was a problem hiding this comment.
Pull the actual number of gossip entrypoints out here so it's a tight limit
| # | ||
| # arg 0 is the file descirptor for the file descriptor to assign. | ||
| # Restrict the logfile and STDERR for the same reasons as above | ||
| bind: (not (or (eq (arg 0) logfile_fd) |
There was a problem hiding this comment.
Not sure you saw my previous comment, but these should not be allowed. Move the bind/connect etc calls into privileged init and you can remove all this stuff except send/recv
There was a problem hiding this comment.
I believe the funk txn lock right now is declared statically in fd_funk_txn.c and lives in the .bss section. Sandboxing is going to make the lock per-tile and neuter it. Might make sense to move the lock into somewhere shareable like fd_funk_t. Admittedly the lock isn't used correctly, but that's another can of worms.
No description provided.