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.gr...@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.gr...@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! _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
