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