From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev@dpdk.org, chenbo.xia@intel.com, stable@dpdk.org
Subject: Re: [PATCH v2 1/2] vhost: fix possible FDs leak
Date: Mon, 30 Jan 2023 10:46:18 +0100 [thread overview]
Message-ID: <abe31dd0-7f0a-f159-0996-01046dcafd21@redhat.com> (raw)
In-Reply-To: <CAJFAV8zeLWxpnE7dYokjkYkGa-v84aLP8X__ckJKs=UrKEVhwA@mail.gmail.com>
On 1/29/23 10:25, David Marchand wrote:
> On Fri, Jan 27, 2023 at 5:55 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> On failure, read_vhost_message() only closed the message
>> FDs if the header size was unexpected, but there are other
>> cases where it is required. For exemple in the case the
>> payload size read from the header is greater than the
>> expected maximum payload size.
>>
>> This patch fixes this by closing all messages FDs in all
>> error cases.
>>
>> Fixes: bf472259dde6 ("vhost: fix possible denial of service by leaking FDs")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
>
> We mentionned offlist that the request type can be logged to help with debug.
> Do you intend to add this as a follow up patch?
>
>
Thinking about it, that's not that trivial because read_vhost_message()
is called for both master and slave channels, so we would need to
differentiate between both to print proper request name.
It is doable by comparing the file descriptor passed as parameter with
the slave one stored in struct virtio_net, but that's not super clean.
Any thoughts?
Maxime
next prev parent reply other threads:[~2023-01-30 9:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230127165540.37863-1-maxime.coquelin@redhat.com>
2023-01-27 16:55 ` Maxime Coquelin
2023-01-29 9:25 ` David Marchand
2023-01-30 9:46 ` Maxime Coquelin [this message]
2023-01-30 14:25 ` David Marchand
2023-02-07 16:18 ` Maxime Coquelin
2023-02-07 5:38 ` Xia, Chenbo
2023-01-27 16:55 ` [PATCH v2 2/2] vhost: fix possible FD leaks on MSG_TRUNC and MSG_CTRUNC Maxime Coquelin
2023-01-29 9:26 ` David Marchand
2023-01-27 16:55 ` [PATCH v2 2/2] vhost: fix possible FD leaks on truncation Maxime Coquelin
2023-02-07 5:38 ` Xia, Chenbo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abe31dd0-7f0a-f159-0996-01046dcafd21@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=chenbo.xia@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).