Re: [PATCH iwl-net v3 2/6] ixgbe: add bounds check for debugfs register access

[Larysa Zaremba <larysa.zaremba@intel.com>]

Tony, please take this patch off the dev-queue, it is conceptually wrong and 
prevents probe on E610 and other adapters most probably, details below.

On Wed, Apr 15, 2026 at 04:28:37PM +0200, Aleksandr Loktionov wrote:
> From: Paul Greenwalt <paul.greenwalt@intel.com>
> 
> Prevent out-of-bounds MMIO accesses triggered through user-controlled
> register offsets.  IXGBE_HFDR (0x15FE8) is the highest valid MMIO
> register in the ixgbe register map; any offset beyond it would address
> unmapped memory.
> 
> Add a defense-in-depth check at two levels:
> 
> 1. ixgbe_read_reg() -- the noinline register read accessor.  A
>    WARN_ON_ONCE() guard here catches any future code path (including
>    ioctl extensions) that might inadvertently pass an out-of-range
>    offset without relying on higher layers to catch it first.
>    ixgbe_write_reg() is a static inline called from the TX/RX hot path;
>    adding WARN_ON_ONCE there would inline the check at every call site,
>    so only the read path gets this guard.
> 
> 2. ixgbe_dbg_reg_ops_write() -- the debugfs 'reg_ops' interface is the
>    only current path where a raw, user-supplied offset enters the driver.
>    Gating it before invoking the register accessors provides a clean,
>    user-visible failure (silent ignore with no kernel splat) for
>    deliberately malformed debugfs writes.
> 
> Add a reg <= IXGBE_HFDR guard to both the read and write paths in
> ixgbe_dbg_reg_ops_write(), and a WARN_ON_ONCE + early-return guard to
> ixgbe_read_reg().

Here is IXGBE_HFDR definition:

#define IXGBE_HFDR      0x15FE8

Meanwhile e.g. E610 uses this register:

#define IXGBE_EEC_X550EM_a      0x15FF8

This leads to the following call trace on probe of E610:

[    4.828801] ------------[ cut here ]------------
[    4.829566] WARNING: drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:356 at ixgbe_read_reg+0x4d/0x140 [ixgbe], CPU#5: (udev-worker)/709
[    4.831135] Modules linked in: intel_uncore(-) ixgbe(+) pcspkr libie_fwlog i2c_i801 i2c_smbus mdio virtio_net(+) dca lpc_ich virtio_balloon net_failover failover joydev 9p fuse loop zram lz4hc_compress lz4_compress 9pnet_virtio 9pnet netfs serio_raw qemu_fw_cfg
[    4.833733] CPU: 5 UID: 0 PID: 709 Comm: (udev-worker) Not tainted 7.1.0-rc2+ #4 PREEMPT(lazy)
[    4.834782] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-8.fc42 06/10/2025
[    4.835798] RIP: 0010:ixgbe_read_reg+0x4d/0x140 [ixgbe]
[    4.836474] Code: 48 83 ec 28 f6 87 4b 06 00 00 02 75 29 89 c6 41 8b 04 30 83 f8 ff 0f 84 92 00 00 00 48 83 c4 28 5b 41 5e 41 5f c3 cc cc cc cc <0f> 0b b8 ff ff ff ff e9 d7 22 35 fc 49 8d b0 98 42 00 00 41 b9 c8
[    4.838714] RSP: 0018:ffffcd2980ea39a8 EFLAGS: 00010202
[    4.839401] RAX: ffffffffc02e29d0 RBX: ffff8aa2442001c0 RCX: 0000000000000000
[    4.840296] RDX: ffffffffc069cd80 RSI: 0000000000015ff8 RDI: ffff8aa244201480
[    4.841176] RBP: ffff8aa241d1f000 R08: ffffcd2983000000 R09: 0000000000180000
[    4.842025] R10: 0000000000180fff R11: ffffcd2983ffffff R12: ffffffffc069e3e0
[    4.842886] R13: ffff8aa241d1f0d0 R14: 000000000000003f R15: ffff8aa2487e3000
[    4.843794] FS:  00007f1cbd6f5980(0000) GS:ffff8aa2fd25b000(0000) knlGS:0000000000000000
[    4.844799] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    4.845521] CR2: 00005634f0c7e018 CR3: 0000000110ff6005 CR4: 0000000000770ef0
[    4.846485] PKRU: 55555554
[    4.847138] Call Trace:
[    4.847743]  <TASK>
[    4.848144]  ixgbe_probe+0x2af/0xea0 [ixgbe]
[    4.848809]  local_pci_probe+0x3e/0x90
[    4.849471]  pci_call_probe+0x59/0x190
[    4.850021]  ? pci_match_device+0x15f/0x180
[    4.850687]  ? pci_assign_irq+0x2d/0x160
[    4.851382]  pci_device_probe+0x95/0x170
[    4.852029]  call_driver_probe+0x26/0x100
[    4.852791]  ? driver_sysfs_add+0x59/0xd0
[    4.853454]  really_probe+0xcc/0x2c0
[    4.854362]  __driver_probe_device+0x84/0x170
[    4.854443] iTCO_wdt iTCO_wdt.1.auto: Found a ICH9 TCO device (Version=2, TCOBASE=0x0660)
[    4.854971]  driver_probe_device+0x1f/0xa0
[    4.856274] iTCO_wdt iTCO_wdt.1.auto: initialized. heartbeat=30 sec (nowayout=0)
[    4.856467]  ? __pfx___driver_attach+0x10/0x10
[    4.858133]  __driver_attach+0xcb/0x210
[    4.858138]  bus_for_each_dev+0x85/0xd0
[    4.858141]  bus_add_driver+0x13c/0x220
[    4.858144]  ? __pfx_ixgbe_init_module+0x10/0x10 [ixgbe]
[    4.860394]  driver_register+0x75/0xe0
[    4.860881]  ixgbe_init_module+0x92/0xff0 [ixgbe]
[    4.861509]  do_one_initcall+0x5d/0x320
[    4.862019]  do_init_module+0x84/0x290
[    4.862521]  init_module_from_file+0xd8/0x140
[    4.863084]  idempotent_init_module+0x114/0x310
[    4.863665]  __x64_sys_finit_module+0x71/0xe0
[    4.864237]  do_syscall_64+0xe2/0x660
[    4.864726]  ? __seccomp_filter+0x52/0x380
[    4.865265]  ? do_syscall_64+0x99/0x660
[    4.865767]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[    4.866472] RIP: 0033:0x7f1cbe0d15ed
[    4.866994] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 13 48 0c 00 f7 d8 64 89 01 48
[    4.869093] RSP: 002b:00007ffc1e56d968 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[    4.870005] RAX: ffffffffffffffda RBX: 00005634f131bf00 RCX: 00007f1cbe0d15ed
[    4.870865] RDX: 0000000000000000 RSI: 00007f1cbe5f907d RDI: 0000000000000010
[    4.871732] RBP: 00007ffc1e56da20 R08: 0000000000000000 R09: 00007ffc1e56d9b0
[    4.872592] R10: 0000000000000010 R11: 0000000000000246 R12: 00007f1cbe5f907d
[    4.873451] R13: 0000000000020000 R14: 00005634f0ce8ae0 R15: 00005634f131d640
[    4.874318]  </TASK>
[    4.874635] ---[ end trace 0000000000000000 ]---

