Fix security and correctness issues from code review
ober
58e595988acf41e66c5409a77783f961464ba1b4
--- a/lib/chez/fuse.sls +++ b/lib/chez/fuse.sls @@ -86,15 +86,23 @@ [(and (< n 0) (= (fuse-get-errno) EINTR)) (loop)] [else n])))) - ;; Write the full bytevector. Retry on EINTR. + ;; Write the full bytevector. Retry on EINTR, advance on partial writes. + ;; We need a temporary buffer for offset writes since c-write always starts + ;; from byte 0 of the bytevector. For partial writes we copy the remainder. (define (fuse-write fd bv) (let ([len (bytevector-length bv)]) - (let loop ([written 0]) + (let loop ([written 0] [buf bv] [remaining len]) (if (>= written len) written - (let ([n (c-write fd bv len)]) + (let ([n (c-write fd buf remaining)]) (cond - [(> n 0) (loop (+ written n))] - [(and (< n 0) (= (fuse-get-errno) EINTR)) (loop written)] + [(= n remaining) (+ written n)] ;; done + [(> n 0) + ;; Partial write — make a new bytevector for the remainder + (let* ([left (- remaining n)] + [rest (make-bytevector left)]) + (bytevector-copy! buf n rest 0 left) + (loop (+ written n) rest left))] + [(and (< n 0) (= (fuse-get-errno) EINTR)) (loop written buf remaining)] [else written])))))) ;; ====================================================================== @@ -155,10 +163,9 @@ (fuse-close-device (fuse-session-fd session)) (fuse-session-fd-set! session -1)) ;; Wait for background thread if any - (let ([t (fuse-session-thread session)]) - (when t - (guard (exn [else (void)]) - (scheme-thread-join t)))) + (when (fuse-session-thread session) + (guard (exn [else (void)]) + (scheme-thread-join-session session))) ;; Unmount (when (fuse-session-mounted? session) (guard (exn [else (void)]) @@ -167,48 +174,38 @@ ;; Wait for a background session to finish (blocks until loop exits). (define (fuse-session-wait session) - (let ([t (fuse-session-thread session)]) - (when t - (scheme-thread-join t)))) + (when (fuse-session-thread session) + (scheme-thread-join-session session))) ;; ====================================================================== ;; Internal: session creation and teardown ;; ====================================================================== - ;; Portable thread join that works for Chez's thread handles. - (define (scheme-thread-join t) - ;; Chez doesn't have thread-join; busy-wait with yield. - ;; The thread sets running? to #f when done. - ;; We could use a condition variable, but keep it simple. - (let loop () - (when (not (thread-dead? t)) - (thread-yield) - (loop)))) - - ;; Check if a thread is dead (Chez-specific). - (define (thread-dead? t) - ;; In Chez, there's no direct "is this thread dead?" API. - ;; We use a mutex + condition variable approach instead. - ;; For now, rely on session running? flag. - #t) - - (define (thread-yield) - ;; Yield to other threads - (sleep (make-time 'time-duration 1000000 0))) ;; 1ms + ;; Wait for a session's background thread to finish using its condition variable. + (define (scheme-thread-join-session session) + (let ([mtx (fuse-session-mutex session)] + [cv (fuse-session-done session)]) + (when cv + (with-mutex mtx + (let loop () + (when (fuse-session-running? session) + (condition-wait cv mtx) + (loop))))))) (define (create-session ops mountpoint options) (let* ([fsname (get-option options 'fsname "chez-fuse")] [debug? (get-option options 'debug #f)] [allow-other? (get-option options 'allow-other #t)] [fd (fuse-open-device)] + [mtx (make-mutex)] [session (make-fuse-session fd mountpoint #f #f FUSE-KERNEL-VERSION FUSE-KERNEL-MINOR-VERSION 131072 131072 ;; max-write, max-readahead ops - (make-mutex) ;; dispatch mutex + mtx ;; dispatch mutex #f ;; thread handle - #f)] ;; done condition + (make-condition))] ;; done condition [uid (getuid)] [gid (getgid)]) (fuse-mount! fd mountpoint fsname uid gid allow-other?) @@ -228,7 +225,11 @@ ;; Then explicitly unmount (belt and suspenders) (guard (exn [else (void)]) (fuse-unmount! mountpoint)) - (fuse-session-mounted?-set! session #f)))) + (fuse-session-mounted?-set! session #f)) + ;; Signal waiters that the session is done + (fuse-session-running?-set! session #f) + (let ([cv (fuse-session-done session)]) + (when cv (condition-broadcast cv))))) ;; Install SIGINT handler to cleanly stop the session. (define (install-interrupt-handler session) --- a/lib/chez/fuse/constants.sls +++ b/lib/chez/fuse/constants.sls @@ -70,7 +70,7 @@ ;; Helpers fuse-rec-align) - (import (rnrs)) + (import (rnrs) (only (chezscheme) machine-type)) ;; ---- Protocol version ---- (define FUSE-KERNEL-VERSION 7) @@ -213,14 +213,22 @@ (define DT-SOCK 12) (define DT-WHT 14) + ;; ---- Platform detection (used for divergent constants below) ---- + (define freebsd? + (case (machine-type) + [(a6fb ta6fb i3fb ti3fb arm64fb tarm64fb) #t] + [else #f])) + ;; ---- Open flags ---- + ;; O_RDONLY, O_WRONLY, O_RDWR are the same everywhere. + ;; O_CREAT, O_EXCL, O_TRUNC, O_APPEND differ between Linux and FreeBSD. (define O-RDONLY #x0000) (define O-WRONLY #x0001) (define O-RDWR #x0002) - (define O-CREAT #x0040) - (define O-EXCL #x0080) - (define O-TRUNC #x0200) - (define O-APPEND #x0400) + (define O-CREAT (if freebsd? #x0200 #x0040)) + (define O-EXCL (if freebsd? #x0800 #x0080)) + (define O-TRUNC (if freebsd? #x0400 #x0200)) + (define O-APPEND (if freebsd? #x0008 #x0400)) ;; ---- Access flags ---- (define F-OK 0) @@ -229,7 +237,8 @@ (define X-OK 1) ;; ---- Errno values ---- - ;; POSIX values — same on Linux and FreeBSD for the common set + ;; Values 1-34 are the same on Linux and FreeBSD. + ;; Higher values diverge — use platform detection. (define EPERM 1) (define ENOENT 2) (define ESRCH 3) @@ -262,15 +271,17 @@ (define EPIPE 32) (define EDOM 33) (define ERANGE 34) - (define ENOSYS 38) - (define ENOTEMPTY 39) - (define ENAMETOOLONG 36) - (define ELOOP 40) - (define ENODATA 61) - (define ENOTCONN 107) - (define EOVERFLOW 75) - (define EOPNOTSUPP 95) - (define ENOTSUP 95) + + ;; Platform-divergent errno values + (define ENAMETOOLONG (if freebsd? 63 36)) + (define ENOSYS (if freebsd? 78 38)) + (define ENOTEMPTY (if freebsd? 66 39)) + (define ELOOP (if freebsd? 62 40)) + (define ENODATA (if freebsd? 89 61)) ;; FreeBSD: ENOATTR=89 + (define ENOTCONN (if freebsd? 57 107)) + (define EOVERFLOW (if freebsd? 84 75)) + (define EOPNOTSUPP (if freebsd? 45 95)) + (define ENOTSUP EOPNOTSUPP) ;; ---- Helpers ---- ;; 8-byte alignment for FUSE records --- a/lib/chez/vault.sls +++ b/lib/chez/vault.sls @@ -563,6 +563,7 @@ (let ([sb-num-bv (vault-decrypt-small pk sb-enc)]) (unless sb-num-bv (bytevector-fill! pk 0) + (bytevector-fill! master-key 0) (error 'vault-open "corrupt vault (superblock locator)")) ;; Set master key in blockstore (blockstore-set-key! bs master-key) --- a/src/mount_helper.c +++ b/src/mount_helper.c @@ -18,6 +18,7 @@ #include <string.h> #include <unistd.h> #include <signal.h> +#include <sys/wait.h> #if defined(FREEBSD) #include <sys/param.h> @@ -156,9 +157,18 @@ int chez_fuse_mount(int fd, const char *mountpoint, const char *fsname, } int chez_fuse_unmount(const char *mountpoint) { - char cmd[512]; - snprintf(cmd, sizeof(cmd), "umount '%s' 2>/dev/null", mountpoint); - return system(cmd); + pid_t pid = fork(); + if (pid < 0) return -1; + if (pid == 0) { + /* Child: exec umount directly — no shell, no injection */ + int devnull = open("/dev/null", O_WRONLY); + if (devnull >= 0) { dup2(devnull, STDERR_FILENO); close(devnull); } + execl("/sbin/umount", "umount", mountpoint, (char *)NULL); + _exit(127); + } + int status; + waitpid(pid, &status, 0); + return WIFEXITED(status) ? WEXITSTATUS(status) : -1; } int chez_fuse_unmount_lazy(const char *mountpoint) {