[BUG] net: devmem: NETDEV_CMD_BIND_TX has no permission check and no accounting

[BUG] net: devmem: NETDEV_CMD_BIND_TX has no permission check and no accounting

[Dairui Zhang]

NETDEV_CMD_BIND_TX appears to be the only mutating devmem genl
command with no permission enforcement:

        BIND_RX        GENL_UNS_ADMIN_PERM
        QUEUE_CREATE   GENL_ADMIN_PERM
        BIND_TX        (just GENL_CMD_CAP_DO)

GENL_CMD_CAP_DO is only metadata - genetlink only enforces
GENL_ADMIN_PERM / GENL_UNS_ADMIN_PERM - and
netdev_nl_bind_tx_doit() (net/core/netdev-genl.c:1155) has no
capable() of its own either. As far as I can tell that means any
user can call it, on any netns; it's been like this since
8802087d20c0 and is still true in mainline today.

On its own that would just be an exposure question, but the bind
path also does no accounting at all:

  - no limit per socket or per user (the only bound is the 2^32
    xarray id space),
  - allocations use GFP_KERNEL, not GFP_KERNEL_ACCOUNT
    (devmem.c:207), so memcg can't throttle them,
  - nothing deduplicates repeated binds of the same dmabuf fd, each
    one doing a full dma_buf_attach + map_attachment plus a gen_pool
    and a per-page tx_vec,
  - everything is held until the netlink socket is closed.

So an unprivileged user with a dmabuf fd can loop BIND_TX on a
NETMEM_TX_DMA device (fbnic, gve, mlx5, bnxt) and eat kernel memory
and IOMMU mappings until the machine falls over. gve is the GCE
guest NIC, which makes "any user inside a GCE VM" the most obvious
victim scenario.

e302aa3d00fb ("net: devmem: allow bind-rx from non-init user
namespaces") revisited the bind-rx permission flags a few months ago
(GENL_ADMIN_PERM -> GENL_UNS_ADMIN_PERM, for the container use
cases) and left bind-tx alone, which is what makes me suspect an
oversight rather than a design choice. I could be missing some
context though, so asking before sending any patch.

Questions:

  1. Is bind-tx meant to be unprivileged (like AF_XDP TX), or is it
     just missing GENL_UNS_ADMIN_PERM like bind-rx?
  2. If unprivileged is the intent, is a per-socket cap plus
     GFP_KERNEL_ACCOUNT acceptable?
  3. Should repeated binds of the same dmabuf fd be deduplicated?

Happy to send the patch for whichever model you pick.

Thanks,
Dairui Zhang

Re: [BUG] net: devmem: NETDEV_CMD_BIND_TX has no permission check and no accounting

[Dairui Zhang]

Mina Almasry wrote:
> bind-tx is meant to be privileged.
> 
> I hope to add a comment in the code this week. We get this suggestion
> quite often now :D

Thanks for the quick answer. Want me to send the patch adding
GENL_UNS_ADMIN_PERM to NETDEV_CMD_BIND_TX (same level as bind-rx),
or would you rather fold it into your own change?

Separate from the permission bit: even a privileged caller can
exhaust memory right now, since binds aren't accounted (GFP_KERNEL,
devmem.c:207) and the same dmabuf fd can be bound over and over.
Is it worth adding GFP_KERNEL_ACCOUNT plus a per-binding cap or fd
dedup? Happy to send patches for either.

Thanks,
Dairui Zhang

Re: [BUG] net: devmem: NETDEV_CMD_BIND_TX has no permission check and no accounting

[Stanislav Fomichev]

On 09/21, Mina Almasry wrote:
> Sorry that was a typo. bind-tx is meant to be UNprivileged.  It's by
> design. We need to add a comment so people stop suggesting this.

I was gonna do it, but looks like you volunteered yourself? We do get this
almost every day now:

https://lore.kernel.org/netdev/20260918150858.3c98728d@kernel.org/

Re: [BUG] net: devmem: NETDEV_CMD_BIND_TX has no permission check and no accounting

[Dairui Zhang]

Stanislav Fomichev wrote:
> I was gonna do it, but looks like you volunteered yourself? We do
> get this almost every day now:
>
> https://lore.kernel.org/netdev/20260918150858.3c98728d@kernel.org/

Thanks for the pointer - I see the same flag patch was posted just
a few days ago. Sorry for adding to the pile, I should have
searched the list harder before sending.

That leaves the accounting side as my one remaining question. With
bind-tx unprivileged on purpose, is there anything that stops a
user from exhausting memory and IOMMU mappings by binding over and
over? As far as I can see there's no per-socket or per-user limit,
the allocations don't go to memcg (GFP_KERNEL, devmem.c:214), and
repeated binds of the same dmabuf fd aren't deduplicated - but I
may well be missing a guard somewhere.

If there isn't one, is that a gap you'd want a patch for, or is it
considered acceptable in practice? Happy to help either way, just
tell me which direction you'd prefer.

Thanks,
Dairui Zhang