Skip to content

Commit

Permalink
Upstream OSS fix for highmem violations by kmap
Browse files Browse the repository at this point in the history
During testing of the most recent grsec patchset utilizing strict
kmap checks, zfs_uiomove_bvec_impl was found to be capable of
kmap-ing more than PAGE_SIZE. Upstream x64 allows this in that it
does not crash outright, x86 likely doesn't.

This commit addresses the zfs_uiomove_bvec_impl condition, but
there may be other such violations such as zfs_copy_bvec directly
below in zfs_uio.c which may require a similar approach forcing
page-sized kmap calls within a loop.

Tests and a test-driven audit would be helpful to find all of
these conditions as they are somewhat arbitrarily triggered and
the same codepath can work correctly or crash being > PAGE_SIZE.

Credits:
  Open Source Security (Spender, probably Pipacs or Minipli too)

Testing:
  Internal VM tests with ZFS send/recv which used to hard-crash
the machine are now completing successfully.

Notes:
  The crash condition didn't occur in the first set of ztests
run during evaluation of the 6.6 patchset.
  • Loading branch information
RageLtMan committed Dec 14, 2023
1 parent 86e115e commit bdbf01f
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions module/os/linux/zfs/zfs_uio.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,16 @@ zfs_uiomove_bvec_impl(void *p, size_t n, zfs_uio_rw_t rw, zfs_uio_t *uio)

while (n && uio->uio_resid) {
void *paddr;
cnt = MIN(bv->bv_len - skip, n);
size_t offset = bv->bv_offset + skip;
cnt = MIN(PAGE_SIZE - (offset & ~PAGE_MASK), MIN(bv->bv_len - skip, n));

Check failure on line 138 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

line > 80 characters

paddr = zfs_kmap_atomic(bv->bv_page);
paddr = zfs_kmap_atomic(bv->bv_page + (offset >> PAGE_SHIFT));
if (rw == UIO_READ) {
/* Copy from buffer 'p' to the bvec data */
memcpy(paddr + bv->bv_offset + skip, p, cnt);
memcpy(paddr + (offset & ~PAGE_MASK), p, cnt);

Check failure on line 143 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

indent by spaces instead of tabs

Check failure on line 143 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

non-continuation indented 4 spaces
} else {
/* Copy from bvec data to buffer 'p' */
memcpy(p, paddr + bv->bv_offset + skip, cnt);
memcpy(p, paddr + (offset & ~PAGE_MASK), cnt);

Check failure on line 146 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

indent by spaces instead of tabs

Check failure on line 146 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

non-continuation indented 4 spaces
}
zfs_kunmap_atomic(paddr);

Expand Down Expand Up @@ -171,10 +172,10 @@ zfs_copy_bvec(void *p, size_t skip, size_t cnt, zfs_uio_rw_t rw,
paddr = zfs_kmap_atomic(bv->bv_page);
if (rw == UIO_READ) {
/* Copy from buffer 'p' to the bvec data */
memcpy(paddr + bv->bv_offset + skip, p, cnt);
memcpy(paddr + bv->bv_offset + skip, p, cnt);

Check failure on line 175 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

indent by spaces instead of tabs

Check failure on line 175 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

non-continuation indented 4 spaces
} else {
/* Copy from bvec data to buffer 'p' */
memcpy(p, paddr + bv->bv_offset + skip, cnt);
memcpy(p, paddr + bv->bv_offset + skip, cnt);

Check failure on line 178 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

indent by spaces instead of tabs

Check failure on line 178 in module/os/linux/zfs/zfs_uio.c

View workflow job for this annotation

GitHub Actions / checkstyle

non-continuation indented 4 spaces
}
zfs_kunmap_atomic(paddr);
}
Expand Down

0 comments on commit bdbf01f

Please sign in to comment.