[PATCH 04/11] xfs: split freemap from xchk_xattr_buf.buf

Darrick J. Wong
2022-12-30T22:49:19Z

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>
---
fs/xfs/scrub/attr.c | 40 ++++++++++++++++++++++++++++++++--------
fs/xfs/scrub/attr.h | 15 ++++-----------
fs/xfs/scrub/scrub.c | 3 +++
fs/xfs/scrub/scrub.h | 10 ++++++++++
4 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 6cd0ae99c2c5..fed159aba6e2 100644
--- 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 be133e0da71b..e6f11d44e84d 100644
--- 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 bc9638c7a379..6697f5f32106 100644
--- 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 20e74179d8a7..5d6e9a9527c3 100644
--- 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. */