Request quectel-firmware-switch plug: 'custom-sysfiles' to 'system-files' interface

store-requests
1

Dears,

This request follows Auto-connection request: quectel-firmware-switch.

It’s for the https://snapcraft.io/quectel-firmware-switch snap, which provides a facility for managing firmware on Quectel EM120/EM160 LTE device for cellular connection. There’re several different firmware images bundled in this snap, a correct firmware image will be selected and updated on the LTE device in background when a new SIM card is detected.

The customized plug ‘custom-sysfiles’ defined in snapcraft.yaml need your approval:

plugs:
  custom-sysfiles:
    interface: system-files
      read:
        - /sys/module/firmware_class/parameters/path
        - /sys/bus/pci/drivers/mhi-pci-generic/bind
        - /dev/wwan0qcdm0
        - /dev/wwan0firehose0
        - /dev/wwan1qcdm0
        - /dev/wwan1firehose0
        - ..
      write:
        - /sys/module/firmware_class/parameters/path
        - /sys/bus/pci/drivers/mhi-pci-generic/bind
        - /dev/wwan0qcdm0
        - /dev/wwan0firehose0
        - /dev/wwan1qcdm0
        - /dev/wwan1firehose0
        - ..

Reason for approval :

This snap need to have the above permissions to access modem device and sysfs to get current firmware info and write new one to the active modem device.

Thanks in advance!

2

Firstly, write permission implies read so there is no need to list both - if write access to all of these files is required then you can remove the read section as that is redundant. Also the name custom-sysfiles is not really in keeping with the conventions so we may need to find a better / more descriptive name for this.

Secondly, access to device files should generally be provided by a specific snapd interface - however there is no such interface for these devices you have listed above. I wonder if instead whether snapd should be extended to allow application snaps to use the custom-device interface to declare such interfaces for themselves - this would then allow to get both the required AppArmor rules as well as the udev rules to properly setup the device cgroup for the snap (whereas using system-files will only grant the AppArmor rules). @pedronis could you comment?

Thirdly, regarding the other two paths

  • /sys/module/firmware_class/parameters/path - this looks to be specific to a particular kernel module - can you provide more details on this as I am not familiar with that - what does this file provide / why is write access required to it?
  • /sys/bus/pci/drivers/mhi-pci-generic/bind - again I am not familiar with this - can you provide more information?

Finally, you have trailing .. at the end which makes me wonder if there are more files declared in your interface - can you please ensure you have provided complete details in your request? Thanks.

3

Yes, this is something I’m considering, we have other use cases for this.

@alexmurray This proposed change interfaces: simplify the InstallCandidateMinimalCheck to help expressivity by pedronis · Pull Request #12389 · canonical/snapd · GitHub relates to that.

4

Hi @zhanggyb,

Could you please provide the requested information? Thanks!

5

Dears,

(Sorry for my late reply.)

Thanks for your comments and I just updated the snapcraft.yaml per @alexmurray 's suggestions.

Here is the part of the .yaml describes the interfaces of this quectel-firmware-switch snap:

slots:
  quectel-modem-slot:
    interface: custom-device
    custom-device: quectel-wwan-modem
    devices:
      - /dev/wwan[0-9]p[0-9]QCDM
      - /dev/wwan[0-9]p[0-9]FIREHOSE
      - /dev/wwan[0-9]qcdm[0-9]
      - /dev/wwan[0-9]firehose[0-9]

plugs:
  quectel-modem:
    interface: custom-device
    custom-device: quectel-wwan-modem
  quectel-fwupdate:
    interface: system-files
    write:
      - /sys/module/firmware_class/parameters/path
      - /sys/bus/pci/drivers/mhi-pci-generic/bind

Where (Quectel’s chip is based on Qualcomm’s):

  • /sys/module/firmware_class/parameters/path
    is for the subsystem of kernel firmware, used to pass the absolute address of the modem’s firmware image (like the image for Quectel EM120, EM160), and switch to Qualcomm’ firehose upgrade mode.

  • /sys/bus/pci/drivers/mhi-pci-generic/bind is the bind point of Qualcomm’s PCIe MHI driver. The firmware tool bundled in this snap uses this sysfs node to write Quectel PCIe ID, to trigger the firehose upgrade mode

  • The device files listed in custom-device slot:

    • /dev/wwan[0-9]p[0-9]QCDM and /dev/wwan[0-9]p[0-9]FIREHOSE are the device nodes create by the driver in kernel version prior to 5.15

    • /dev/wwan[0-9]qcdm[0-9] and /dev/wwan[0-9]firehose[0-9], since kernel 5.15, these devices are renamed with lowercase

    • The default modem device after system startup uses index of 0 and will be increased after reset. This won’t always happen so the number between 0 - 9 is enough from my perspective.

And the link to the revisions of this snap: https://dashboard.snapcraft.io/snaps/quectel-firmware-switch/revisions/ . Where you can find the latest revision waiting for manual review, not sure if you need this.

6

Apologies for the long delay on responding to this request.

This is a tricky situation since ideally the snap would use custom-device but currently it is not possible for application snaps (only gadget snaps).

Therefore, I think the best way to proceed at this time is to instead go back to using system-files to declare the /dev/wwanNxxx devices. Note however you cannot use regexs here, and instead will have to list the various device files by full path. Then we can look at granting that, and if/when snapd supports using custom-device by application snaps, we can transition to that.

Thanks.

7

Thanks! I’ve changed the snap config to use system-files back on my side, like:

plugs:
  quectel-fwupdate:
    interface: system-files
    write:
      - /sys/module/firmware_class/parameters/path
      - /sys/bus/pci/drivers/mhi-pci-generic/bind
      - /dev/wwan0qcdm0
      - /dev/wwan0qcdm1
      .
      .
      - /dev/wwan0firehose0
      - /dev/wwan0firehose1
      .
      .

Please be noted the custom plug is renamed from custom-sysfiles (in the title) to quectel-fwupdate to describe the purpose of using those system files.

Is this OK and can get approval for the quectel-fwupdate?

The snap revision was submitted and waiting for manual review: https://dashboard.snapcraft.io/snaps/quectel-firmware-switch/revisions/20/

8

Thanks - so to close the loop on this - the use of /sys/module/firmware_class/parameters/path feels like they it potentially allow the snap to take device ownership since this allows the snap to get the kernel to load any firmware image that it provides in preference to the ones on the device - and there is nothing here that is specific to just the Quectel devices/driver here - so I think it could also allow the snap to load firmware for other devices on the system as well.

However, given the snap is from a verified publisher this seems reasonable (and still much more constrained that a snap with classic confinement).

Also whilst the long list of possible device files is a bit unwieldy, there is no other way currently to achieve this access (ideally snapd would implement hotplug support for such devices).

Therefore, +1 from me for use-of and auto-connect of this system-files interface.

9

+1 from my side to use auto-connect request on system-files. I agree with @alexmurray, since this snap is from a vetted publisher, this request sounds appropriate

+2 for, 0 against, granting auto-connect and use of system-files called quectel-fwupdate to have a write access to /sys/module/firmware_class/parameters/path, /sys/bus/pci/drivers/mhi-pci-generic/bind and defined files starting from /dev/wwan*. This is now live