Skip to content

Commit

Permalink
ZIO: Add overflow checks for linear buffers
Browse files Browse the repository at this point in the history
Since we use a limited set of kmem caches, quite often we have unused
memory after the end of the buffer.  Put there up to a 512-byte canary
when built with debug to detect buffer overflows at the free time.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15553
  • Loading branch information
amotin authored and mmatuska committed Dec 27, 2023
1 parent f9abc6d commit 650c4b0
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 2 deletions.
3 changes: 3 additions & 0 deletions lib/libspl/include/assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ libspl_assert(const char *buf, const char *file, const char *func, int line)
#undef verify
#endif

#define PANIC(fmt, a...) \
libspl_assertf(__FILE__, __FUNCTION__, __LINE__, fmt, ## a)

#define VERIFY(cond) \
(void) ((!(cond)) && \
libspl_assert(#cond, __FILE__, __FUNCTION__, __LINE__))
Expand Down
57 changes: 55 additions & 2 deletions module/zfs/zio.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,53 @@ zio_fini(void)
* ==========================================================================
*/

#ifdef ZFS_DEBUG
static const ulong_t zio_buf_canary = (ulong_t)0xdeadc0dedead210b;
#endif

/*
* Use empty space after the buffer to detect overflows.
*
* Since zio_init() creates kmem caches only for certain set of buffer sizes,
* allocations of different sizes may have some unused space after the data.
* Filling part of that space with a known pattern on allocation and checking
* it on free should allow us to detect some buffer overflows.
*/
static void
zio_buf_put_canary(ulong_t *p, size_t size, kmem_cache_t **cache, size_t c)
{
#ifdef ZFS_DEBUG
size_t off = P2ROUNDUP(size, sizeof (ulong_t));
ulong_t *canary = p + off / sizeof (ulong_t);
size_t asize = (c + 1) << SPA_MINBLOCKSHIFT;
if (c + 1 < SPA_MAXBLOCKSIZE >> SPA_MINBLOCKSHIFT &&
cache[c] == cache[c + 1])
asize = (c + 2) << SPA_MINBLOCKSHIFT;
for (; off < asize; canary++, off += sizeof (ulong_t))
*canary = zio_buf_canary;
#endif
}

static void
zio_buf_check_canary(ulong_t *p, size_t size, kmem_cache_t **cache, size_t c)
{
#ifdef ZFS_DEBUG
size_t off = P2ROUNDUP(size, sizeof (ulong_t));
ulong_t *canary = p + off / sizeof (ulong_t);
size_t asize = (c + 1) << SPA_MINBLOCKSHIFT;
if (c + 1 < SPA_MAXBLOCKSIZE >> SPA_MINBLOCKSHIFT &&
cache[c] == cache[c + 1])
asize = (c + 2) << SPA_MINBLOCKSHIFT;
for (; off < asize; canary++, off += sizeof (ulong_t)) {
if (unlikely(*canary != zio_buf_canary)) {
PANIC("ZIO buffer overflow %p (%zu) + %zu %#lx != %#lx",
p, size, (canary - p) * sizeof (ulong_t),
*canary, zio_buf_canary);
}
}
#endif
}

/*
* Use zio_buf_alloc to allocate ZFS metadata. This data will appear in a
* crashdump if the kernel panics, so use it judiciously. Obviously, it's
Expand All @@ -322,7 +369,9 @@ zio_buf_alloc(size_t size)
atomic_add_64(&zio_buf_cache_allocs[c], 1);
#endif

return (kmem_cache_alloc(zio_buf_cache[c], KM_PUSHPAGE));
void *p = kmem_cache_alloc(zio_buf_cache[c], KM_PUSHPAGE);
zio_buf_put_canary(p, size, zio_buf_cache, c);
return (p);
}

/*
Expand All @@ -338,7 +387,9 @@ zio_data_buf_alloc(size_t size)

VERIFY3U(c, <, SPA_MAXBLOCKSIZE >> SPA_MINBLOCKSHIFT);

return (kmem_cache_alloc(zio_data_buf_cache[c], KM_PUSHPAGE));
void *p = kmem_cache_alloc(zio_data_buf_cache[c], KM_PUSHPAGE);
zio_buf_put_canary(p, size, zio_data_buf_cache, c);
return (p);
}

void
Expand All @@ -351,6 +402,7 @@ zio_buf_free(void *buf, size_t size)
atomic_add_64(&zio_buf_cache_frees[c], 1);
#endif

zio_buf_check_canary(buf, size, zio_buf_cache, c);
kmem_cache_free(zio_buf_cache[c], buf);
}

Expand All @@ -361,6 +413,7 @@ zio_data_buf_free(void *buf, size_t size)

VERIFY3U(c, <, SPA_MAXBLOCKSIZE >> SPA_MINBLOCKSHIFT);

zio_buf_check_canary(buf, size, zio_data_buf_cache, c);
kmem_cache_free(zio_data_buf_cache[c], buf);
}

Expand Down

0 comments on commit 650c4b0

Please sign in to comment.