Here are the warning locations:

(gdb) l *ixgbe_probe+0x2af
0x1480f is in ixgbe_probe (drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:11696).
11691                   hw->link.ops  = *ii->link_ops;
11692
11693           /* EEPROM */
11694           hw->eeprom.ops = *ii->eeprom_ops;
11695           eec = IXGBE_READ_REG(hw, IXGBE_EEC(hw));
11696           if (ixgbe_removed(hw->hw_addr)) {
11697                   err = -EIO;
11698                   goto err_ioremap;
11699           }
11700           /* If EEPROM is valid (bit 8 = 1), use default otherwise use bit bang */

(gdb) l *ixgbe_read_reg+0x4d
0x703d is in ixgbe_read_reg (drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:356).
351             u8 __iomem *reg_addr = READ_ONCE(hw->hw_addr);
352             u32 value;
353
354             if (ixgbe_removed(reg_addr))
355                     return IXGBE_FAILED_READ_REG;
356             if (WARN_ON_ONCE(reg > IXGBE_HFDR))
357                     return IXGBE_FAILED_READ_REG;
358             if (unlikely(hw->phy.nw_mng_if_sel &
359                          IXGBE_NW_MNG_IF_SEL_SGMII_ENABLE)) {
360                     struct ixgbe_adapter *adapter;


> 
> Fixes: 91fbd8f081e2 ("ixgbe: added reg_ops file to debugfs")
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> Cc: stable@vger.kernel.org
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> v2 -> v3:
>  - Add Reviewed-by: Simon Horman; no code change.
> 
> v1 -> v2:
>  - Add Fixes: tag; reroute from iwl-next to iwl-net (security-relevant
>    hardening for user-controllable out-of-bounds MMIO).
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 6 ++++--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
> index 5b1cf49d..a6a19c0 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
> @@ -86,7 +86,8 @@ static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
>  		u32 reg, value;
>  		int cnt;
>  		cnt = sscanf(&ixgbe_dbg_reg_ops_buf[5], "%x %x", &reg, &value);
> -		if (cnt == 2) {
> +		/* bounds-check register offset */
> +		if (cnt == 2 && reg <= IXGBE_HFDR) {
>  			IXGBE_WRITE_REG(&adapter->hw, reg, value);
>  			value = IXGBE_READ_REG(&adapter->hw, reg);
>  			e_dev_info("write: 0x%08x = 0x%08x\n", reg, value);
> @@ -97,7 +98,8 @@ static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
>  		u32 reg, value;
>  		int cnt;
>  		cnt = sscanf(&ixgbe_dbg_reg_ops_buf[4], "%x", &reg);
> -		if (cnt == 1) {
> +		/* bounds-check register offset */
> +		if (cnt == 1 && reg <= IXGBE_HFDR) {
>  			value = IXGBE_READ_REG(&adapter->hw, reg);
>  			e_dev_info("read 0x%08x = 0x%08x\n", reg, value);
>  		} else {
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 210c7b9..4a1f3c2 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -354,4 +354,6 @@ u32 ixgbe_read_reg(struct ixgbe_hw *hw, u32 reg)
>  	if (ixgbe_removed(reg_addr))
>  		return IXGBE_FAILED_READ_REG;
> +	if (WARN_ON_ONCE(reg > IXGBE_HFDR))
> +		return IXGBE_FAILED_READ_REG;
>  	if (unlikely(hw->phy.nw_mng_if_sel &
>  		     IXGBE_NW_MNG_IF_SEL_SGMII_ENABLE)) {
> -- 
> 2.52.0
>