Re: [PATCH v2 1/2] vhost: fix possible FDs leak

[Maxime Coquelin <maxime.coquelin@redhat.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