updates from codex security scan and testing

Makefile

@@ -73,14 +73,14 @@ mavlink.o: mavlink.cpp mavlink.h $(MAVLINK_DIR)/protocol.h
 
 # Dependencies. mavlink.h includes keydb.h, so any object that pulls in
 # mavlink.h transitively depends on keydb.h too.
-supportproxy.o: supportproxy.cpp mavlink.h util.h keydb.h conntdb.h tlog.h session.h cleanup.h websocket.h
+supportproxy.o: supportproxy.cpp mavlink.h util.h keydb.h conntdb.h tlog.h binlog.h session.h cleanup.h websocket.h
 mavlink.o: mavlink.cpp mavlink.h keydb.h $(MAVLINK_DIR)/protocol.h
 util.o: util.cpp util.h
 keydb.o: keydb.cpp keydb.h
 conntdb.o: conntdb.cpp conntdb.h
 tlog.o: tlog.cpp tlog.h session.h
 session.o: session.cpp session.h
-binlog.o: binlog.cpp binlog.h session.h mavlink.h util.h $(MAVLINK_DIR)/protocol.h
+binlog.o: binlog.cpp binlog.h session.h mavlink.h util.h cleanup.h $(MAVLINK_DIR)/protocol.h
 cleanup.o: cleanup.cpp cleanup.h keydb.h
 websocket.o: websocket.cpp websocket.h util.h
 

binlog.cpp

@@ -5,6 +5,7 @@
 #include "session.h"
 #include "mavlink.h"
 #include "util.h"
+#include "cleanup.h"
 
 #include "libraries/mavlink2/generated/all/mavlink.h"
 
@@ -139,7 +140,9 @@ void BinlogWriter::refresh_other_sessions_bytes()
                     continue;
                 }
             }
-            other_sessions_bytes_ += st.st_size;
+            // allocated size, not apparent: sparse .bin files would
+            // otherwise overstate their disk cost and starve the quota
+            other_sessions_bytes_ += off_t(st.st_blocks) * 512;
         }
         closedir(dd);
     }
