[ovs-dev] [PATCH v1 0/8] RCU: Add blocking mode for debugging

Ben Pfaff blp at ovn.org
Wed May 5 23:19:00 UTC 2021


On Thu, May 06, 2021 at 12:37:36AM +0200, Gaëtan Rivet wrote:
> On Wed, May 5, 2021, at 21:36, Ben Pfaff wrote:
> > On Wed, Apr 28, 2021 at 01:03:24AM +0200, Gaetan Rivet wrote:
> > > This series adds a compilation option that changes the behavior of the RCU
> > > module. Once enabled, RCU reclamation by user threads becomes blocking until
> > > the RCU threads has executed the scheduled callbacks.
> > 
> > It's a great idea to improve the quality of the tests, and RCU can be a
> > sticking point.
> > 
> > I don't quite understand the description.  I think that's for one
> > particular reason: I don't understand yet what it causes to block.
> > Would you mind giving an example?  I haven't yet read the patches (I
> > might not; I don't plan to review this series in detail), so maybe
> > there is an example in one of the patch.  If so, then please just point
> > me to that one.
> 
> Hello Ben,
> 
> The feature unit-test in patch 4 gives an example:
> https://patchwork.ozlabs.org/project/openvswitch/patch/6de208dfd9d2fdea5999140c88632d42bdfe428b.1619564350.git.grive@u256.net/
> 
> In short:
> 
>     char *x = xmalloc(1);
>     x[0] = 'a';
>     ovsrcu_postpone(free, x);
>     ovsrcu_quiesce();
>     x[0] = 'b'; /* potential UAF. */
> 
> If the RCU is made blocking, the user thread will wait at the end of 'ovsrcu_quiesce()'
> for 'free(x)' to be executed. Only threads having scheduled callbacks would wait.
> This resolves the above race condition. As it executes deterministically, if ASAN is
> enabled as well it will detect the UAF.

This is a good example, thank you.  I think that it would be useful to
put this or a similar example into the documentation.

> In development, it could help verify that RCU is properly used.
> As it makes ASAN more effective, the RCU could be made blocking in conjunction in CI jobs.
> 
> > This message is a good introduction to the series.  Usually,
> > introductory messages don't make it into the repository, so I hope that
> > this material is also included in commit messages or in documentation or
> > comments.
> 
> Most of the information here is explained in patch 8 commit log:
> https://patchwork.ozlabs.org/project/openvswitch/patch/54575393370fa6076ba6ce7201f36bbfecbe9039.1619564350.git.grive@u256.net/
> 
> It also gives the configure option and how to test it.
> However, this feature is not otherwise documented.
> I could add a description in lib/ovs-rcu.h?

I think that would be a good place for it.  You could add a new
"Debugging" section at the end of the big comment, for example.

Thanks!


More information about the dev mailing list