Re: [PATCH net-next v1] net: devmem: document that bind-tx is unprivileged by design

[Mina Almasry]

On Tue, Sep 22, 2026 at 3:56 AM <netdev-bot+sashiko@xxxxxxxxxx> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 2 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] NETDEV_CMD_BIND_TX is unprivileged (net/core/netdev-genl-
>   gen.c:240-245: .flags = GENL_CMD_CAP_DO), and this patch cements that…
> - [Medium] The comment added above netdev_nl_bind_tx_doit()
>   (net/core/netdev-genl.c:1183-1188) and the mirrored note in…
> - [Medium] bind-tx is reachable by any unprivileged user
>   (net/core/netdev-genl-gen.c:244: .flags = GENL_CMD_CAP_DO) with no…
>
> --
>
> net: devmem: document that bind-tx is unprivileged by design
>
> This adds a comment above netdev_nl_bind_tx_doit() and a matching note in
> Documentation/netlink/specs/netdev.yaml stating that NETDEV_CMD_BIND_TX
> carries no GENL_ADMIN_PERM / GENL_UNS_ADMIN_PERM on purpose, on the grounds
> that bind-tx only DMA-maps the caller's dmabuf.
>
> A few questions about the rationale being recorded here.
>
> > diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> > index e13461cf3fc33..4303593e5003c 100644
> > --- a/Documentation/netlink/specs/netdev.yaml
> > +++ b/Documentation/netlink/specs/netdev.yaml
> > @@ -852,6 +852,8 @@ operations:
> >        name: bind-tx
> >        doc: Bind dmabuf to netdev for TX
> >        attribute-set: dmabuf
> > +      # Intentionally unprivileged (no admin-perm / uns-admin-perm); see
> > +      # comment above netdev_nl_bind_tx_doit().
> >        do:
> >          request:
> >            attributes:
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index fa9edfdb32c29..690c27f63d260 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -1180,6 +1180,12 @@ netdev_find_netmem_tx_dev(struct net_device *dev)
> >       return NULL;
> >  }
> >
> > +/* Note: NETDEV_CMD_BIND_TX is intentionally unprivileged (no
> > + * GENL_ADMIN_PERM / GENL_UNS_ADMIN_PERM). Unlike bind-rx, which configures
> > + * shared NIC RX queues, bind-tx only DMA-maps the caller's dmabuf so they can
> > + * transmit from it on their own sockets without affecting other traffic or
> > + * device state.
> > + */
>
> [Severity: High]
> Is the "on their own sockets" part of this rationale actually enforced
> anywhere?
>
> net_devmem_bind_dmabuf() in net/core/devmem.c allocates the id out of a
> single global xarray and only records the owning netlink socket for cleanup:
>
>         err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id,
>                               binding, xa_limit_32b, &id_alloc_next,
>                               GFP_KERNEL);
>         ...
>         list_add(&binding->list, &priv->bindings);
>
> priv->bindings is then only walked by netdev_nl_sock_priv_destroy().
>
> On the TX side, net_devmem_get_binding() resolves whatever u32 the sender
> passed and checks only that a tx_vec exists and that the route egresses the
> bound device:
>
>         binding = net_devmem_lookup_dmabuf(dmabuf_id);
>         if (!binding || !binding->tx_vec) {
>                 err = -EINVAL;
>                 goto out_err;
>         }
>         ...
>         dst_dev = dst_dev_rcu(dst);
>         if (unlikely(!dst_dev) ||
>             unlikely(dst_dev != READ_ONCE(binding->dev) &&
>                      dst_dev != READ_ONCE(binding->vdev))) {
>                 err = -ENODEV;
>
> and net_devmem_lookup_dmabuf() is a plain xa_load() with no credential,
> netns or creator comparison.
>
> So can an unrelated unprivileged process reach another user's binding like
> this?
>
>   process A: NETDEV_CMD_BIND_TX -> gets id N, keeps the netlink socket open
>   process B: sendmsg(MSG_ZEROCOPY) with SCM_DEVMEM_DMABUF carrying id N
>
> __sock_cmsg_send() in net/core/sock.c takes the bare id:
>
>         case SCM_DEVMEM_DMABUF:
>                 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
>                         return -EINVAL;
>                 sockc->dmabuf_id = *(u32 *)CMSG_DATA(cmsg);
>
> tcp_sendmsg_locked() then calls net_devmem_get_binding(sk, sockc.dmabuf_id)
> and zerocopy_fill_skb_from_devmem() builds frags from the resulting niovs at
> attacker-chosen offsets. If B's route egresses the same netdev, does this
> let B transmit the contents of A's dmabuf to a peer of B's choosing?
>
> Ids are allocated cyclically starting low, and the return codes differ
> (-EINVAL for an unknown or non-TX id, -ENODEV for the wrong device), so is
> guessing a live id cheap? Should the TX path check the binding against its
> creating netdev_nl_sock before this is documented as safe, or should the
> command grow a permission flag instead?
>

This is a pre-existing issue that we can tighten up in the future. It
technically may be possible that 1 socket guesses the binding id of
another socket.

> [Severity: Medium]
> Separately from the enforcement question above, is the wording here wider
> than what the code does?
>
> Two parts look overstated:
>
> "so they can transmit from it on their own sockets" - nothing in
> net_devmem_get_binding() ties a binding to the socket, task, uid, netns or
> cgroup that created it; the u32 id from the global xarray is the only
> credential.
>
> "without affecting other traffic or device state" -
> netdev_find_netmem_tx_dev() can resolve the bind target to a different,
> shared physical NIC via an rx queue lease:
>
>                 phys_dev = lease_rxq->dev;
>                 if (netif_device_present(phys_dev) &&
>                     phys_dev->netmem_tx == NETMEM_TX_DMA)
>                         return phys_dev;
>
> and net_devmem_bind_dmabuf() then runs dma_buf_attach() and
> dma_buf_map_attachment_unlocked() against that physical device's dma_dev.
>
> Would it be more accurate to state the narrow property that bind-tx does
> not reconfigure RX queues or steering, rather than claiming no effect on
> other traffic or device state? The same wording is mirrored into the uAPI
> spec in Documentation/netlink/specs/netdev.yaml, so it becomes the answer
> future readers get when they ask why this command has no permission flag.
>

Ugh, commit message can be tighten up a bit but it doesn't matter. The
critical point is that bind-tx doesn't make the appliaction take
ownership of an rx queue, unlike bind-rx.

> [Severity: Medium]
> Does anything bound how many times an unprivileged caller can do this, or
> account the memory it consumes?
>
> net_devmem_bind_dmabuf() can be called repeatedly on the same dmabuf FD, and
> each call allocates with plain GFP_KERNEL / kvmalloc, so none of it is
> charged to the caller's memcg:
>
>                 binding->tx_vec = kvmalloc_objs(struct net_iov *,
>                                                 dmabuf->size / PAGE_SIZE);
>         ...
>                 owner->area.niovs = kvmalloc_objs(*owner->area.niovs,
>                                                   owner->area.num_niovs);
>
> and each call installs another mapping on the shared device:
>
>         binding->attachment = dma_buf_attach(binding->dmabuf, dma_dev);
>         ...
>         binding->sgt = dma_buf_map_attachment_unlocked(binding->attachment,
>                                                        direction);
>
> There is no duplicate-binding check, no count limit on entries added to
> priv->bindings, no __GFP_ACCOUNT, and no RLIMIT_MEMLOCK accounting for the
> long-lived mapping. With the rx-lease path in netdev_find_netmem_tx_dev()
> these mappings land on the physical NIC shared with other users.
>
> Given that, is "without affecting other traffic or device state" the right
> thing to document, or should a per-socket or per-user cap and memcg
> accounting come first?
>

I don't see an issue here. Binding one dmabuf to many netdevs should
be fine. Binding one dmabuf to the same netdev many times is possible
but pointless.


-- 
Thanks,
Mina