Fix static analysis bugs

Author: mancausoft

BUGS.md

@@ -0,0 +1,110 @@
+# BUGS
+
+Static review as of 2026-05-05.  The defects found in this pass have
+been fixed in the working tree; this file records what was found and how
+it was addressed.
+
+Checks used during the review:
+
+- `make objs`
+- `make lib`
+- `cc --analyze` on the FUSE-independent wrapper sources
+- `git diff --check`
+
+`make bin` was not run locally because this workstation does not expose
+FUSE headers through pkg-config.
+
+## Fixed In This Pass
+
+### High: `direct_dirid()` over-copied the `.DIR;1` suffix
+
+Location: `ods2lib/direct.c`
+
+`direct_dirid()` used the total descriptor length (`len + 6`) as the
+copy size for the literal `.DIR;1`, reading past the string literal and
+potentially writing past `nambuf`.
+
+Fix: copy only the suffix length and validate `len + suffix_len` before
+writing into `nambuf`.
+
+### High: long directory specs could abort the FUSE process
+
+Location: `ods2lib/direct.c`, `src/lookup.c`
+
+User-controlled FUSE paths could produce a VMS directory string longer
+than `direct_dirid()` accepted, and that path called `abort()`.
+
+Fix: return `SS$_BADFILENAME` instead of aborting, and make POSIX-to-VMS
+directory conversion fail on truncation instead of silently shortening
+the path.
+
+### High: default multithreaded FUSE mode raced global caches
+
+Location: `src/fuse_ods2.c`
+
+The code recommended `-s`, but did not require it.  ods2lib caches and
+the textmode decode cache are global mutable state without locking.
+
+Fix: inject `-s` into the libfuse argument vector so the filesystem runs
+single-threaded until the cache layers are made thread-safe.
+
+### Medium: long legal names could be truncated in `readdir()`
+
+Location: `src/ops.c`
+
+The display buffer was 80 bytes, but a legal versioned ODS-2 name can be
+`40 + "." + 40 + ";" + 5` characters.
+
+Fix: use a buffer sized for the real exposed maximum and report the same
+limit through `statfs()`.
+
+### Medium: `readdir()` was not resumable
+
+Location: `src/ops.c`
+
+`readdir()` ignored the incoming offset and passed offset `0` for every
+entry, which can break large directories when libfuse needs multiple
+calls to drain the listing.
+
+Fix: add stable monotonically increasing offsets for `.`, `..`, and
+directory entries; honor the incoming offset; stop cleanly when
+`filler()` reports a full buffer.
+
+### Medium: corrupt directory records looked like successful short listings
+
+Location: `src/lookup.c`
+
+Directory parser validation failures broke out of the current block but
+still returned `SS$_NORMAL`.
+
+Fix: return `SS$_BADIRECTORY` when record size, record extent, or name
+extent checks fail.
+
+### Medium: malformed HEAD timestamps could read outside the header block
+
+Location: `src/lookup.c`
+
+`fh2$b_idoffset` was checked only against the lower fixed-header bound.
+A malformed offset near the end of the 512-byte HEAD could make timestamp
+reads go out of bounds.
+
+Fix: validate `idoffset * 2 + sizeof(struct IDENT) <= sizeof(struct HEAD)`
+before reading the IDENT area.
+
+### Low: `/000000` did not map to the root directory
+
+Location: `src/lookup.c`
+
+The comment promised `/000000` behaved like `/`, but only `/` was special
+cased.
+
+Fix: treat both `/` and `/000000` as the MFD.
+
+### Low: smoke test raced first unmount against the second mount
+
+Location: `test/smoke.sh`
+
+The first foreground FUSE process was unmounted but not waited for before
+starting the second mount.
+
+Fix: wait for the first background FUSE process after `fusermount3 -u`.

ods2lib/direct.c

