Re: [PATCH net v6] rust: net: phy: fix off-by-one bit positions in device status accessors

[FUJITA Tomonori <tomo@flapping.org>]

On Thu, 10 Sep 2026 05:51:10 +0000
Chunfeng Song <springbreeze@stu.pku.edu.cn> wrote:

> The hand-written bitfield offsets in is_link_up(), is_autoneg_enabled()
> and is_autoneg_completed() were correct when the abstraction was
> merged: at that time autoneg, link, and autoneg_complete were at bits
> 13, 14, and 15 of struct phy_device's first bitfield unit. Commit
> 2796ff1e3dca ("net: phy: add flag is_genphy_driven to struct phy_device")
> later inserted is_genphy_driven just before autoneg, shifting the three
> fields up by one, so the accessors now read:
> 
>   is_link_up()           reads bit 14 = autoneg
>   is_autoneg_enabled()   reads bit 13 = is_genphy_driven
>   is_autoneg_completed() reads bit 15 = link
> 
> The official ax88796b Rust driver uses all three accessors in its
> read_status() implementation, so it inherits the bug.
> phy_attach_direct() sets is_genphy_driven only when it falls back to
> the generic driver, and ax88796b has a real driver, so
> is_genphy_driven stays 0. The broken is_autoneg_enabled() therefore
> reads bit 13 as 0, compares it against AUTONEG_ENABLE (1), and always
> returns false, so read_status() never reaches the
> resolve_aneg_linkmode() call.
> 
> The ordinary bindgen accessors take &self. Calling them through
> (*phydev).link() would create a shared reference to the complete
> bindings::phy_device, which is not appropriate for an object wrapped in
> Opaque.
> 
> Use the bindgen-generated raw accessors (link_raw(), autoneg_raw(),
> and autoneg_complete_raw()) instead. They retain the bit positions and
> endianness handling generated from the C layout without creating a Rust
> reference to the complete phy_device. Drop the hand-written numbers
> together with the TODO comment that marked them as a stopgap.
> 
> The raw accessors are only emitted by bindgen 0.71 and later, and were
> added at the Rust-for-Linux project's request, so this fix can only be
> backported to stable branches whose minimum bindgen version is at least
> that, hence the scope on the Cc: stable line below.
> 
> Found by a static equivalence audit (C2RustDrv, a C-to-Rust driver
> migration tool) that compares hand-written bitfield offsets against
> the bindgen layout of struct phy_device. Verified by building the
> bindings and checking the generated accessors; no runtime testing was
> possible without PHY hardware.
> 
> Fixes: 2796ff1e3dca ("net: phy: add flag is_genphy_driven to struct phy_device")
> Cc: stable@vger.kernel.org # Only 7.1.y and later (requires bindgen's raw pointer accessors).
> Link: https://github.com/rust-lang/rust-bindgen/issues/2674
> Signed-off-by: Chunfeng Song <springbreeze@stu.pku.edu.cn>

Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Thanks,