DPDK patches and discussions
 help / color / mirror / Atom feed
From: Gongming Chen <chengongming1900@outlook.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	chenbox@nvidia.com, chengongming1900@outlook.com
Cc: dev@dpdk.org, Gongming Chen <chengm11@chinatelecom.cn>, stable@dpdk.org
Subject: Re: [PATCH v1] vhost: fix crash caused by accessing a freed vsocket
Date: Fri, 10 May 2024 15:28:29 +0800	[thread overview]
Message-ID: <TYAP286MB064965C76BF9FDD86FA2727AD8E72@TYAP286MB0649.JPNP286.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <TYAP286MB06494783C3973058BF499642D83D2@TYAP286MB0649.JPNP286.PROD.OUTLOOK.COM>

Hi Maxime and Chenbo,

Do you have any suggestions for how to address this?

Looking forward to hearing from you!

Thanks,
Gongming

> On Apr 3, 2024, at 11:52 PM, Gongming Chen <chengongming1900@outlook.com> wrote:
> 
> Hi Maxime,
> Thanks for review.
> 
>> On Apr 3, 2024, at 5:39 PM, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>> 
>> Hi Gongming,
>> 
>> It's the 9th time the patch has been sent.
>> I'm not sure whether there are changes between them or these are just
>> re-sends, but that's something to avoid.
>> 
> 
> Sorry, there's something wrong with my mailbox.
> I will send a v1 version as the latest patch, but they are actually the same.
> 
>> If there are differences, you should use versionning to highlight it.
>> If unsure, please check the contributions guidelines first.
>> 
>> Regarding the patch itself, I don't know if this is avoidable, but I
>> would prefer we do not introduce yet another lock in there.
>> 
>> Thanks,
>> Maxime
>> 
> 
> I totally agree with your.
> Therefore, initially I hoped to solve this problem without introducing
> new lock. However, the result was not expected.
> 
> 1. The vsocket is shared between the event and reconnect threads by
> transmitting the vsocket pointer. Therefore, there is no way to protect
> vsocket through a simple vsocket lock.
> 
> 2. The event and reconnect threads can transmit vsocket pointers to
> each other, so there is no way to ensure that vsocket will not be
> accessed by locking the two threads separately.
> 
> 3. Therefore, on the vsocket resource, event and reconnect are in the
> same critical section. Only by locking two threads at the same time
> can the vsocket be ensured that it will not be accessed and can be
> freed safely.
> 
> Currently, app config, event, and reconnect threads respectively have
> locks corresponding to their own maintenance resources,
> vhost_user.mutex, pfdset->fd_mutex, and reconn_list.mutex.
> 
> I think there is a thread-level lock missing here to protect the
> critical section between threads, just like the rcu scene protection.
> 
> After app config acquires the write lock, it ensures that the event and
> reconnect threads are outside the critical section.
> This is to completely clean up the resources associated with vsocket
> and safely free vsocket.
> 
> Therefore, considering future expansion, if there may be more
> resources like vsocket, this thread lock can also be used to ensure
> that resources are safely released after complete cleanup.
> 
> In this way, the threads will be clearer, and the complicated try lock
> method is no longer needed.
> 
> Thanks,
> Gongming


  reply	other threads:[~2024-05-10  7:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03 16:05 Gongming Chen
2024-05-10  7:28 ` Gongming Chen [this message]
2024-07-02  7:48   ` Maxime Coquelin
2024-07-08 23:50     ` Gongming Chen
2024-07-08  2:17 ` [PATCH v2] " Gongming Chen
2024-07-08  2:23 ` [PATCH v3] " Gongming Chen
2024-07-08  4:41 ` [PATCH v4] " Gongming Chen
2024-07-09  7:26   ` David Marchand

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=TYAP286MB064965C76BF9FDD86FA2727AD8E72@TYAP286MB0649.JPNP286.PROD.OUTLOOK.COM \
    --to=chengongming1900@outlook.com \
    --cc=chenbox@nvidia.com \
    --cc=chengm11@chinatelecom.cn \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    --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).