Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <126c3133-2bca-4bb3-9cb2-7ca4fae2eda1@uipath.com>
Date:   Fri, 20 Oct 2023 00:12:48 +0300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] vsock: initialize the_virtio_vsock before using VQs
Content-Language: en-US
To:     Vadim Fedorenko <vadim.fedorenko@linux.dev>,
        Stefan Hajnoczi <stefanha@redhat.com>,
        Stefano Garzarella <sgarzare@redhat.com>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
Cc:     kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Mihai Petrisor <mihai.petrisor@uipath.com>,
        Viorel Canja <viorel.canja@uipath.com>
References: <20231018183247.1827-1-alexandru.matei@uipath.com>
 <f2d0aaad-70ca-4417-bf8e-0d7006be6ebc@linux.dev>
From:   Alexandru Matei <alexandru.matei@uipath.com>
In-Reply-To: <f2d0aaad-70ca-4417-bf8e-0d7006be6ebc@linux.dev>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OEVyUmE4S3Fzd2p5UVZpNitWR1Zhdk1TcXJUSUliQ1hwaUZHUWp0K3lPK3BM?=
 =?utf-8?B?R0cwdzBuU25Wdm5BeWRvRjNsNXo0WlowcXdrTmZBUUpUcDlvSG83MUt3S0ZD?=
 =?utf-8?B?c2NMOXgzbXZwN0dHaHRSVUdIdDdBeVF0MmwwWHpodldXdVNJWnJqSVQ3Q1Rz?=
 =?utf-8?B?WTRINHhsNTVYVWtNOU9oYUliRlZMRVlHR1RNS2NZQlJWQ3NGQXRlRE9HWEtE?=
 =?utf-8?B?citZdUFiSkc2THk1ekZJODZSOW9XbVRoLzA0TWE0aGdsOHVkaHFIS0Q1OE5h?=
 =?utf-8?B?U3dlSXlRU21PSTJCNUM3cDZMelJuTWgwRHVLdnA3L251UmVGd0cwZElYVDJ1?=
 =?utf-8?B?bTV5QlNvNWRCdEV4NzVoRU82ZU12SDNsSjFjTlV5dzd5dHJid2YxWTkrV0Zj?=
 =?utf-8?B?K3RZVVNoMkJFVCtJbWFhVWRoYzJieWRRNW8yR0RnTGhFczNvUWgweWp4MlEz?=
 =?utf-8?B?VnRGdnVuS0FyUGlEaFMwWXZ6OHlvQlowRW5SRHRpdUFUVVNwbjl1TUhxcHdq?=
 =?utf-8?B?VlZNSGpWeG5weWY5bHA4RUNZOUZhcnBBeTFUdUhOSERWVVZ4blh4LzA3cGVV?=
 =?utf-8?B?YWQvNzF5RkkwRllZSkwzejBwdGJUNjZvWHpzbC9Zc1g4YW5YeTJoOXRGT2lV?=
 =?utf-8?B?aGZOdDhQOVlyM25yUXpVOHE1NzJnTXBlZVNVdXUzekRBVGtKU0h3ZElKZlVv?=
 =?utf-8?B?NVprdWNOeEJrZ3hCVTlUZEhQWlZsdVF5SWczeHA5U3kwUTZOMDNVSWk2RUxH?=
 =?utf-8?B?Sy80Q3pXZTNYSGlkYzBINlFRb05nQTdHNUtyU0o5cGhZVzlua09kYU5OeEta?=
 =?utf-8?B?UWM4YzdnczJGTFl0aFVBcXNUWXVROGpsQ3VzSU02RXBrQTdYRndWdjg1Y1J3?=
 =?utf-8?B?ZGNqRXpkSGdtNFJ6aWpKd3lmSGdXNVZWV255OHRWMWVwWVpickg1ZzM5RUsw?=
 =?utf-8?B?WGVTa2VjUTNHQXZTMTc2YTE1K0QzR3llQTY4K3MwQ1hQejlFbm9MV0VvSERE?=
 =?utf-8?B?YU1vNDYrNVpIMW0vNnh6cEU0NCtUc1Z2SjQySjJscTZEWCtCQzFNeXFIMXh3?=
 =?utf-8?B?QS95cXpZNHlnYXdDdlNxUlhVOGJ2UXE1SUF2b0JuamswTkdtRzBIdytpaHZE?=
 =?utf-8?B?Yk9KMWdGdUFEYzJPWTN2Szh2b0VZeUlZK0doVEVDRGZGQ3FRN0M1SDlFZ200?=
 =?utf-8?B?UmtydG1WUDkxSk83UDI5Z2NpSXFRTGJEM01Za0l1dHM3aXhBemwwQnREcVM5?=
 =?utf-8?B?MlNJQ2haa205L2QyeHJUeGlPNlFsME16Z2VpRlVpNFlSMFp4OWt2NHFHK0Va?=
 =?utf-8?B?enJCc2RERjlNWkFVczVCOXZ4U1czNnpZUkFKdGhuWHAxN0IrSnVsUlpHTzRU?=
 =?utf-8?B?aEQrRnpwekxYYWsrcHJaaWZ5MjMyY0orV25DQnk0UEd1WlRic3hiS2ZNb1RR?=
 =?utf-8?B?OE13dkhVZmhudWVZZXFFK1p6eFdraFU0Y2VrV2pXYkd3UHZFTnhJVGRPUnVz?=
 =?utf-8?B?L3VTM28zeXFpNGZndzNDanRYQXlBUUlFL2x0dElRWmp3eU11dnBLdEovU3pS?=
 =?utf-8?B?a3didFcvaDNReVBNL216clA4OEVRSXpicW1KaG8zNFdkbUJCK1lHcnRrbWVn?=
 =?utf-8?B?YlovcC9FQVVQazhWQ2FpeWs5ajFDaTUrYlpQdHZET3JYMmN1bU5lQXpvaWs0?=
 =?utf-8?B?ZHZJaGMzc0ZENmxxbU5CNitlWDlBRTdnVWJLYVFEM3ZxNmk2YVljYjgycG01?=
 =?utf-8?B?cC9nbEVYYUVaWm9yR1NJUGZMU1JCUGhsdnZWRGhPV1U1L25Kb3Jtb01icDV6?=
 =?utf-8?B?S0hjM0FEZE4wZURva3NNUkdFQ3NQd1Q5eitUNytXWndDM1lvMm5QM1VWVTBo?=
 =?utf-8?B?WkZSdytOQ1owaTN6VFZXQ1ptQTBzaVE5RVlwempsSkU1ZzF3KzY1bzdZNFJT?=
 =?utf-8?B?MktmVE0wdnFubDdUb1N0dDAxODlaN25XYzZnRUVodFNHdUVZdC9jOHdFczlR?=
 =?utf-8?B?b1MyVFRUbzdwQjVHWlJtb2JNbFdEYlNaOTZPUEZQOGpzbzMvYXhFNDY0WW00?=
 =?utf-8?B?TStQNHdnQWMyS0k1Y0todklYUnlsVXV2MnR4VjM3a0ZwVWI0L3BncmhZcURz?=
 =?utf-8?B?Q2FOUWJJTWYrbFRJU1cwVlNwdk9JOXIzZ1JGczF3SmszcmZ4Q0tKcDlLVGdp?=
 =?utf-8?B?dEE9PQ==?=
