Fix race condition in BufferPage allocation function

BufferPage::claimEntry implements a lockfree algorithm to allocate
space for trace messages within a single page of memory. In performing
the bounds check for the allocation, a local copy of a member variable
is created, and a compare-and-swap strategy is used to update the
member variable with the new size of the allocated portion of the
buffer. However, in the bounds check, the member variable was
mistakenly used, rather than the local copy. This leads to a race
condition where if the member variable is updated in memory by another
thread between the time that its value is loaded from memory for the
bounds check and loaded again for the local variable, the bounds check
can succeed when it should fail and the allocation can go beyond the
bounds of the page owned by the BufferPage object.

This issue manifests only when the claimEntry function is compiled by
GCC (versions 4.9.2 and 8.2.0 tested) with size optimization (-Os)
because at this level, the compiler emits two load instructions for
the value of the member variable. Using -O3, only a single load
instruction is emitted and so the race condition is averted.

Change-Id: I89fbffee9806972bab410c4bc75022a33793b453
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/84284
Reviewed-by: Daniel M Crowell <dcrowell@us.ibm.com>
Reviewed-by: Glenn Miles <milesg@ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E Bofferding <bofferdn@us.ibm.com>

1 files changed, 2 insertions, 2 deletions

diff --git a/src/usr/trace/bufferpage.C b/src/usr/trace/bufferpage.C
index c6cfb57e0..27feb9bd7 100644
--- a/src/usr/trace/bufferpage.C
+++ b/src/usr/trace/bufferpage.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2012,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2012,2019                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -41,7 +41,7 @@ namespace TRACE
         uint64_t l_usedSize = this->usedSize;
 
         // Verify there is enough space.
-        while ((usedSize + i_size) < (PAGESIZE - sizeof(BufferPage)))
+        while ((l_usedSize + i_size) < (PAGESIZE - sizeof(BufferPage)))
         {
             // Atomically attempt to claim i_size worth.
             uint64_t newSize = l_usedSize + i_size;