RE: [PATCH v2 1/1] vhost: tolerate file descriptor in REM_MEM_REG msg

"Bathija, Pravin" <[email protected]> Fri, 31 Jul 2026 08:19:07 +0000
Newsgroups org.dpdk.dev
Message-ID <IA0PPF659BD4C46C174C843A7FE6C0208C2F5C82@IA0PPF659BD4C46.namprd19.prod.outlook.com>


Internal Use - Confidential
> -----Original Message-----
> From: David Marchand <[email protected]>
> Sent: Friday, July 31, 2026 12:28 AM
> To: Bathija, Pravin <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/1] vhost: tolerate file descriptor i=
n REM_MEM_REG
> msg
>
>
> [EXTERNAL EMAIL]
>
> On Fri, 31 Jul 2026 at 09:08, Bathija, Pravin <Pravin.Bathija@=
-dell.com> wrote:
> > Internal Use - Confidential
>
> It is not.
>
> > > -----Original Message-----
> > > From: David Marchand <[email protected]
D4-
> > > Sent: Thursday, July 30, 2026 11:22 PM
> > > To: Bathija, Pravin <[email protected]>
Ow-
> > > [email protected]
> > > Cc: [email protected]; [email protected]=
ADs-
> > > [email protected]; [email protected];=
 [email protected]
> > > Subject: Re: [PATCH v2 1/1] vhost: tolerate file =
descriptor in
> > > REM_MEM_REG msg
> > >
> > >
> > > [EXTERNAL EMAIL]
> > >
> > > On Fri, 31 Jul 2026 at 05:14, <pravin.bathija@del=
l.com> wrote:
> > > >
> > > > From: Pravin M Bathija <pravin.bathija@dell=
.com>
> > > >
> > > > The vhost-user specification (vhost-user.rst) state=
s that no file
> > > > descriptors SHOULD be passed with VHOST_USER
8-REM_MEM_REG.
> > > However, it
> > > > also says: "For compatibility with existing inc=
orrect
> > > > implementations, the back-end MAY accept messages w=
ith one file
> > > > descriptor.  If a file descriptor is passed, the ba=
ck-end MUST
> > > > close it without using it otherwise."
> > > >
> > > > Some front-ends, notably libblkio, reuse the same m=
essage-building
> > > > helper for both ADD_MEM_REG and REM_MEM=
_REG and unconditionally
> > > attach
> > > > the mapping fd.  The previous implementation reject=
ed any
> > > > REM_MEM_REG carrying a file descriptor with=
 the error:
> > > >
> > > >         expect 0 FDs for request VHOST_USER_=
-REM_MEM_REG, received
> > > > 1
> > > >
> > > > This broke teardown and memory region hot-swap with=
 these front-ends.
> > > >
> > > > To reproduce, run any libblkio (v1.5.0) application=
 using the
> > > > virtio-blk-vhost-user driver against a DPDK vhost b=
ack-end.  The
> > > > connection is dropped during cleanup or whenever a =
memory region
> > > > is unmapped and remapped.
> > >
> > > Is libblkio fixed now?
> > >
> > > I am not a fan of such compatibility fix, having to accep=
t one buggy client...
> > >
> >
> > Yes, the libblkio fix is ready and will be submitted upstream w=
ithin a day or so.
> > It stops sending the fd with REM_MEM_REG.
> >
> > That said, this DPDK fix stands on its own regardless of libblk=
io. The
> > vhost-user specification explicitly anticipates this situation =
and requires back-
> ends to handle it:
> >
> > "For compatibility with existing incorrect implementations,=
 the
> > back-end MAY accept messages with one file descriptor. If a fil=
e
> > descriptor is passed, the back-end MUST close it without using =
it otherwise."
>
> Well, yes, I understand the specification was updated or written for =
a buggy
> client :-)
>
Understood 😊. but the clause is in the spec nonetheless, and other i=
mplementations follow it.

>
> > While the immediate motivation was libblkio, the spec's compati=
bility
> > clause was written for exactly this situation — any front-e=
nd could
> > make the same mistake. QEMU's libvhost-user reference implement=
ation
> already tolerates it (see vu_rem_mem_reg).
>
> Which does not change that I dislike such compat.

I couldn't agree with you more.  The libblkio fix is ready and will stop se=
nding the fd.
In an ideal world we'd fix libblkio and move on,  but there are already sev=
eral released versions
of libblkio in the wild that exhibit this behavior.  Users pairing those wi=
th a current DPDK will hit
a broken connection with no obvious workaround.

>
>
> > The fix is 2 lines with no downside: accept the message, close =
the fd.
> > Rejecting it drops the connection entirely, which is a dispropo=
rtionate
> response to a harmless extra fd.
>
> Leaving behind a "harmless extra fd" causes exhaustion of a p=
rocess FD.
> At least, CVE-2019-14818 and CVE-2020-10726 come to mind.
>
> So strictly speaking, rejecting is really not disproportionate.

The patch does not leave the fd behind — close_msg_fds(ctx) clo=
ses it immediately. More
importantly, the fd is already received by the process once recvmsg() deliv=
ers it. The current
code rejects the message and drops the connection, but the fd has already b=
een transferred
into the process's fd table at that point. Without closing it explicitly, r=
ejecting actually
causes the leak those CVEs warn about. This patch prevents that.

>
>
> For now, drop the wrong RN update.
>

Thank you. Done in V3

>
> --
> David Marchand