pcscd service activation causes boot failure

Done

Details

3 participants

Arun Isaac
Chris Marusich
Ludovic Courtès

Owner: unassigned

Submitted by: Chris Marusich

Severity: normal

Chris Marusich wrote on 20 Aug 2018 09:26

Recipients:(address . bug-guix@gnu.org)(name . Arun Isaac)(address . arunisaac@systemreboot.net)

Message-ID:87pnyd8pn5.fsf@gmail.com

Hi,

Commit de30205ba0f63eb987097a9f47b6e4fd38cd9044 added a pcscd service

(hooray!). However, its activation procedure doesn't always work. The

system test passes, and the activation procedure works the first time

you boot, but if you reboot, it will fail because the symlink it creates

already exists. The attached patch fixes the problem.

To reproduce the bug, use the attached operating system configuration

file (bare-bones.scm), and run:

guix system vm-image bare-bones.scm

This takes a long time to finish (up to a few hours, depending on your

system). Once it's done, copy the image out of the store:

cp $the_store_path /tmp/qemu-image

Then run it:

sudo qemu-system-x86_64 -smp cpus=1 -net user -net nic,model=virtio -enable-kvm -m 2048 /tmp/qemu-image

Observe how the VM boots successfully. Now log in as root (no password)

and reboot. Observe that the boot process fails now because the symlink

created by the pcscd activation service already exists.

The attached patch fixes the issue by using an idempotent procedure to

create the symlink. If there are no objections, I'll push the patch to

master about 24 hours from now. There is a fairly long comment in my

patch because although I wanted to re-use the switch-symlinks procedure

from (guix utils), I couldn't figure out how to do it. Ideas regarding

this are welcome! To be specific, I tried to use (guix utils), but then

I got the following error message at boot time (see my comment for more

information):

Toggle snippet (60 lines)loading '/gnu/store/f4ng1dlpm7q74vssbb049vpf2gvw3n3r-system/boot'...
[    2.175072] random: fast init done
making '/gnu/store/f4ng1dlpm7q74vssbb049vpf2gvw3n3r-system' the current system..
.
setting up setuid programs in '/run/setuid-programs'...
populating /etc from /gnu/store/n6cwz1hlmjylva2xrv61njl68g6c8k5l-etc...
usermod: no changes
usermod: no changes
usermod: no changes
ERROR: In procedure dynamic-func:
In procedure dynamic-pointer: Symbol not found: strverscmp

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
GNU Guile 2.2.3
Copyright (C) 1995-2017 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guix utils)> ,bt
In gnu/build/linux-boot.scm:
   530:13 22 (_)
In unknown file:
          21 (primitive-load "/gnu/store/f4ng1dlpm7q74vssbb049vpf2gv·")
