xfs: split freemap from xchk_xattr_buf.buf

Move the free space bitmap from somewhere in xchk_xattr_buf.buf[] to an
explicit pointer.  This is the start of removing the complex overloaded
memory buffer that is the source of weird memory misuse bugs.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 45fa8a5967deff07a7038dfbed25662f8ee1b54e..bc529b53a84fb3b1778ee18fbf10a307296ab462 100644 (file)
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -20,6 +20,17 @@
 #include "scrub/dabtree.h"
 #include "scrub/attr.h"
 
+/* Free the buffers linked from the xattr buffer. */
+static void
+xchk_xattr_buf_cleanup(
+       void                    *priv)
+{
+       struct xchk_xattr_buf   *ab = priv;
+
+       kvfree(ab->freemap);
+       ab->freemap = NULL;
+}
+
 /*
  * Allocate enough memory to hold an attr value and attr block bitmaps,
  * reallocating the buffer if necessary.  Buffer contents are not preserved
@@ -32,15 +43,18 @@ xchk_setup_xattr_buf(
        gfp_t                   flags)
 {
        size_t                  sz;
+       size_t                  bmp_sz;
        struct xchk_xattr_buf   *ab = sc->buf;
+       unsigned long           *old_freemap = NULL;
+
+       bmp_sz = sizeof(long) * BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
 
        /*
         * We need enough space to read an xattr value from the file or enough
-        * space to hold two copies of the xattr free space bitmap.  We don't
+        * space to hold one copy of the xattr free space bitmap.  We don't
         * need the buffer space for both purposes at the same time.
         */
-       sz = 2 * sizeof(long) * BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
-       sz = max_t(size_t, sz, value_size);
+       sz = max_t(size_t, bmp_sz, value_size);
 
        /*
         * If there's already a buffer, figure out if we need to reallocate it
@@ -49,6 +63,7 @@ xchk_setup_xattr_buf(
        if (ab) {
                if (sz <= ab->sz)
                        return 0;
+               old_freemap = ab->freemap;
                kvfree(ab);
                sc->buf = NULL;
        }
@@ -60,9 +75,18 @@ xchk_setup_xattr_buf(
        ab = kvmalloc(sizeof(*ab) + sz, flags);
        if (!ab)
                return -ENOMEM;
-
        ab->sz = sz;
        sc->buf = ab;
+       sc->buf_cleanup = xchk_xattr_buf_cleanup;
+
+       if (old_freemap) {
+               ab->freemap = old_freemap;
+       } else {
+               ab->freemap = kvmalloc(bmp_sz, flags);
+               if (!ab->freemap)
+                       return -ENOMEM;
+       }
+
        return 0;
 }
 
@@ -222,21 +246,21 @@ xchk_xattr_check_freemap(
        unsigned long                   *map,
        struct xfs_attr3_icleaf_hdr     *leafhdr)
 {
-       unsigned long                   *freemap = xchk_xattr_freemap(sc);
+       struct xchk_xattr_buf           *ab = sc->buf;
        unsigned int                    mapsize = sc->mp->m_attr_geo->blksize;
        int                             i;
 
        /* Construct bitmap of freemap contents. */
-       bitmap_zero(freemap, mapsize);
+       bitmap_zero(ab->freemap, mapsize);
        for (i = 0; i < XFS_ATTR_LEAF_MAPSIZE; i++) {
-               if (!xchk_xattr_set_map(sc, freemap,
+               if (!xchk_xattr_set_map(sc, ab->freemap,
                                leafhdr->freemap[i].base,
                                leafhdr->freemap[i].size))
                        return false;
        }
 
        /* Look for bits that are set in freemap and are marked in use. */
-       return !bitmap_intersects(freemap, map, mapsize);
+       return !bitmap_intersects(ab->freemap, map, mapsize);
 }
 
 /*

diff --git a/fs/xfs/scrub/attr.h b/fs/xfs/scrub/attr.h
index daf354a581bb457a13635f35b0cfb31f628a24c5..341855b3201bcb2e0787fc3676beb8b1127f5319 100644 (file)
--- a/fs/xfs/scrub/attr.h
+++ b/fs/xfs/scrub/attr.h
@@ -10,6 +10,9 @@
  * Temporary storage for online scrub and repair of extended attributes.
  */
 struct xchk_xattr_buf {
+       /* Bitmap of free space in xattr leaf blocks. */
+       unsigned long           *freemap;
+
        /* Size of @buf, in bytes. */
        size_t                  sz;
 
@@ -20,8 +23,7 @@ struct xchk_xattr_buf {
         *
         * Each bitmap contains enough bits to track every byte in an attr
         * block (rounded up to the size of an unsigned long).  The attr block
-        * used space bitmap starts at the beginning of the buffer; the free
-        * space bitmap follows immediately after.
+        * used space bitmap starts at the beginning of the buffer.
         */
        uint8_t                 buf[];
 };
@@ -46,13 +48,4 @@ xchk_xattr_usedmap(
        return (unsigned long *)ab->buf;
 }
 
-/* A bitmap of free space computed by walking attr leaf block free info. */
-static inline unsigned long *
-xchk_xattr_freemap(
-       struct xfs_scrub        *sc)
-{
-       return xchk_xattr_usedmap(sc) +
-                       BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
-}
-
 #endif /* __XFS_SCRUB_ATTR_H__ */

diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 03ec455318f4dbf8c1287202ad3699764edec2ec..02819bedc5b1589e9bc94d4c85d7806f871455c0 100644 (file)
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -189,7 +189,10 @@ xchk_teardown(
        if (sc->flags & XCHK_REAPING_DISABLED)
                xchk_start_reaping(sc);
        if (sc->buf) {
+               if (sc->buf_cleanup)
+                       sc->buf_cleanup(sc->buf);
                kvfree(sc->buf);
+               sc->buf_cleanup = NULL;
                sc->buf = NULL;
        }
 

diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
index c519927355fe3a4bda18c5a1a0731e5cecb6d676..e71903474cd794cf90541103926258399bd3feb3 100644 (file)
--- a/fs/xfs/scrub/scrub.h
+++ b/fs/xfs/scrub/scrub.h
@@ -77,7 +77,17 @@ struct xfs_scrub {
         */
        struct xfs_inode                *ip;
 
+       /* Kernel memory buffer used by scrubbers; freed at teardown. */
        void                            *buf;
+
+       /*
+        * Clean up resources owned by whatever is in the buffer.  Cleanup can
+        * be deferred with this hook as a means for scrub functions to pass
+        * data to repair functions.  This function must not free the buffer
+        * itself.
+        */
+       void                            (*buf_cleanup)(void *buf);
+
        uint                            ilock_flags;
 
        /* See the XCHK/XREP state flags below. */