@@ -1116,8 +1116,8 @@ vmscond_t direct_dirid( struct VCB *vcb, struct dsc$descriptor *dirdsc,
 
     dirlen = dirdsc->dsc$w_length;
 
-    if( (size_t)dirlen > sizeof( nambuf ) -1 )
-        abort();
+    if( (size_t)dirlen > sizeof( nambuf ) -1 )
+        return SS$_BADFILENAME;
     dirnam = dirdsc->dsc$a_pointer;
 
     srcdir.fid$w_num = FID$C_MFD;
@@ -1156,9 +1156,12 @@ vmscond_t direct_dirid( struct VCB *vcb, struct dsc$descriptor *dirdsc,
         if( len == 0 )
             break;
 
-        memcpy( nambuf, bp, len );
-        namdsc.dsc$w_length = (uint16_t)(len + sizeof( ".DIR;1" ) -1);
-        memcpy( nambuf+len, ".DIR;1", namdsc.dsc$w_length );
+        const size_t suffix_len = sizeof( ".DIR;1" ) - 1;
+        if( len + suffix_len > sizeof( nambuf ) )
+            return SS$_BADFILENAME;
+        memcpy( nambuf, bp, len );
+        namdsc.dsc$w_length = (uint16_t)(len + suffix_len);
+        memcpy( nambuf + len, ".DIR;1", suffix_len );
 
         namdsc.dsc$a_pointer = nambuf;
 

src/fuse_ods2.c

@@ -83,7 +83,7 @@ static void usage( const char *prog ) {
         "  -o uid=N           force a specific uid on every file\n"
         "  -o gid=N           force a specific gid on every file\n"
         "  -o debug           verbose diagnostics on stderr\n"
-        "  -s                 single-threaded (recommended)\n"
+        "  -s                 single-threaded (always enabled)\n"
         "  -f                 foreground (do not daemonize)\n"
         "  -d                 same as -odebug + -f\n"
         "\n"
@@ -294,6 +294,10 @@ int main( int argc, char *argv[] ) {
         fuse_opt_free_args( &args );
         return 0;
     }
+    if( fuse_opt_add_arg( &args, "-s" ) == -1 ) {
+        fuse_opt_free_args( &args );
+        return 1;
+    }
 
     /* Read-only is enforced inside every write op (-EROFS).  We do
      * NOT inject -oro / -odefault_permissions into the libfuse arg

src/lookup.c

@@ -152,9 +152,10 @@ void ods2_head_to_stat( const struct HEAD *head, struct stat *st ) {
         st->st_gid = (gid_t)F11WORD( head->fh2$l_fileowner.uic$w_grp );
 
     /* Times.  IDENT area lives at fh2$b_idoffset (units of words). */
-    if( head->fh2$b_idoffset >= FH2$C_LENGTH / 2 ) {
+    size_t idoff = (size_t)head->fh2$b_idoffset * sizeof(f11word);
+    if( idoff >= FH2$C_LENGTH && idoff + sizeof(struct IDENT) <= sizeof *head ) {
         const struct IDENT *id = (const struct IDENT *)
-            ((const uint16_t *)head + head->fh2$b_idoffset);
+            ((const char *)head + idoff);
         st->st_mtime = ods2_vmstime_to_time_t( id->fi2$q_revdate );
         st->st_ctime = ods2_vmstime_to_time_t( id->fi2$q_credate );
         st->st_atime = st->st_mtime;
@@ -183,20 +184,24 @@ static char *split_last( char *path ) {
  * argument expected by direct_dirid().  Empty input -> empty output
  * (which direct_dirid() treats as MFD).
  */
-static void posix_dir_to_vms( const char *posix, char *out, size_t outlen ) {
+static int posix_dir_to_vms( const char *posix, char *out, size_t outlen ) {
     size_t o = 0;
     int first = 1;
-    if( outlen == 0 ) return;
+    if( outlen == 0 ) return 0;
     for( const char *p = posix; *p && o + 1 < outlen; ) {
         while( *p == '/' ) ++p;
         if( !*p ) break;
         if( !first && o + 1 < outlen )
             out[o++] = '.';
         first = 0;
-        while( *p && *p != '/' && o + 1 < outlen )
+        while( *p && *p != '/' ) {
+            if( o + 1 >= outlen )
+                return 0;
             out[o++] = (char)toupper( (unsigned char)*p++ );
+        }
     }
     out[o] = '\0';
+    return 1;
 }
 
 /* Parse "FILE.EXT" or "FILE.EXT;n" into (name, version).  When no
@@ -264,6 +269,7 @@ vmscond_t ods2_iterate_dir( struct FCB *dir_fcb, int latest_only,
     char last_name[128];
     int  last_name_len = -1;
     int  stop = 0;
+    vmscond_t retsts = SS$_NORMAL;
 
     for( uint32_t blk = 1; blk <= efblk && !stop; ++blk ) {
         struct VIOC *vioc = NULL;
@@ -299,23 +305,30 @@ vmscond_t ods2_iterate_dir( struct FCB *dir_fcb, int latest_only,
                     fprintf( stderr,
                              "fuse-ods2: iterate_dir:   record size=%u too big at offs=%ld\n",
                              (unsigned)size, (long)(p - buf) );
+                retsts = SS$_BADIRECTORY;
+                stop = 1;
                 break;
             }
             if( p + size + 2 > end ) {
                 if( ods2_rt.debug )
                     fprintf( stderr,
                              "fuse-ods2: iterate_dir:   record overruns block at offs=%ld size=%u\n",
                              (long)(p - buf), (unsigned)size );
+                retsts = SS$_BADIRECTORY;
+                stop = 1;
                 break;
             }
 
             uint8_t namecount = dr->dir$b_namecount;
             char   *namebase  = dr->dir$t_name;
-            if( namebase + namecount > end ) {
+            char   *rec_end   = (char *)dr + size + 2;
+            if( namebase + namecount > rec_end ) {
                 if( ods2_rt.debug )
                     fprintf( stderr,
                              "fuse-ods2: iterate_dir:   name overruns block (count=%u)\n",
                              (unsigned)namecount );
+                retsts = SS$_BADIRECTORY;
+                stop = 1;
                 break;
             }
             if( ods2_rt.debug )
@@ -332,7 +345,6 @@ vmscond_t ods2_iterate_dir( struct FCB *dir_fcb, int latest_only,
                                  && memcmp( last_name, namebase, namecount ) == 0 );
 
             char *eptr    = namebase + ((namecount + 1) & ~1);
-            char *rec_end = (char *)dr + size + 2;
             int   first   = 1;
             while( eptr + sizeof(struct dir$r_ent) <= rec_end ) {
                 struct dir$r_ent *de = (struct dir$r_ent *)eptr;
@@ -363,7 +375,7 @@ vmscond_t ods2_iterate_dir( struct FCB *dir_fcb, int latest_only,
         deaccesschunk( vioc, 0, 0, 1 );
     }
 
-    return SS$_NORMAL;
+    return retsts;
 }
 
 /* ------------------------------------------------------ path resolver */
@@ -412,7 +424,7 @@ vmscond_t ods2_path_to_fid( const char *path, struct fiddef *out_fid,
         return SS$_BADPARAM;
 
     /* Root and "/000000" both map to the MFD. */
-    if( strcmp( path, "/" ) == 0 ) {
+    if( strcmp( path, "/" ) == 0 || strcmp( path, "/000000" ) == 0 ) {
         out_fid->fid$w_num = FID$C_MFD;
         out_fid->fid$w_seq = FID$C_MFD;
         out_fid->fid$b_rvn = 0;
@@ -439,7 +451,10 @@ vmscond_t ods2_path_to_fid( const char *path, struct fiddef *out_fid,
 
     /* "work" now holds the dir prefix (with leading '/') or empty. */
     char vmsdir[512];
-    posix_dir_to_vms( work, vmsdir, sizeof vmsdir );
+    if( !posix_dir_to_vms( work, vmsdir, sizeof vmsdir ) ) {
+        free( work );
+        return SS$_BADFILENAME;
+    }
 
     /* Resolve directory part. */
     struct dsc_descriptor dirdsc;

src/ops.c

@@ -101,9 +101,13 @@ int ods2_getattr( const char *path, struct stat *st,
 
 /* ------------------------------------------------------ readdir */
 
+#define ODS2_POSIX_NAMEMAX (40 + 1 + 40 + 1 + 5)
+
 struct readdir_ctx {
     void                  *buf;
     fuse_fill_dir_t        filler;
+    off_t                  offset;
+    off_t                  next_off;
 };
 
 static int has_dir_suffix( const char *raw, int rawlen ) {
@@ -141,9 +145,13 @@ static void emit_name( const char *raw, int rawlen, int version, int is_dir,
 static int readdir_cb( const char *name, int namelen, int version,
                        const struct fiddef *fid, void *ctx ) {
     struct readdir_ctx *r = ctx;
-    char display[80];
+    char display[ODS2_POSIX_NAMEMAX + 1];
     struct stat st;
     int is_dir = 0;
+    off_t this_off = r->next_off++;
+
+    if( this_off <= r->offset )
+        return 0;
 
     struct VIOC *vioc = NULL;
     struct HEAD *head = NULL;
@@ -170,7 +178,7 @@ static int readdir_cb( const char *name, int namelen, int version,
      */
     st.st_mode = is_dir ? S_IFDIR : S_IFREG;
 
-    return r->filler( r->buf, display, &st, 0, 0 );
+    return r->filler( r->buf, display, &st, this_off, 0 );
 }
 
 int ods2_readdir( const char *path, void *buf, fuse_fill_dir_t filler,
@@ -231,10 +239,24 @@ int ods2_readdir( const char *path, void *buf, fuse_fill_dir_t filler,
         return -ENOTDIR;
     }
 
-    filler( buf, ".",  NULL, 0, 0 );
-    filler( buf, "..", NULL, 0, 0 );
+    off_t next_off = 1;
+    if( offset < next_off && filler( buf, ".", NULL, next_off, 0 ) != 0 ) {
+        deaccessfile( dfcb );
+        return 0;
+    }
+    ++next_off;
+    if( offset < next_off && filler( buf, "..", NULL, next_off, 0 ) != 0 ) {
+        deaccessfile( dfcb );
+        return 0;
+    }
+    ++next_off;
 
-    struct readdir_ctx ctx = { .buf = buf, .filler = filler };
+    struct readdir_ctx ctx = {
+        .buf = buf,
+        .filler = filler,
+        .offset = offset,
+        .next_off = next_off
+    };
 
     sts = ods2_iterate_dir( dfcb,
                             ods2_rt.allversions ? 0 : 1,
@@ -404,7 +426,7 @@ int ods2_statfs( const char *path, struct statvfs *st ) {
     st->f_bavail  = 0;
     st->f_files   = 0;
     st->f_ffree   = 0;
-    st->f_namemax = 80;         /* VMS file spec including version    */
+    st->f_namemax = ODS2_POSIX_NAMEMAX;
     return 0;
 }
 

test/smoke.sh

@@ -77,6 +77,7 @@ ok "write attempt is rejected as expected"
 
 # ---- versioned file (HELLO.TXT was copied 3 times -> versions 1..3) ----
 fusermount3 -u "$MNT"
+wait "$FPID" || true
 "$BIN" -s -o allversions "$IMG" "$MNT"
 for _ in 1 2 3 4 5; do
     if mountpoint -q "$MNT"; then break; fi