On 11/6/20 3:53 AM, Xia, Chenbo wrote:
> Hi Maxime,
>
>> -----Original Message-----
>> From: Maxime Coquelin <[email protected]>
>> Sent: Thursday, November 5, 2020 7:46 PM
>> To: [email protected]; Ding, Xuan <[email protected]>;
>> [email protected]; Yigit, Ferruh <[email protected]>;
>> [email protected]; Xia, Chenbo <[email protected]>
>> Cc: [email protected]; Maxime Coquelin <[email protected]>
>> Subject: [PATCH 2/3] vhost: fix fd leak in dirty logging setup
>>
>> This patch fixes a file descriptor leak which happens
>> in the error path of vhost_user_set_log_base().
>>
>> Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application")
>> Cc: [email protected]
>>
>> Reported-by: Xuan Ding <[email protected]>
>> Signed-off-by: Maxime Coquelin <[email protected]>
>> ---
>> lib/librte_vhost/vhost_user.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>> index 473fd778ca..7dfda15991 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -2076,14 +2076,14 @@ vhost_user_set_log_base(struct virtio_net **pdev,
>> struct VhostUserMsg *msg,
>>
>> if (fd < 0) {
>> VHOST_LOG_CONFIG(ERR, "invalid log fd: %d\n", fd);
>> - return RTE_VHOST_MSG_RESULT_ERR;
>> + goto close_msg_fds;
>
> IMHO, there's nothing to close in this case. Before this check,
> validate_msg_fds
> makes sure that this msg has only one fd and now this fd is invalid. So when
> this
> error occurs, going to close_msg_fds always does nothing. So I think we don't
> need
> this change here?
Thanks for the detailed review.
Yes, you are right, we can simply return here as before since no FD are
to close.
I will post v2 soon.
Thanks,
Maxime
> Thanks!
> Chenbo
