bug#35996: User account password got locked when booting old generation

["Ludovic Courtès" <ludo@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]

Hi,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> I got a locked /etc/shadow again now despite Ludovic’s patches (which
> would nonetheless give me a better feeling when pushed).

Will do.  :-)

> When booting an unbootable generation with Ludovic’s patches and then
> rebooting a normal generation with Ludovic’s patches, /etc/shadow is
> locked.

So with this scenario, the problem is 100% reproducible, right?

> Note that I get a message like “/dev/mapper/Guix: recovering journal”
> when booting (I did not pay attention to that before).  I shut down
> the unbootable generation with Ctrl+Alt+Del.  When I normally shut
> down with Ctrl+Alt+Del I get no such message.

Indeed, ‘shepherd’ calls ‘disable-reboot-on-ctrl-alt-del’ (which
disables “hard” reboots upon ctrl-alt-del and instead notifies it) after
it has loaded its config file.

In your case, loading the config file never completes because the
‘start’ method is called from the config file for every service, and one
of them, udev, never starts.  Thus, when you press Ctrl-Alt-Del, you
perform a hard reboot.

The hard reboot happens after Guix has written to /etc/shadow.  One
possibility is that changes to this file haven’t been flushed to disk.
Thus, on the next boot, we start off with an empty or truncated
/etc/shadow, leading the activation snippet to initialize passwords.


If that theory holds, the patch below (on top of the others) should
help.  Could you give it a try?

Actually, the fact that ‘rename-file’ was called *before* ‘close-port’
could be problematic in itself; so perhaps, even without the ‘fdatasync’
call, we’d get better results…  especially since ‘fdatasync’ won’t be
available in the initrd anyway, hmm…

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1525 bytes --]

diff --git a/gnu/build/accounts.scm b/gnu/build/accounts.scm
index 8687446aa6..c13e6f2e89 100644
--- a/gnu/build/accounts.scm
+++ b/gnu/build/accounts.scm
@@ -19,6 +19,7 @@
 (define-module (gnu build accounts)
   #:use-module (guix records)
   #:use-module (guix combinators)
+  #:use-module ((guix build syscalls) #:select (fdatasync))
   #:use-module (gnu system accounts)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
@@ -230,6 +231,14 @@ each field."
   ;; grab this lock with 'with-file-lock' when they access the databases.
   "/etc/.pwd.lock")
 
+(define-syntax-rule (catch-ENOSYS exp)
+  (catch 'system-error
+    (lambda () exp)
+    (lambda args
+      (if (= ENOSYS (system-error-errno args))
+          #f
+          (apply throw args)))))
+
 (define (database-writer file mode entry->string)
   (lambda* (entries #:optional (file-or-port file))
     "Write ENTRIES to FILE-OR-PORT.  When FILE-OR-PORT is a file name, write
@@ -249,9 +258,12 @@ to it atomically and set the appropriate permissions."
             (lambda ()
               (chmod port mode)
               (write-entries port)
-              (rename-file template file-or-port))
-            (lambda ()
+              (catch-ENOSYS (fdatasync port))
               (close-port port)
+              (rename-file template file-or-port))
+            (lambda ()
+              (unless (port-closed? port)
+                (close-port port))
               (when (file-exists? template)
                 (delete-file template))))))))