[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Tricca, Philip B]

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

Any chance I can get a PR on github? I can pull the patch directly if you prefer but no guarantees I'll get all of the metadata right.

-----Original Message-----
From: Steven Clark [mailto:davolfman(a)gmail.com] 
Sent: 27 November, 2019 14:27
To: tpm2(a)lists.01.org
Subject: [tpm2] [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

I've created a situation where systemd starts with most of the root filesystem in disk cache already (as a side effect) and discovered that tpm2-abrmd.service has no After or Requires directives.  Systemd was running tpm2-abrmd,service before the udev rule had chowned the tpm device and as a result it was hanging until the 5-second restart countdown finished.  The following patch fixed it for me, by forcing the resource manager to wait until udev settled.  Patch is against
2.2.0 but should be easy to port.

---
 dist/tpm2-abrmd.service.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in index 00aa031..4300cb3 100644
--- a/dist/tpm2-abrmd.service.in
+++ b/dist/tpm2-abrmd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=TPM2 Access Broker and Resource Management Daemon
+After=systemd-udev-settle.service
+Requires=systemd-udev-settle.service

 [Service]
 Type=dbus
_______________________________________________
tpm2 mailing list -- tpm2(a)lists.01.org
To unsubscribe send an email to tpm2-leave(a)lists.01.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Steven Clark]

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

I pulled the patch for the wrong branch.  Here's the version for
2.3.0.  I was wondering why it had the wrong email.
From a2cb4d84507a12d0cf701ad12cbe2e1416e1f091 Mon Sep 17 00:00:00 2001
From: Steven Clark <davolfman(a)gmail.com>
Date: Tue, 3 Dec 2019 01:39:28 +0000
Subject: [PATCH 1/1] Hold Resource Manager until tpm is accessible

In testing in an unusual boot scenario it was discovered that
tpm2-abrmd.service was starting before udev had changed ownership
of the TPM device from root to TSS.  This patch changes the
Systemd unit file to force a udev settle before starting the
resource manager service.

Signed-off-by: Steven Clark <davolfman(a)gmail.com>
---
 dist/tpm2-abrmd.service.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in
index 00aa031..4300cb3 100644
--- a/dist/tpm2-abrmd.service.in
+++ b/dist/tpm2-abrmd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=TPM2 Access Broker and Resource Management Daemon
+After=systemd-udev-settle.service
+Requires=systemd-udev-settle.service

 [Service]
 Type=dbus
-- 
2.18.1

On Thu, Dec 12, 2019 at 11:09 AM Philip Tricca <flihp(a)twobit.org> wrote:
>
> Thanks Steve. I don't have much time these days to work on this stuff so
> no worries about the delay. I'll get this in and turn the crank on a
> bugfix release.
>
> Thanks,
> Philip
>
> On 12/12/19 10:52 AM, Steven Clark wrote:
> > Here's a version with a sign-off.  By the time I hit a lull I had to
> > resynthesize the repository used to build the patch.  At least I get
> > some process practice with something innocuous this way.
> >
> > From 4b95f1d7988a2f4d4d5e1f7b19602a8f0b1c66a4 Mon Sep 17 00:00:00 2001
> > From: Steven Clark <davolfman(a)gmail.com>
> > Date: Wed, 27 Nov 2019 21:10:00 +0000
> > Subject: [PATCH 1/1] Hold Resource Manager until tpm is accessible
> >
> > In testing in an unusual boot scenario it was discovered that
> > tpm2-abrmd.service was starting before udev had changed ownership
> > of the TPM device from root to TSS.  This patch changes the
> > Systemd unit file to force a udev settle before starting the
> > resource manager service.
> >
> > Signed-off-by: Steven Clark <davolfman(a)gmail.com>
> > ---
> >  dist/tpm2-abrmd.service.in | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in
> > index 00aa031..4300cb3 100644
> > --- a/dist/tpm2-abrmd.service.in
> > +++ b/dist/tpm2-abrmd.service.in
> > @@ -1,5 +1,7 @@
> >  [Unit]
> >  Description=TPM2 Access Broker and Resource Management Daemon
> > +After=systemd-udev-settle.service
> > +Requires=systemd-udev-settle.service
> >
> >  [Service]
> >  Type=dbus
> >
>

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Philip Tricca]

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

Thanks Steve. I don't have much time these days to work on this stuff so
no worries about the delay. I'll get this in and turn the crank on a
bugfix release.

Thanks,
Philip

