Re: [PATCH] ovl: fortify ovl_copy_xattr() against underlying layer modifications

TL;DR: your patch fixes it.  On v7.2-rc6, FUSE probe LEAK -> NO LEAK, and the
unprivileged tmpfs racer goes from 78 of 5,000,000 copy-ups leaking (5 of 5
boots) to 0 of 5,000,000 (0 of 5 boots).  51 of 51 functional xattr checks
unchanged.  One behaviour change worth a look: under a racing lower fs about
6% of copy-ups now fail with EIO.  Tested-by below.

Your three earlier points are taken; hence the shape of this mail.

Setup: v7.2-rc6 (075b74841bd0), your patch applied clean, both kernels built
from a byte-identical config so the patch is the only variable.  I checked it
reached the binary rather than trusting the build: ovl_copy_xattr shrinks from
0x504 to 0x443 bytes as the grow loop moves into the helper.

Results
-------

  test                              base            patched
  FUSE probe (hostile lower)        LEAK            NO LEAK
  unprivileged tmpfs racer          5/5 boots       0/5 boots
                                    78 of 5e6       0 of 5e6
  functional, 51 xattr checks       51 of 51        51 of 51
  termination probe                 0.100 s         0.101 s, EIO

Before, the upper file receives 6b6b..6ba5; after, the real value.

The EIO path works as designed: against a lower fs that shrinks the xattr
between the two getxattr calls, the copy-up now returns -EIO in 0.101 s
instead of copying out the unfilled buffer.

The behaviour change
--------------------

Under the racer, on 1,000,000 copy-ups per boot:

  base      ok=1000000  other=0
  patched   ok=939548   other=60452  (errno 5)

So ~6% of copy-ups now fail rather than silently copying wrong bytes.  I read
overlayfs.rst as saying that is exactly right -- modifying a lower layer of a
mounted overlay is undefined behaviour, and EIO is the defined detection --
but it is a real, unprivileged-triggerable failure rate for anything doing
copy-up over a changing lower fs, so I would rather you saw the number.

EAGAIN=0 on every patched boot.  Copy-up latency max 44-48 ms, nothing over
100 ms.  No BUG, WARNING, stall or oops attributable to the patch; the one
"Attempted to kill init!" trace in some logs is my harness shutting down and
appears on both kernels.

Two limits: these builds are CONFIG_SLUB_DEBUG_ON=y, so the leaked bytes are
poison -- proof the memory was never written, but no kernel pointers in this
run; those came from a separate non-debug build.  And the racer is a
size-flapping tmpfs lower, not a general workload.

Tested-by: Fahad Alharbi <[email protected]>

Reproducers and full logs on request.

Fahad