Re: Why do the compiler checks .go directory is writeable?

[Ludovic Courtès]

Hi,

Sorry for the delay!

address@hidden skribis:

> I'm refering to the ensure-writable-dir function.
> I've stumbled upon this problem recently : I have a program that's suid root
> but calls guile. The guile compiler then creates some directories in
> .cache/guile but then check (with access()) that he can write in there, which
> he can't since access revoke the effective uid for the caller uid. We thus 
> have
> this situation: the compiler creates a bunch of directories then complains he
> cannot write in them.
>
> It's not obvious to me why the compiler should ensure a directory is writable
> just to throw an error. Wouldn't it be better to just call opens and writes
> and let these fails and report these more acurate errors instead (or, in my
> case, let them succeed) ?

Agreed, not to mention time-of-check-to-time-of-use-errors.

We basically want ‘mkdir -p’, but we can even omit the stat(2) call upon
EEXIST, because if DIR doesn’t point to a directory, the error will be
caught soon after anyway.

I’ll install the following patch if there’s no objection:

diff --git a/module/system/base/compile.scm b/module/system/base/compile.scm
index 0bc11a3..afcb55a 100644
--- a/module/system/base/compile.scm
+++ b/module/system/base/compile.scm
@@ -1,6 +1,6 @@
 ;;; High-level compiler interface
 
-;; Copyright (C) 2001, 2009, 2010, 2011 Free Software Foundation, Inc.
+;; Copyright (C) 2001, 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
 
 ;;; This library is free software; you can redistribute it and/or
 ;;; modify it under the terms of the GNU Lesser General Public
@@ -72,7 +72,7 @@
 ;; before the check, so that we avoid races (possibly due to parallel
 ;; compilation).
 ;;
-(define (ensure-writable-dir dir)
+(define (ensure-directory dir)
   (catch 'system-error
     (lambda ()
       (mkdir dir))
@@ -80,13 +80,12 @@
       (let ((errno (and (pair? rest) (car rest))))
         (cond
          ((eqv? errno EEXIST)
-          (let ((st (stat dir)))
-            (if (or (not (eq? (stat:type st) 'directory))
-                    (not (access? dir W_OK)))
-                (error "directory not writable" dir))))
+          ;; Assume it's a writable directory, to avoid TOCTOU errors,
+          ;; as well as UID/EUID mismatches that occur with access(2).
+          #t)
          ((eqv? errno ENOENT)
-          (ensure-writable-dir (dirname dir))
-          (ensure-writable-dir dir))
+          (ensure-directory (dirname dir))
+          (ensure-directory dir))
          (else
           (throw k subr fmt args rest)))))))
 
@@ -125,7 +124,7 @@
                  %compile-fallback-path
                  (canonical->suffix (canonicalize-path file))
                  (compiled-extension))))
-         (and (false-if-exception (ensure-writable-dir (dirname f)))
+         (and (false-if-exception (ensure-directory (dirname f)))
               f))))
 
 (define* (compile-file file #:key
@@ -144,7 +143,7 @@
       ;; Choose the input encoding deterministically.
       (set-port-encoding! in (or enc "UTF-8"))
 
-      (ensure-writable-dir (dirname comp))
+      (ensure-directory (dirname comp))
       (call-with-output-file/atomic comp
         (lambda (port)
           ((language-printer (ensure-language to))

Thanks,
Ludo’.