refresh fails when gpio is not exported

[Ondrej Kubik]

snapd: 2.44~pre1
preconditions:
 - gadget snap has defined multiple gpio slots
 - there are snap(s) with gpio plugs connected to those slots
 - not all gpios are exported at the time of refresh

expected result:
 snap refresh refreshes snaps

actual result:
snapd goes cuckoo and refresh fails with:
error: cannot perform the following tasks:
- Setup snap "snapd" (6709) security profiles (cannot setup profiles for snap "my-gpio-consuming-snap": cannot obtain apparmor specification for snap "my-gpio-consuming-snap": lstat /sys/class/gpio/gpio401: no such file or directory)

checking the system, reported gpio is not exported so file path is not valid at the time of refresh. plug and slot for defined gpio pin are connected though

[Ondrej Kubik (ondrak)]

I was actually able to reproduce this also with snapd 2.43.3
I tested following scenarios:
edge -> beta : fail
beta -> edge: fail
beta -> candidate: fail
candidate -> beta: fail
candidate -> candidate: fail
at the time of testing candidate = stable

[Ondrej Kubik (ondrak)]

https://pastebin.canonical.com/p/Dx3NShTZVy/

[Ondrej Kubik (ondrak)]

as once possible explanations how did we actually get here is that snap in question here is also having gpio-control interface so it's likely it's self harming itself.
Regardless we should be more resilient in situation like this, and definitely not fail snapd or gadget snap updates, as those snaps are victims of the situation.

[Michael Vogt (mvo)]

Some ideas from Zyga:

<zyga> mvo: when an interface instance (plug or slot)'s corresponding method fails, we could mark it as bad and skip it in the profile generation
<zyga> mvo: we could warn (finally a use case for warnings)
<zyga> mvo: it would require some changes around but not too hard
<zyga> mvo: I would rather do it at interface level
<zyga> mvo: rather than at the level of a specific method
<zyga> mvo: because it's more proper, in my eyes, to say "this plug or slot is entirely absent"
...
<zyga> mvo: rather than to say "a specific part of the implementation failed so we skipped it"
<zyga> mvo: since those can have security implications
<zyga> mvo: .e.g. apparmor gave you access to /dev but udev tagging failed so you have open access

[Ondrej Kubik (ondrak)]

we should also consider case:
gadget: slots: gpio-1
                      gpio-2
                      gpio-3

snap-a: plug: gpio-control

snap-b: plug: gpio: connected to gpio-1, gpio-2

If snap-a unexports gpio-2, we should probably try to export it again.
Since now snap-b has no gpio-2 or way to restore it
Obviously it's bad situation already, but we might try to fix it at this
point. Since we already have service to export that gpio anyway.
just thinking loud :)

On Tue, Mar 10, 2020 at 5:20 PM Michael Vogt <email address hidden>
wrote:

> Some ideas from Zyga:
>
> <zyga> mvo: when an interface instance (plug or slot)'s corresponding
> method fails, we could mark it as bad and skip it in the profile generation
> <zyga> mvo: we could warn (finally a use case for warnings)
> <zyga> mvo: it would require some changes around but not too hard
> <zyga> mvo: I would rather do it at interface level
> <zyga> mvo: rather than at the level of a specific method
> <zyga> mvo: because it's more proper, in my eyes, to say "this plug or
> slot is entirely absent"
> ...
> <zyga> mvo: rather than to say "a specific part of the implementation
> failed so we skipped it"
> <zyga> mvo: since those can have security implications
> <zyga> mvo: .e.g. apparmor gave you access to /dev but udev tagging failed
> so you have open access
>
> ** Changed in: snapd
> Status: New => Triaged
>
> ** Changed in: snapd
> Importance: Undecided => Critical
>
> ** Changed in: snapd
> Importance: Critical => High
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1866424
>
> Title:
> refresh fails when gpio is not exported
>
> Status in snapd:
> Triaged
>
> Bug description:
> snapd: 2.44~pre1
> preconditions:
> - gadget snap has defined multiple gpio slots
> - there are snap(s) with gpio plugs connected to those slots
> - not all gpios are exported at the time of refresh
>
> expected result:
> snap refresh refreshes snaps
>
> actual result:
> snapd goes cuckoo and refresh fails with:
> error: cannot perform the following tasks:
> - Setup snap "snapd" (6709) security profiles (cannot setup profiles for
> snap "my-gpio-consuming-snap": cannot obtain apparmor specification for
> snap "my-gpio-consuming-snap": lstat /sys/class/gpio/gpio401: no such file
> or directory)
>
> checking the system, reported gpio is not exported so file path is not
> valid at the time of refresh. plug and slot for defined gpio pin are
> connected though
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/snapd/+bug/1866424/+subscriptions
>

