Comment 1 for bug 1991508

Review for Package: licheerv-rtl8723ds-dkms

[Summary]
MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.
As this dkms module is interacting a lot with HAL and the radio stack, I’m not confortable enough in my code review to be bullet proof. Consequently, this does need a security review, so I'll assign ubuntu-security.
List of specific binary packages to be promoted to main: licheerv-rtl8723ds-dkms

Notes:
Required TODOs:
- There is no autopkgtests/build time test for this dkms module. There are some comments that the kernel team is going to do some testing for every release, which is OK. However, (as described in the MIR template), one requirement is to make this test plan written down (like, on the ubuntu wiki) and linked here. That way, we can ensure that every upload is tested with someone having access to the hardware as a minimal, QAed, assurance.
Recommended TODOs:
- There is no debian/watch. We also thus don’t know if the current release is packaged (we have latest commit from Git included though). Is it possible to modify debian/watch to take latest upstream main branch as it seems upstream don’t cut releases?
- It seems to me that it could be easy to fix the lintian warning (or override some of them), so that the output is cleaned when building. Mind having a look?

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
   - SRCPKG checked with `check-mir`
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring more tests now.

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root (but it’s a kernel module, running as root then, see problems)
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates, signing, ...)

Problems:
As part of the kernel itself, it does run as root. It’s interacting a lot with HAL and the wireless stack. It does parse many different binary protocols. This is why a security review is in order.

[Common blockers]
OK:
- does not FTBFS currently
- no new python2 dependency

Problems:
- There is no autopkgtests/build time test for this dkms module. There are some comments that the kernel team is going to do some testing for every release, which is OK. However, (as described in the MIR template), one requirement is to make this test plan written down (like, on the ubuntu wiki) and linked here. That way, we can ensure that every upload is tested with someone having access to the hardware as a minimal, QAed, assurance.
Recommended TODOs:

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- Upstream update history is unknown (too NEW to assess)
- Debian/Ubuntu update history is unknown (too NEW to assess)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
- no massive Lintian warnings
- d/rules is rather clean
- It is not on the lto-disabled list
 (fix, or the work-around should be directly in the package,
 see https://launchpad.net/ubuntu/+source/lto-disabled-list)

Problems:
- There is no debian/watch. We also thus don’t know if the current release is packaged (we have latest commit from Git included though). Is it possible to modify debian/watch to take latest upstream main branch as it seems upstream don’t cut releases?
- It seems to me that it could be easy to fix the lintian warning (or override some of them), so that the output is cleaned when building. Mind having a look?

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as we can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
 - not part of the UI for extra checks
- no translation present, but none needed for this case