X-OriginatorOrg: uipath.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c7f60476-e472-47cd-1bed-08dbd0e82696
X-MS-Exchange-CrossTenant-AuthSource: VI1PR02MB4527.eurprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2023 21:12:50.9266
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: d8353d2a-b153-4d17-8827-902c51f72357
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 44neU2rj/uUlFYaRi2x1bzJ/xVYHJqcTDU/GTag+RNJf3zRhS+ss6ie19Fx2BI4Jdqr2DR//3I0UDLgxMqXwL11+y2NNo+I5peDeGZkw+WE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU0PR02MB9467
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 19 Oct 2023 14:13:08 -0700 (PDT)

On 10/19/2023 3:12 AM, Vadim Fedorenko wrote:
> On 18/10/2023 19:32, Alexandru Matei wrote:
>> Once VQs are filled with empty buffers and we kick the host, it can send
>> connection requests. If 'the_virtio_vsock' is not initialized before,
>> replies are silently dropped and do not reach the host.
>>
>> Fixes: 0deab087b16a ("vsock/virtio: use RCU to avoid use-after-free on the_virtio_vsock")
>> Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>
>> ---
>>   net/vmw_vsock/virtio_transport.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>> index e95df847176b..eae0867133f8 100644
>> --- a/net/vmw_vsock/virtio_transport.c
>> +++ b/net/vmw_vsock/virtio_transport.c
>> @@ -658,12 +658,13 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
>>           vsock->seqpacket_allow = true;
>>         vdev->priv = vsock;
>> +    rcu_assign_pointer(the_virtio_vsock, vsock);
>>         ret = virtio_vsock_vqs_init(vsock);
>> -    if (ret < 0)
>> +    if (ret < 0) {
>> +        rcu_assign_pointer(the_virtio_vsock, NULL);
>>           goto out;
>> -
>> -    rcu_assign_pointer(the_virtio_vsock, vsock);
>> +    }
>>         mutex_unlock(&the_virtio_vsock_mutex);
>>   
> 
> Looks like virtio_vsock_restore() needs the same changes. But
> virtio_vsock_vqs_init() can fail only in virtio_find_vqs(). Maybe it can be split into 2 functions to avoid second rcu_assign_pointer() in case
> of error?
> 
Thanks, it definitely looks better and it solves a few other issues by splitting the function in two.