@@ -184,6 +187,19 @@ void BinlogWriter::handle_block(uint32_t port2, unsigned session_n,
     mavlink_remote_log_data_block_t blk {};
     mavlink_msg_remote_log_data_block_decode(&msg, &blk);
 
+    // Vehicle-side log restart detected from the data itself: the
+    // vehicle rebooted (or its client-timeout fired) and began a new
+    // log from block 0 before we saw the SYSTEM_TIME backward jump.
+    // Without this, block 0 overwrites the head of the old session
+    // file, and when SYSTEM_TIME finally shows the jump the rotation
+    // waits for a fresh seqno=0 that never comes.
+    if (fp != nullptr && blk.seqno == 0
+        && highest_seen >= SEQNO0_RESTART_MIN_HIGHEST) {
+        ::printf("binlog: seqno=0 with highest_seen=%u — vehicle log "
+                 "restart, rotating\n", unsigned(highest_seen));
+        rotate_for_reboot();
+    }
+
     // Strict-start gate. Without this we sparse-extend the file out
     // to whatever the vehicle's current seqno is — a vehicle that
     // was already streaming when SupportProxy activated will have
@@ -196,13 +212,25 @@ void BinlogWriter::handle_block(uint32_t port2, unsigned session_n,
     // vehicle's new boot sends seqno=0 to start the new file.
     if (fp == nullptr) {
         if (blk.seqno != 0) {
+            // vehicle is streaming mid-log at a closed file: only a
+            // restart from seqno 0 can unblock it. tick() sends STOP
+            // to trigger that promptly. Latch the sender's identity
+            // so the STOP is targeted, not broadcast.
+            if (target_system == 0) {
+                target_system = msg.sysid;
+                target_component = msg.compid;
+            }
+            if (gated_block_count_ < STOP_MIN_GATED_BLOCKS) {
+                gated_block_count_++;
+            }
             return;
         }
         unsigned n = (pending_session_n_ != 0) ? pending_session_n_ : session_n;
         if (!open(port2, n)) {
             return;
         }
         pending_session_n_ = 0;
+        gated_block_count_ = 0;
     }
 
     // Caps to limit damage from a malicious or buggy peer sending a
@@ -223,13 +251,52 @@ void BinlogWriter::handle_block(uint32_t port2, unsigned session_n,
                  (long long)MAX_FORWARD_JUMP_BYTES);
         return;
     }
-    if (other_sessions_bytes_ + prospective_size > MAX_PER_PORT2_BYTES) {
+    const off_t quota = port2_quota_bytes();
+    // Charge our own file by its allocated size, matching how every
+    // other file is counted: a legitimate forward jump can grow the
+    // sparse logical extent by up to MAX_FORWARD_JUMP_BYTES while
+    // allocating almost nothing, and charging the logical size would
+    // falsely trip the quota (and the cleanup pass, which sees only
+    // allocated bytes, would rightly refuse to free anything).
+    // Project growth as one filesystem block, not BLOCK_BYTES: a
+    // 200-byte write into an unallocated region allocates a whole
+    // block, and projecting less would let the total overshoot the
+    // quota by the difference.
+    off_t own_alloc = 0;
+    off_t write_growth = off_t(BLOCK_BYTES);
+    {
+        struct stat fst;
+        if (fstat(fileno(fp), &fst) == 0) {
+            own_alloc = off_t(fst.st_blocks) * 512;
+            if (off_t(fst.st_blksize) > write_growth) {
+                write_growth = off_t(fst.st_blksize);
+            }
+        }
+    }
+    const off_t projected = other_sessions_bytes_ + own_alloc + write_growth;
+    if (projected > quota) {
+        // Try to free space now rather than dropping every block until
+        // the hourly cleanup pass: age out the oldest sessions for this
+        // port2 and re-baseline. Rate-limited so a dir that genuinely
+        // can't get under quota isn't rescanned per block.
+        double cnow = time_seconds();
+        if (cnow - last_quota_cleanup_s_ >= QUOTA_CLEANUP_MIN_INTERVAL_S) {
+            last_quota_cleanup_s_ = cnow;
+            // the pass counts our own on-disk allocation itself; the
+            // extra headroom we need beyond that is this write
+            log_cleanup_port2_quota(port2_, base_dir_.c_str(),
+                                    write_growth);
+            refresh_other_sessions_bytes();
+        }
+    }
+    if (other_sessions_bytes_ + own_alloc + write_growth > quota) {
         ::printf("binlog: dropping seqno=%u (port2=%u total would be "
                  "%lld > %lld byte quota; cleanup pass will age out "
                  "old sessions)\n",
                  unsigned(blk.seqno), unsigned(port2_),
-                 (long long)(other_sessions_bytes_ + prospective_size),
-                 (long long)MAX_PER_PORT2_BYTES);
+                 (long long)(other_sessions_bytes_ + own_alloc
+                             + write_growth),
+                 (long long)quota);
         return;
     }
 
@@ -363,6 +430,23 @@ void BinlogWriter::tick(MAVLink &user_link)
 {
     double now_s = time_seconds();
 
+    // A vehicle streaming mid-log with no file open (fresh child
+    // attached mid-flight, or post-rotation) can't make progress until
+    // it restarts from seqno 0; left alone, only its 10 s no-ACK
+    // client timeout gets it there. Send STOP so it stops now — the
+    // START logic below then restarts it from 0 within a second or so
+    // (ArduPilot ignores STARTs while streaming, but honours STOP).
+    // The threshold keeps a lone stale block from stopping a healthy
+    // stream that is about to deliver its seqno 0.
+    if (fp == nullptr && gated_block_count_ >= STOP_MIN_GATED_BLOCKS
+        && now_s - last_stop_sent_s >= STOP_REPEAT_S) {
+        if (send_stop_packet(user_link)) {
+            last_stop_sent_s = now_s;
+            // make the follow-up START prompt in both START loops
+            last_start_sent_s = 0.0;
+        }
+    }
+
     if (!any_block_seen) {
         // Vehicle hasn't begun streaming yet. Send the magic
         // REMOTE_LOG_BLOCK_STATUS(status=ACK,
@@ -436,7 +520,7 @@ void BinlogWriter::tick(MAVLink &user_link)
     }
 }
 
-bool BinlogWriter::send_start_packet(MAVLink &user_link)
+bool BinlogWriter::send_magic_packet(MAVLink &user_link, uint32_t magic_seqno)
 {
     // See the long comment in send_status() about why we can't use
     // user_link.send_message() — the pack_chan call finalises the
@@ -448,7 +532,7 @@ bool BinlogWriter::send_start_packet(MAVLink &user_link)
         PROXY_SYSID, PROXY_COMPID, CHAN_COMM1, &msg,
         /*target_system*/ target_system,
         /*target_component*/ target_component,
-        /*seqno*/ MAV_REMOTE_LOG_DATA_BLOCK_START,
+        /*seqno*/ magic_seqno,
         /*status*/ MAV_REMOTE_LOG_DATA_BLOCK_ACK);
     uint8_t buf[MAVLINK_MAX_PACKET_LEN];
     uint16_t len = mavlink_msg_to_send_buffer(buf, &msg);
@@ -458,6 +542,16 @@ bool BinlogWriter::send_start_packet(MAVLink &user_link)
     return user_link.send_buf(buf, len) == ssize_t(len);
 }
 
