[ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

Ben Pfaff blp at ovn.org
Wed Jun 3 05:26:28 UTC 2020


On Wed, Jun 03, 2020 at 01:22:52AM +0000, Linhaifeng wrote:
> 
> 
> -----Original Message-----
> From: Ben Pfaff [mailto:blp at ovn.org] 
> Sent: Wednesday, June 3, 2020 1:28 AM
> To: Linhaifeng <haifeng.lin at huawei.com>
> Cc: Yanqin Wei <Yanqin.Wei at arm.com>; dev at openvswitch.org; nd <nd at arm.com>; Lilijun (Jerry) <jerry.lilijun at huawei.com>; chenchanghu <chenchanghu at huawei.com>; Lichunhe <lichunhe at huawei.com>
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> On Tue, Jun 02, 2020 at 07:27:59AM +0000, Linhaifeng wrote:
> > We should update rcu pointer first then use ovsrcu_postpone to free 
> > otherwise maybe cause use-after-free.
> > e.g.,reader indicates momentary quiescent and access old pointer after 
> > writer postpone free old pointer and before setting new pointer.
> > 
> > Signed-off-by: Linhaifeng <haifeng.lin at huawei.com>
> 
> I don't see how that's possible, since the writer hasn't quiesced.
> 
> I think the logic is as follow, Could you help me find out where is incorrect?
> 
> 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4 -> 3.3 -> 2.2(use after free)
> 
> wirter:
> 1.1 use postone to free old pointer
> 1.2 flush cbsets to flushed_cbsets
> 1.3 update new pointer
> 1.4 quiesced
> 
> Read:
> 2.1. read pointer
> 2.2. use pointer
> 2.3. quiesced
> 
> Rcu:
> 3.1 pop flushed_cbsets
> 3.2 ovsrcu_synchronize
> 3.3 call all cb to free

So you're saying this:

    1.1 use postone to free old pointer (A)
    1.2 flush cbsets to flushed_cbsets

    3.1 pop flushed_cbsets
    3.2 ovsrcu_synchronize

    2.1. read pointer (A)
    2.2. use pointer (A)
    2.3. quiesced

    2.1. read pointer (A)

    1.3 update new pointer (B)
    1.4 quiesced

    3.3 call all cb to free (A)

    2.2. use pointer (A)

Wow, you are absolutely right.  This had never occurred to me.  Thank
you!  I'll review your patch.



More information about the dev mailing list