[Ondrej Kubik (ondrak)]

and do warning that something is misbehaving :)

On Tue, Mar 10, 2020 at 5:26 PM Ondrej Kubik <email address hidden>
wrote:

> we should also consider case:
> gadget: slots: gpio-1
> gpio-2
> gpio-3
>
> snap-a: plug: gpio-control
>
> snap-b: plug: gpio: connected to gpio-1, gpio-2
>
> If snap-a unexports gpio-2, we should probably try to export it again.
> Since now snap-b has no gpio-2 or way to restore it
> Obviously it's bad situation already, but we might try to fix it at this
> point. Since we already have service to export that gpio anyway.
> just thinking loud :)
>
> On Tue, Mar 10, 2020 at 5:20 PM Michael Vogt <email address hidden>
> wrote:
>
>> Some ideas from Zyga:
>>
>> <zyga> mvo: when an interface instance (plug or slot)'s corresponding
>> method fails, we could mark it as bad and skip it in the profile generation
>> <zyga> mvo: we could warn (finally a use case for warnings)
>> <zyga> mvo: it would require some changes around but not too hard
>> <zyga> mvo: I would rather do it at interface level
>> <zyga> mvo: rather than at the level of a specific method
>> <zyga> mvo: because it's more proper, in my eyes, to say "this plug or
>> slot is entirely absent"
>> ...
>> <zyga> mvo: rather than to say "a specific part of the implementation
>> failed so we skipped it"
>> <zyga> mvo: since those can have security implications
>> <zyga> mvo: .e.g. apparmor gave you access to /dev but udev tagging
>> failed so you have open access
>>
>> ** Changed in: snapd
>> Status: New => Triaged
>>
>> ** Changed in: snapd
>> Importance: Undecided => Critical
>>
>> ** Changed in: snapd
>> Importance: Critical => High
>>
>> --
>> You received this bug notification because you are subscribed to the bug
>> report.
>> https://bugs.launchpad.net/bugs/1866424
>>
>> Title:
>> refresh fails when gpio is not exported
>>
>> Status in snapd:
>> Triaged
>>
>> Bug description:
>> snapd: 2.44~pre1
>> preconditions:
>> - gadget snap has defined multiple gpio slots
>> - there are snap(s) with gpio plugs connected to those slots
>> - not all gpios are exported at the time of refresh
>>
>> expected result:
>> snap refresh refreshes snaps
>>
>> actual result:
>> snapd goes cuckoo and refresh fails with:
>> error: cannot perform the following tasks:
>> - Setup snap "snapd" (6709) security profiles (cannot setup profiles
>> for snap "my-gpio-consuming-snap": cannot obtain apparmor specification for
>> snap "my-gpio-consuming-snap": lstat /sys/class/gpio/gpio401: no such file
>> or directory)
>>
>> checking the system, reported gpio is not exported so file path is not
>> valid at the time of refresh. plug and slot for defined gpio pin are
>> connected though
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/snapd/+bug/1866424/+subscriptions
>>
>

[Michael Vogt (mvo)]

I pushed PR#8249 that should address this.

[Michael Vogt (mvo)]

This is fixed with the 2.44 release.