Skip to content

Commit 54bd852

Browse files
two-heartripatel-fd
authored andcommitted
vm: polishing syscall
focusing on non-crypto, non-direct mapping syscalls. Resolving FIXMEs, correcting comments, etc.
1 parent 5d600f1 commit 54bd852

File tree

8 files changed

+265
-225
lines changed

8 files changed

+265
-225
lines changed

src/flamenco/log_collector/fd_log_collector.h

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -102,27 +102,33 @@ fd_log_collector_delete( fd_log_collector_t const * log ) {
102102
*/
103103

104104
/* fd_log_collector_msg logs msg of size msg_sz.
105-
This is analogous of Agave's ic_msg!() / ic_logger_msg!().
105+
This is analogous to Agave's ic_msg!() / ic_logger_msg!().
106106
107-
msg is expected to be a valid utf8 string, it's responsibility
107+
Logs are not recorded on-chain, and are therefore not
108+
consensus-critical, however, there exist 3rd party off-chain
109+
applications that parses logs, and expects logs to be equivalent to
110+
agave.
111+
112+
msg is expected to be a valid utf8 string, it is the responsibility
108113
of the caller to enforce that. msg doesn't have to be \0 terminated
109114
and can contain \0 within. Most logs are cstr, base58/64, so
110115
they are utf8. For an example of log from user input, see the
111-
sol_log_() syscall where we use fd_utf8_validate().
116+
sol_log_() syscall where we use fd_utf8_verify().
112117
113118
if msg is a cstr, for compatibility with rust, msg_sz is the msg
114119
length (not the size of the buffer), and the final \0 should not
115-
be included in logs. For literals, use fd_log_collector_msg_literal().
120+
be included in logs. For literals, use
121+
fd_log_collector_msg_literal().
116122
117-
msg_sz==0 is ok, however it's important to understand that log
123+
msg_sz==0 is ok, however, it's important to understand that log
118124
collector is an interface to developers, not exposed to users.
119125
Users can, for example, log inside BPF programs using msg!(), that
120126
gets translated to the syscall sol_log_(), that in turn appends
121127
a log of the form "Program log: ...". So msg_sz, realistically,
122128
is never 0 nor small. This is important for our implementation,
123129
to keep serialization overhead low.
124130
125-
When msg is made of multiple disjoing buffers, we should use
131+
When msg consists of multiple disjoint buffers, we should use
126132
fd_log_collector_msg_many(), and implement more variants as
127133
needed. The core idea is very simple: we know the total msg_sz,
128134
we decide if the log needs to be included or truncated, and
@@ -250,14 +256,14 @@ fd_log_collector_printf_dangerous_max_127( fd_exec_instr_ctx_t * ctx,
250256
int res = vsnprintf( (char *)(buf + buf_sz + 2), FD_LOG_COLLECTOR_PRINTF_MAX_1B, fmt, ap );
251257
va_end( ap );
252258

253-
/* We use vsnprintf to protect against oob writes, however it should never
254-
truncate. If truncate happens, it means that we're using
255-
fd_log_collector_printf_dangerous_max_127(), incorrectly for example
256-
with a "%s" and an unbound variable (user input, var that's not
257-
null-terminated cstr, ...).
259+
/* We use vsnprintf to protect against oob writes, however, it should
260+
never truncate. If truncate happens, it means that we're using
261+
fd_log_collector_printf_dangerous_max_127(), incorrectly for
262+
example with a "%s" and an unbound variable (user input, var that's
263+
not null-terminated cstr, ...).
258264
We MUST only use fd_log_collector_printf_dangerous_max_127()
259-
as a convenince method, when we can guarantee that the total msg_sz is
260-
bound by FD_LOG_COLLECTOR_PRINTF_MAX_1B. */
265+
as a convenience method, when we can guarantee that the total
266+
msg_sz is bound by FD_LOG_COLLECTOR_PRINTF_MAX_1B. */
261267
FD_TEST_CUSTOM( res>=0 && res<FD_LOG_COLLECTOR_PRINTF_MAX_1B,
262268
"A transaction log was truncated unexpectedly. Please report to developers." );
263269

@@ -307,14 +313,14 @@ fd_log_collector_printf_dangerous_128_to_2k( fd_exec_instr_ctx_t * ctx,
307313
va_start( ap, fmt );
308314
int res = vsnprintf( (char *)(buf + buf_sz + 3), FD_LOG_COLLECTOR_PRINTF_MAX_2B, fmt, ap );
309315
va_end( ap );
310-
/* We use vsnprintf to protect against oob writes, however it should never
311-
truncate. If truncate happens, it means that we're using
312-
fd_log_collector_printf_dangerous_max_127(), incorrectly for example
313-
with a "%s" and an unbound variable (user input, var that's not
314-
null-terminated cstr, ...).
315-
We MUST only use fd_log_collector_printf_dangerous_max_127()
316-
as a convenince method, when we can guarantee that the total msg_sz is
317-
bound by FD_LOG_COLLECTOR_PRINTF_MAX_2B. */
316+
/* We use vsnprintf to protect against oob writes, however it should
317+
never truncate. If truncate happens, it means that we're using
318+
fd_log_collector_printf_dangerous_max_127(), incorrectly for
319+
example with a "%s" and an unbound variable (user input, var that's
320+
not null-terminated cstr, ...).
321+
We MUST only use fd_log_collector_printf_dangerous_max_128_to_2k()
322+
as a convenience method, when we can guarantee that the total
323+
msg_sz is bound by FD_LOG_COLLECTOR_PRINTF_MAX_2B. */
318324
FD_TEST_CUSTOM( res>=FD_LOG_COLLECTOR_PRINTF_MAX_1B && res<FD_LOG_COLLECTOR_PRINTF_MAX_2B,
319325
"A transaction log was truncated unexpectedly. Please report to developers." );
320326

@@ -389,10 +395,11 @@ fd_log_collector_program_invoke( fd_exec_instr_ctx_t * ctx ) {
389395
}
390396

391397
/* fd_log_collector_program_log logs:
392-
"Program <ProgramIdBase58> log: <msg>"
398+
"Program log: <msg>"
393399
394400
msg must be a valid utf8 string, it's responsibility of the caller to
395-
validate that. This is the implementation underlying _sol_log() syscall. */
401+
validate that. This is the implementation underlying the _sol_log()
402+
syscall. */
396403
static inline void
397404
fd_log_collector_program_log( fd_exec_instr_ctx_t * ctx, char const * msg, ulong msg_sz ) {
398405
fd_log_collector_msg_many( ctx, 2, "Program log: ", 13UL, msg, msg_sz );

src/flamenco/vm/fd_vm_private.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ struct __attribute__((packed)) fd_vm_vec {
7575

7676
typedef struct fd_vm_vec fd_vm_vec_t;
7777

78+
FD_STATIC_ASSERT( sizeof(fd_vm_vec_t)==FD_VM_VEC_SIZE, fd_vm_vec size mismatch );
79+
7880
/* SBPF version and features
7981
https://github.com/solana-labs/rbpf/blob/4b2c3dfb02827a0119cd1587eea9e27499712646/src/program.rs#L22
8082

src/flamenco/vm/syscall/fd_vm_syscall.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ fd_vm_syscall_register_slot( fd_sbpf_syscalls_t * syscalls,
6565
syscall_cnt++; \
6666
} while(0)
6767

68-
/* Firedancer only (FIXME: HMMMM) */
68+
/* https://github.com/anza-xyz/agave/blob/v2.2.20/programs/bpf_loader/src/syscalls/mod.rs#L392-L396 */
6969

7070
REGISTER( "abort", fd_vm_syscall_abort );
7171
REGISTER( "sol_panic_", fd_vm_syscall_sol_panic );

0 commit comments

Comments
 (0)