Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZIO: Add overflow checks for linear buffers #15553

Merged
merged 1 commit into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -295,6 +295,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 @@ -311,7 +358,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 @@ -327,7 +376,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 @@ -340,6 +391,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 @@ -350,6 +402,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