On 12/12/19 10:52 AM, Steven Clark wrote:
> Here's a version with a sign-off.  By the time I hit a lull I had to
> resynthesize the repository used to build the patch.  At least I get
> some process practice with something innocuous this way.
> 
> From 4b95f1d7988a2f4d4d5e1f7b19602a8f0b1c66a4 Mon Sep 17 00:00:00 2001
> From: Steven Clark <davolfman(a)gmail.com>
> Date: Wed, 27 Nov 2019 21:10:00 +0000
> Subject: [PATCH 1/1] Hold Resource Manager until tpm is accessible
> 
> In testing in an unusual boot scenario it was discovered that
> tpm2-abrmd.service was starting before udev had changed ownership
> of the TPM device from root to TSS.  This patch changes the
> Systemd unit file to force a udev settle before starting the
> resource manager service.
> 
> Signed-off-by: Steven Clark <davolfman(a)gmail.com>
> ---
>  dist/tpm2-abrmd.service.in | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in
> index 00aa031..4300cb3 100644
> --- a/dist/tpm2-abrmd.service.in
> +++ b/dist/tpm2-abrmd.service.in
> @@ -1,5 +1,7 @@
>  [Unit]
>  Description=TPM2 Access Broker and Resource Management Daemon
> +After=systemd-udev-settle.service
> +Requires=systemd-udev-settle.service
> 
>  [Service]
>  Type=dbus
> 

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Steven Clark]

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

Here's a version with a sign-off.  By the time I hit a lull I had to
resynthesize the repository used to build the patch.  At least I get
some process practice with something innocuous this way.

From 4b95f1d7988a2f4d4d5e1f7b19602a8f0b1c66a4 Mon Sep 17 00:00:00 2001
From: Steven Clark <davolfman(a)gmail.com>
Date: Wed, 27 Nov 2019 21:10:00 +0000
Subject: [PATCH 1/1] Hold Resource Manager until tpm is accessible

In testing in an unusual boot scenario it was discovered that
tpm2-abrmd.service was starting before udev had changed ownership
of the TPM device from root to TSS.  This patch changes the
Systemd unit file to force a udev settle before starting the
resource manager service.

Signed-off-by: Steven Clark <davolfman(a)gmail.com>
---
 dist/tpm2-abrmd.service.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in
index 00aa031..4300cb3 100644
--- a/dist/tpm2-abrmd.service.in
+++ b/dist/tpm2-abrmd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=TPM2 Access Broker and Resource Management Daemon
+After=systemd-udev-settle.service
+Requires=systemd-udev-settle.service

 [Service]
 Type=dbus
-- 
2.18.1

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Tricca, Philip B]

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

Sorry to do this to you Steven but I need a 'signed-off-by' line to meet DCO requirements. Can you please update?

Thanks,
Philip

-----Original Message-----
From: Steven Clark [mailto:davolfman(a)gmail.com] 
Sent: 3 December, 2019 10:39
To: Tricca, Philip B <philip.b.tricca(a)intel.com>
Cc: tpm2(a)lists.01.org
Subject: Re: [tpm2] [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

Here's a full patch against 2.3.0, I decided against uploading to github for now:

From c75d97443efadb8495087a777d06f3c0f5f43e87 Mon Sep 17 00:00:00 2001
From: Steven Clark <davolfman(a)gmail.com>
Date: Tue, 3 Dec 2019 01:39:28 +0000
Subject: [PATCH] Hold Resource Manager until tpm is accessible

In testing in an unusual boot scenario it was discovered that tpm2-abrmd.service was starting before udev had changed ownership of the TPM device from root to TSS.  This patch changes the Systemd unit file to force a udev settle before starting the resource manager service.
---
 dist/tpm2-abrmd.service.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in index 00aa031..4300cb3 100644
--- a/dist/tpm2-abrmd.service.in
+++ b/dist/tpm2-abrmd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=TPM2 Access Broker and Resource Management Daemon
+After=systemd-udev-settle.service
+Requires=systemd-udev-settle.service

 [Service]
 Type=dbus

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Steven Clark]

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

Here's a full patch against 2.3.0, I decided against uploading to
github for now:

From c75d97443efadb8495087a777d06f3c0f5f43e87 Mon Sep 17 00:00:00 2001
From: Steven Clark <davolfman(a)gmail.com>
Date: Tue, 3 Dec 2019 01:39:28 +0000
Subject: [PATCH] Hold Resource Manager until tpm is accessible

In testing in an unusual boot scenario it was discovered that
tpm2-abrmd.service was starting before udev had changed ownership
of the TPM device from root to TSS.  This patch changes the
Systemd unit file to force a udev settle before starting the
resource manager service.
---
 dist/tpm2-abrmd.service.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dist/tpm2-abrmd.service.in b/dist/tpm2-abrmd.service.in
index 00aa031..4300cb3 100644
--- a/dist/tpm2-abrmd.service.in
+++ b/dist/tpm2-abrmd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=TPM2 Access Broker and Resource Management Daemon
+After=systemd-udev-settle.service
+Requires=systemd-udev-settle.service

 [Service]
 Type=dbus

[tpm2] Re: [tpm2][tpm2-abrmd]tpm2-abrmd.service may run before udev rule.

[Steven Clark]

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

It would be largely unrelated to the project's actual source control,
but I could manufacture one, given a day or two, if you want.  It
might even be from HEAD and tested if I can find the time.  I've been
using my personal account for interaction with the mailing list until
there's an SOP for contributing back from work.

This patch could probably use a little outside review, honestly.
Systemd ordering cycles have mostly non-deterministic failure
behavior, as part of the spec.  If someone out there is performing TPM
operations in a systemd-based initramfs (maybe Dracut) and could check
they don't end up with any cycles in their graph it would be nice.
This has only been tested with a manual init system and no ABRMD in
the initramfs in my case.