Skip to content

Disable YT6801 private ioctl interface - #263

Draft
AFOliveira wants to merge 1 commit into
omacom:masterfrom
AFOliveira:codex/harden-yt6801-private-ioctl
Draft

Disable YT6801 private ioctl interface#263
AFOliveira wants to merge 1 commit into
omacom:masterfrom
AFOliveira:codex/harden-yt6801-private-ioctl

Conversation

@AFOliveira

Copy link
Copy Markdown

What

  • Update yt6801-dkms to the current official Motorcomm 1.0.34 source.
  • Remove both netdevice private-ioctl callbacks and omit the diagnostic ioctl object from the linked module.
  • Add a packaging check that prevents either interface from returning.
  • Disable scheduled AUR replacement for this security-maintained package.

Why

The vendor private ioctl exposes an unprivileged command parser with unsafe buffer handling and direct device-control operations. Omarchy is migrating supported hardware to the in-tree kernel driver; this package change is defense in depth for any system that still builds the external module.

Verification

  • Official archive and patch SHA-256 validation passed.
  • Complete makepkg prepare, check, and package cycle passed.
  • The 1.0.34 module built against Arch Linux 7.1.9 headers.
  • nm confirms fxgmac_netdev_ops_ioctl, fxgmac_ioctl, and fxgmac_siocdevprivate are absent.
  • bin/sync-aur yt6801-dkms skips the package and leaves the hardened tree unchanged.
  • git diff --check

@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed. Nothing blocking — the change does what it says, and I was able to prove the part that mattered most rather than take it on trust.

What was actually run. On a disposable Omarchy worker VM (kernel 7.1.9-arch1-2, not on the machine holding credentials), I fetched the vendor archive, applied this branch's patch.diff to a pristine tree, and compiled the module. A package build was not run — bin/build only packages DKMS sources, so the compile that matters happens on each user's machine at install and at every kernel upgrade, and that is the one I wanted to see.

  • The source checksum is real: the file behind downloadFile.html?id=1817 hashes to 877e6953…, exactly the sha256sums entry, and the zip contains yt6801-1.0.34.tar.gz, which extracts flat into src/ as prepare() and package() assume.
  • patch -p1 applies cleanly to pristine 1.0.34, all three files, no fuzz.
  • make -C /usr/lib/modules/7.1.9-arch1-2/build M=$PWD modules → exit 0, no warnings, yt6801.ko produced.
  • nm yt6801.ko | grep -i ioctl → nothing. The interface is genuinely gone from the linked module, not merely unreferenced: the two net_device_ops callbacks are deleted and fuxi-gmac-ioctl.o is dropped from yt6801-objs.

The compat hunks you dropped are safe to drop, and here is why. Rewriting patch.diff removes the old timer_container_of / timer_init_key / timer_shutdown_sync shims and the FXGMAC_INT_MODERATION_ENABLED define, which looked like the risky part of this PR, because from_timer, init_timer_key and del_timer_sync are all absent from 7.1.9's headers. 1.0.34 covers every one of them itself:

  • fuxi-gmac-phy.c:340/342 and :374/376/378 already carry the KERNEL_VERSION(6,16,0) branches upstream.
  • fuxi-gmac-phy.c:392-397 guards on KERNEL_VERSION(6,2,0) and calls timer_delete_sync.
  • The remaining from_timer at fuxi-gmac-net.c:776 sits behind #if FXGMAC_TX_HANG_TIMER_ENABLED, which fuxi-os.h:141 defines as 0.
  • FXGMAC_INT_MODERATION_ENABLED now comes from Makefile:26 (ccflags-y += -D…=$(moderation_en)); the build used -DFXGMAC_INT_MODERATION_ENABLED=1, so the old define was redundant rather than load-bearing.

That is worth stating explicitly in the PR body — a future reader diffing the two patches will otherwise think the kernel-compat work was lost.

Two things worth a look, neither blocking:

  1. PKGBUILD:24-29 — the new check() is fail-open. grep -q exits 2 when the file is missing, which the if reads as "no match", so check() returns success when ${srcdir}/src/fuxi-gmac-net.c or src/Makefile is not where it expects. prepare() would normally fail first, so it is masked today, but a guard whose whole job is to stop the ioctl coming back should fail closed. [[ -f … && -f … ]] || return 1 ahead of the greps covers it. Codex reproduced this independently and got check_status=0 with both paths absent.

  2. PKGBUILD:12downloadFile.html?id=1817 is a mutable record, not a versioned URL. That same id served 1.0.31 at the previous checksum and serves 1.0.34 now. It fails closed (the checksum stops matching), but it means the exact source this hardening is pinned to becomes unfetchable the day Motorcomm publishes 1.0.35, and a rebuild will need a same-day PR. Pre-existing, not introduced here, but it matters more now that a security patch rides on it.

Minor: package()'s install -Dm644 ${srcdir}/src/*.c still ships fuxi-gmac-ioctl.c into /usr/src/yt6801-1.0.34/. It is dormant — nothing links it — but it is dead source in the shipped tree.

On merge order with basecamp/omarchy#9682. I looked, and there is no trap: the two touch different repositories and no shared files, and 263 does not depend on anything 9682 changes. The ordering argument runs one way only — this should land first or independently. 9682's own migration is explicitly retry-safe and can leave a machine on the old driver until it reboots into a kernel carrying the in-tree alias, and every machine in that window keeps the unhardened DKMS module unless this has already shipped. Landing 9682 first buys nothing and leaves that gap open. After 9682 has fully rolled out, yt6801-dkms is orphaned in the repo rather than broken.

On PR #271 (linux-ptl → 7.2.2.arch1): no file overlap — it touches only pkgbuilds/linux-ptl/, and "carries all patches" refers to linux-ptl's own patch series, not this one. The real interaction is that my build here is against 7.1.9 only. Since this is DKMS, an incompatibility with 7.2.2 would surface as a build failure on users' machines at upgrade rather than anywhere in CI, and I could not test that — linux-ptl in the repo is still 7.1.8, so 7.2.2 headers were not available to me. Worth a build check against 7.2.2 before both land.

Second opinion: Codex at xhigh reviewed this independently. It reached the fail-open check() and the mutable-URL findings on its own and confirmed the ioctl removal covers the linked module; it also traced "sync": false through helpers/package-metadata.sh:61 and bin/sync-aur:321, which I had only checked at the second of those. Its independence is not currently guaranteed, so where we agree, treat that as two readings rather than two proofs. It flagged the dropped timer shims as an unverified release blocker and said the archive had to be compiled before that could be cleared — it had no worker and could not do so. The build above is that check, and it clears it.

Nothing was pushed to this branch. Waiting on the maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants