Savefile FD records leave `mount_id` uninitialized, and the FD deduplication compares it

[leogr]

Describe the bug

When reading a capture, scap_read_fdlist() reuses a stack-allocated scap_fdinfo for every record without clearing it, and scap_fd_read_from_disk() never sets info.regularinfo.mount_id, since the savefile format does not store it (the dev field is optional too, and it is left as is when the record is too short). sinsp_threadinfo::add_fd_from_scap() then copies that value into sinsp_fdinfo::m_mount_id, and since 0.26.0 the FD table deduplication compares it in sinsp_fdinfo::content_equals().

• libs/userspace/libscap/engine/savefile/scap_savefile.c

  Lines 1592 to 1617 in 1800b33

  	scap_fdinfo fdi;
  	// uint16_t stlen;
  	uint64_t tid;
  	uint32_t padding;
  	//
  	// Read the tid
  	//
  	readsize = r->read(r, &tid, sizeof(tid));
  	CHECK_READ_SIZE_ERR(readsize, sizeof(tid), error);
  	totreadsize += readsize;
  	while(((int32_t)block_length - (int32_t)totreadsize) >= 4) {
  	if(scap_fd_read_from_disk(&fdi, &readsize, block_type, r, error) != SCAP_SUCCESS) {
  	return SCAP_FAILURE;
  	}
  	totreadsize += readsize;
  	//
  	// Add the entry to the table, or fire the notification callback
  	//
  	proclist->m_callbacks.m_proc_entry_cb(proclist->m_callbacks.m_callback_context,
  	error,
  	tid,
  	NULL,
  	&fdi,
  	NULL);

• libs/userspace/libscap/engine/savefile/scap_savefile.c

  Lines 1516 to 1530 in 1800b33

  	case SCAP_FD_FILE_V2:
  	if(r->read(r, &(fdi->info.regularinfo.open_flags), sizeof(uint32_t)) != sizeof(uint32_t)) {
  	return scap_errprintf(error, 0, "error reading the fd info from file (fi1)");
  	}
  	(*nbytes) += sizeof(uint32_t);
  	res = scap_fd_read_fname_from_disk(fdi->info.regularinfo.fname, nbytes, r, error);
  	if(!sub_len || (sub_len < *nbytes + sizeof(uint32_t))) {
  	break;
  	}
  	if(r->read(r, &(fdi->info.regularinfo.dev), sizeof(uint32_t)) != sizeof(uint32_t)) {
  	return scap_errprintf(error, 0, "error reading the fd info from file (dev)");
  	}
  	(*nbytes) += sizeof(uint32_t);
  	break;

• libs/userspace/libsinsp/threadinfo.cpp

  Line 310 in 1800b33

  newfdi->m_mount_id = fdi.info.regularinfo.mount_id;

• libs/userspace/libsinsp/fdinfo.cpp

  Lines 48 to 50 in 1800b33

  	bool sinsp_fdinfo::content_equals(const sinsp_fdinfo& other) const {
  	if(m_type != other.m_type || m_openflags != other.m_openflags || m_flags != other.m_flags ||
  	m_dev != other.m_dev || m_mount_id != other.m_mount_id || m_ino != other.m_ino ||

• libs/userspace/libsinsp/fdtable.cpp

  Line 103 in 1800b33

  if(canonical->content_equals(*info)) {

Valgrind reports it as conditional jumps depending on uninitialised values at sinsp_fdinfo::content_equals() when replaying a capture with Falco 0.45.0-rc3 (libs 0.26.0), with the origin in scap_read_fdlist().

How to reproduce it

Replay a capture containing file FDs under Valgrind, e.g. valgrind --track-origins=yes falco -r <capture> ... with any rule that matches.

Expected behaviour

Fields missing from the capture are deterministically zero for every record, as the live parser does 👉

libs/userspace/libsinsp/parsers.cpp

Line 2022 in 1800b33

fdi->m_mount_id = 0;

Additional context

• The impact is limited: identical FDs may not be deduplicated on replay (memory only), alerts were unchanged in my tests
• The copy in add_fd_from_scap() predates 0.26.0, the comparison is new with the FD deduplication 👉 feat(sinsp): CoW individual fdinfos #3072
• Suggested fix: zero the record at each iteration (not once before the loop, since the union keeps the contents of the previous record types) and add a replay test with mixed record types
• Found during the Falco 0.45.0 testing, not blocking for it

/kind bug
/area libscap
/area libsinsp