+bool BinlogWriter::send_start_packet(MAVLink &user_link)
+{
+    return send_magic_packet(user_link, MAV_REMOTE_LOG_DATA_BLOCK_START);
+}
+
+bool BinlogWriter::send_stop_packet(MAVLink &user_link)
+{
+    return send_magic_packet(user_link, MAV_REMOTE_LOG_DATA_BLOCK_STOP);
+}
+
 void BinlogWriter::observe(const mavlink_message_t &msg)
 {
     if (msg.msgid != MAVLINK_MSG_ID_SYSTEM_TIME) {
@@ -498,6 +592,10 @@ bool BinlogWriter::rotate_for_reboot()
     // (whose _sending_to_client is now false post-reboot) resumes
     // streaming without waiting out the keep-alive interval.
     last_start_sent_s = 0.0;
+    // Stale pre-rotation blocks must re-accumulate before a STOP nudge
+    // fires: the restarted stream's seqno 0 is usually already on the
+    // way and must not be interrupted by a lone straggler.
+    gated_block_count_ = 0;
     // Re-arm the first-message-seen guard so the new boot's first
     // SYSTEM_TIME becomes the new watermark, not a spurious second
     // trigger of the backward-jump check.

binlog.h

@@ -147,6 +147,19 @@ class BinlogWriter {
     // How often to re-send START until the vehicle starts streaming.
     // ArduPilot's client-timeout is 10 s so 1 Hz is comfortable.
     static constexpr double   START_REPEAT_S     = 1.0;
+    // Throttle for the STOP nudge sent while a vehicle streams mid-log
+    // blocks at us with no file open (see gated_block_count_).
+    static constexpr double   STOP_REPEAT_S      = 2.0;
+    // Gated blocks required before the first STOP: one or two stale
+    // packets (e.g. delayed pre-reboot blocks right after a rotation)
+    // must not stop a healthy stream that is about to deliver its
+    // seqno 0; a genuinely mid-log stream reaches this in well under
+    // a second. Residual: ArduPilot has a single remote-log stream
+    // with no ownership check, so if another collector (e.g. a local
+    // MAVProxy dataflash_logger) is also attached to the vehicle, our
+    // STOP restarts its stream too — inherent to the protocol, and
+    // two collectors on one vehicle fight over ACKs regardless.
+    static constexpr unsigned STOP_MIN_GATED_BLOCKS = 3;
     // Keep-alive START cadence after streaming has begun: defence in
     // depth so a post-reboot vehicle (whose _sending_to_client got
     // cleared) resumes streaming within ~5 s of the next keepalive.
@@ -158,6 +171,11 @@ class BinlogWriter {
     // wobble and well below the multi-second pause a real reboot
     // creates.
     static constexpr uint32_t REBOOT_TIME_BACKWARD_MS = 10000;
+    // A block-0 arriving while the file is open with highest_seen at
+    // least this far along is a vehicle-side log restart, not a
+    // retransmission: AP_Logger_MAVLink's resend window is ~32 blocks,
+    // so a legitimate block-0 retry can never arrive this late.
+    static constexpr uint32_t SEQNO0_RESTART_MIN_HIGHEST = 200;
     // Caps to limit the damage from an attacker (or a buggy vehicle)
     // sending a giant seqno on the unsigned-by-default user-side port.
     // A bare seqno=0 followed by seqno=2^32-1 would otherwise sparse-
@@ -170,10 +188,11 @@ class BinlogWriter {
     //    in a single seqno step. Covers ~30 minutes of streaming at
     //    400 blocks/s; anything bigger is unambiguously bogus.
     static constexpr off_t MAX_FORWARD_JUMP_BYTES = off_t(100) * 1024 * 1024;
-    // 2. Per-port-pair on-disk quota: total size of all .tlog + .bin
-    //    files under logs/<port2>/ may not exceed 1 GiB. The hourly
-    //    cleanup loop also enforces this by deleting oldest files.
-    static constexpr off_t MAX_PER_PORT2_BYTES   = off_t(1024) * 1024 * 1024;
+    // 2. Per-port-pair on-disk quota: total allocated size of all
+    //    .tlog + .bin files under logs/<port2>/ may not exceed
+    //    port2_quota_bytes() (cleanup.h; default 1 GiB, env-
+    //    overridable). The hourly cleanup loop enforces the same
+    //    quota by deleting oldest files.
 
     FILE *fp = nullptr;
 
@@ -212,9 +231,20 @@ class BinlogWriter {
                          double now_s);
     void drop_stale_nack_state(double now_s);
     bool send_status(MAVLink &user_link, uint32_t seqno, uint8_t status);
-    // The magic START packet emit, factored out so both the pre-stream
-    // 1 Hz loop and the streaming-mode keepalive call it.
+    // The magic START/STOP packet emit, shared by the pre-stream 1 Hz
+    // loop, the streaming-mode keepalive and the mid-log STOP nudge.
+    bool send_magic_packet(MAVLink &user_link, uint32_t magic_seqno);
     bool send_start_packet(MAVLink &user_link);
+    bool send_stop_packet(MAVLink &user_link);
+
+    // Count of DATA_BLOCKs rejected by the strict-start gate (no file
+    // open, seqno != 0) since the last open/rotation: the vehicle is
+    // streaming mid-log and can only recover by restarting from
+    // seqno 0. Once it passes STOP_MIN_GATED_BLOCKS, tick() sends
+    // STOP so that happens now instead of after the vehicle's 10 s
+    // no-ACK client timeout. Saturating.
+    unsigned gated_block_count_ = 0;
+    double last_stop_sent_s = 0.0;
 
     // Captured on the first successful open() so rotate_for_reboot()
     // can re-scan the per-day dir for a fresh session N without
@@ -233,6 +263,12 @@ class BinlogWriter {
     off_t other_sessions_bytes_ = 0;
     unsigned writes_since_quota_refresh_ = 0;
     void refresh_other_sessions_bytes();
+    // Rate limit for the write-time quota-breach cleanup: when the
+    // gate trips we run the per-port2 quota pass immediately (instead
+    // of dropping blocks until the hourly pass), but at most once per
+    // this interval so a genuinely full dir isn't rescanned per block.
+    static constexpr double QUOTA_CLEANUP_MIN_INTERVAL_S = 30.0;
+    double last_quota_cleanup_s_ = 0.0;
 
     // Per-entry MAVLink sysid filter for SYSTEM_TIME-based reboot
     // detection. 0 = match any (default). Set from KeyEntry.fc_sysid