In ice-9/eval.scm:
    619:8 20 (_ #f)
In unknown file:
          19 (primitive-load "/gnu/store/b6yg0pkp835a3zky9sj53yyjxac·")
In ice-9/boot-9.scm:
   260:13 18 (for-each #<procedure primitive-load (_)> _)
In unknown file:
          17 (primitive-load "/gnu/store/v42zaw2hjwxy5wnd0dwa6j245nr·")
In ice-9/eval.scm:
   721:20 16 (primitive-eval (begin (use-modules (guix build #) #) ·))
In ice-9/psyntax.scm:
  1235:36 15 (expand-top-sequence ((begin (use-modules (# # ·) ·) ·)) ·)
  1182:24 14 (parse _ (("placeholder" placeholder)) ((top) #(# # ·)) ·)
  1182:24 13 (parse _ (("placeholder" placeholder)) ((top) #(# # ·)) ·)
   285:10 12 (parse _ (("placeholder" placeholder)) (()) _ c&e (eval) ·)
In ice-9/boot-9.scm:
  3365:20 11 (process-use-modules _)
   222:29 10 (map1 (((guix build utils)) ((guix utils))))
   222:17  9 (map1 (((guix utils))))
  3366:31  8 (_ ((guix utils)))
  2788:17  7 (resolve-interface (guix utils) #:select _ #:hide _ # _ ·)
  2714:10  6 (_ (guix utils) _ _ #:ensure _)
  2982:16  5 (try-module-autoload _ _)
   2312:4  4 (save-module-excursion #<procedure 2de04e0 at ice-9/boo·>)
  3002:22  3 (_)
In unknown file:
           2 (primitive-load-path "guix/utils" #<procedure 2abb480 a·>)
In guix/utils.scm:
   485:24  1 (_)
In unknown file:
           0 (dynamic-func "strverscmp" #<dynamic-object #f>)
scheme@(guix utils)> [   53.048203] random: crng init done

To be clear, the above stack trace is NOT related to the bug I am
reporting.  It's just a different problem that made it difficult to
re-use switch-symlinks from (guix utils), which is why in my fix I
decided to just copy the switch-symlinks definition verbatim.

-- 
Chris

Attachment: bare-bones.scm

From 3fa1e930b827aebca2dbbfe84c36cf203f15afda Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Mon, 20 Aug 2018 00:16:06 -0700
Subject: [PATCH] gnu: services: Fix pcscd activation bug.

* gnu/services/security-token.scm (pcscd-activation): Idempotently create the
/var/lib/pcsc symlink so that it does not fail when it already exists.
---
 gnu/services/security-token.scm | 36 +++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

Toggle diff (56 lines)diff --git a/gnu/services/security-token.scm b/gnu/services/security-token.scm
index 7e7ea54a5..8bea49538 100644
--- a/gnu/services/security-token.scm
+++ b/gnu/services/security-token.scm
@@ -20,6 +20,7 @@
   #:use-module (gnu services)
   #:use-module (gnu services shepherd)
   #:use-module (gnu packages admin)
+  #:use-module (gnu packages base)
   #:use-module (gnu packages security-token)
   #:use-module (gnu system shadow)
   #:use-module (guix gexp)
@@ -62,14 +63,33 @@
 (define pcscd-activation
   (match-lambda
     (($ <pcscd-configuration> pcsc-lite usb-drivers)
-     #~(begin
-         (use-modules (guix build utils))
-         (mkdir-p "/var/lib")
-         (symlink #$(directory-union
-                     "pcsc"
-                     (map (cut file-append <> "/pcsc")
-                          usb-drivers))
-                  "/var/lib/pcsc")))))
+     (with-imported-modules (source-module-closure
+                             '((guix build utils)))
+       #~(begin
+           (use-modules (guix build utils))
+           ;; This switch-symlinks procedure was copied from (guix utils).  It
+           ;; would be nice to re-use the procedure from that module, but if
+           ;; we add that module to this gexp's imported modules and try to
+           ;; use it, then this activation gexp can fail when it runs.  To be
+           ;; specific, if you try to use (guix utils) and then build a VM
+           ;; with a pcscd-service-type using "guix system vm-image", then
+           ;; when you boot the VM, it will fail.  It fails because (guix
+           ;; utils) dynamically links glibc's strverscmp function when
+           ;; defining the version-compare procedure, and for some reason
+           ;; strverscmp can't be found.  Perhaps there's a way to fix or
+           ;; avoid this, but since we don't need the version-compare
+           ;; procedure here, anyway, it's simpler to just define our own
+           ;; switch-symlinks procedure instead.
+           (define (switch-symlinks link target)
+             (let ((pivot (string-append link ".new")))
+               (symlink target pivot)
+               (rename-file pivot link)))
+           (mkdir-p "/var/lib")
+           (switch-symlinks "/var/lib/pcsc"
+                            #$(directory-union
+                               "pcsc"
+                               (map (cut file-append <> "/pcsc")
+                                    usb-drivers))))))))
 
 (define pcscd-service-type
   (service-type
-- 
2.18.0

Ludovic Courtès wrote on 20 Aug 2018 18:13

Recipients:(name . Chris Marusich)(address . cmmarusich@gmail.com)(address . 32478@debbugs.gnu.org)

Message-ID:87bm9xdniy.fsf@gnu.org

Hello,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (21 lines)> The attached patch fixes the issue by using an idempotent procedure to
> create the symlink.  If there are no objections, I'll push the patch to
> master about 24 hours from now.  There is a fairly long comment in my
> patch because although I wanted to re-use the switch-symlinks procedure
> from (guix utils), I couldn't figure out how to do it.  Ideas regarding
> this are welcome!  To be specific, I tried to use (guix utils), but then
> I got the following error message at boot time (see my comment for more
> information):
>
> loading '/gnu/store/f4ng1dlpm7q74vssbb049vpf2gvw3n3r-system/boot'...
> [    2.175072] random: fast init done
> making '/gnu/store/f4ng1dlpm7q74vssbb049vpf2gvw3n3r-system' the current system..
> .
> setting up setuid programs in '/run/setuid-programs'...
> populating /etc from /gnu/store/n6cwz1hlmjylva2xrv61njl68g6c8k5l-etc...
> usermod: no changes
> usermod: no changes
> usermod: no changes
> ERROR: In procedure dynamic-func:
> In procedure dynamic-pointer: Symbol not found: strverscmp

This is because (guix utils) pulls in lots of things that are not really
meant to be used on the “build side” (a misnomer in this case, but you
see what I mean ;-)).  Here, it assumes that dlopen support is available
but it’s not because the activation snippet runs on the
statically-linked Guile.

Toggle quote (8 lines)

> From 3fa1e930b827aebca2dbbfe84c36cf203f15afda Mon Sep 17 00:00:00 2001

> From: Chris Marusich <cmmarusich@gmail.com>

> Date: Mon, 20 Aug 2018 00:16:06 -0700

> Subject: [PATCH] gnu: services: Fix pcscd activation bug.

> * gnu/services/security-token.scm (pcscd-activation): Idempotently create the

> /var/lib/pcsc symlink so that it does not fail when it already exists.

LGTM!

Toggle quote (22 lines)> +     (with-imported-modules (source-module-closure
> +                             '((guix build utils)))
> +       #~(begin
> +           (use-modules (guix build utils))
> +           ;; This switch-symlinks procedure was copied from (guix utils).  It
> +           ;; would be nice to re-use the procedure from that module, but if
> +           ;; we add that module to this gexp's imported modules and try to
> +           ;; use it, then this activation gexp can fail when it runs.  To be
> +           ;; specific, if you try to use (guix utils) and then build a VM
> +           ;; with a pcscd-service-type using "guix system vm-image", then
> +           ;; when you boot the VM, it will fail.  It fails because (guix
> +           ;; utils) dynamically links glibc's strverscmp function when
> +           ;; defining the version-compare procedure, and for some reason
> +           ;; strverscmp can't be found.  Perhaps there's a way to fix or
> +           ;; avoid this, but since we don't need the version-compare
> +           ;; procedure here, anyway, it's simpler to just define our own
> +           ;; switch-symlinks procedure instead.
> +           (define (switch-symlinks link target)
> +             (let ((pivot (string-append link ".new")))
> +               (symlink target pivot)
> +               (rename-file pivot link)))

I think “TODO: Deduplicate” would be enough as a comment, or at least
something more concise like “XXX: We cannot use (guix utils) because it
requires a dynamically-linked Guile, hence the duplicate
‘switch-symlinks’.”  I leave it up to you!

Thanks,
Ludo’.

Arun Isaac wrote on 20 Aug 2018 21:33

Re: pcscd service activation causes boot failure

Recipients:(name . Chris Marusich)(address . cmmarusich@gmail.com)

Message-ID:cu7y3d0de9o.fsf@systemreboot.net

Toggle quote (6 lines)

> Commit de30205ba0f63eb987097a9f47b6e4fd38cd9044 added a pcscd service

> (hooray!). However, its activation procedure doesn't always work. The

> system test passes, and the activation procedure works the first time

> you boot, but if you reboot, it will fail because the symlink it creates

> already exists. The attached patch fixes the problem.

Thanks for catching and fixing this! I am still in the process of
building and testing your patch. I ran out of memory and had to `guix
gc' and rebuild again. As you said, it takes a while. But, your patch
LGTM. Please go ahead and push to master at your discretion, after
addressing Ludo's feedback.

Chris Marusich wrote on 21 Aug 2018 06:52

Re: bug#32478: pcscd service activation causes boot failure

Recipients:(address . 32478-done@debbugs.gnu.org)

Message-ID:87o9dwmid8.fsf@gmail.com

Hi Ludo and Arun,

ludo@gnu.org (Ludovic Courtès) writes:

Toggle quote (9 lines)

>> ERROR: In procedure dynamic-func:

>> In procedure dynamic-pointer: Symbol not found: strverscmp

> This is because (guix utils) pulls in lots of things that are not really

> meant to be used on the “build side” (a misnomer in this case, but you

> see what I mean ;-)). Here, it assumes that dlopen support is available

> but it’s not because the activation snippet runs on the

> statically-linked Guile.

That makes sense. I keep forgetting about this. :-) Thank you for

reminding me!

Toggle quote (5 lines)

> I think “TODO: Deduplicate” would be enough as a comment, or at least

> something more concise like “XXX: We cannot use (guix utils) because it

> requires a dynamically-linked Guile, hence the duplicate

> ‘switch-symlinks’.” I leave it up to you!

Yes, I agree. I'll simplify the comment. I included the long comment

mainly in the hopes that somebody would be able to help me understand

why it wasn't working.

Arun Isaac <arunisaac@systemreboot.net> writes:

Toggle quote (12 lines)>> Commit de30205ba0f63eb987097a9f47b6e4fd38cd9044 added a pcscd service
>> (hooray!).  However, its activation procedure doesn't always work.  The
>> system test passes, and the activation procedure works the first time
>> you boot, but if you reboot, it will fail because the symlink it creates
>> already exists.  The attached patch fixes the problem.
>
> Thanks for catching and fixing this! I am still in the process of
> building and testing your patch. I ran out of memory and had to `guix
> gc' and rebuild again. As you said, it takes a while. But, your patch
> LGTM. Please go ahead and push to master at your discretion, after
> addressing Ludo's feedback.

OK! I've rewritten the comment as Ludo suggested and committed this to

master as 6fb6ac6857df31c55a640eead2a5f79edd7dab14.

Now, I'll close this bug report.

Chris

Closed

Your comment

This issue is archived.

To comment on this conversation send an email to 32478@debbugs.gnu.org

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date