From 938ac092487a260b248e88c7c0640da8f18763e3 Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Wed, 17 Jun 2026 12:36:04 +0300 Subject: [PATCH] Fix small shared file extend-write potentially reading unallocated memory --- src/nfs/nfs_kv_write.cpp | 25 +++++++++++++++++++++++-- tests/test_nfs.sh | 10 ++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/nfs/nfs_kv_write.cpp b/src/nfs/nfs_kv_write.cpp index f0ff2d0d..274ee9a6 100644 --- a/src/nfs/nfs_kv_write.cpp +++ b/src/nfs/nfs_kv_write.cpp @@ -30,6 +30,7 @@ struct nfs_kv_write_state uint64_t new_size = 0; uint64_t aligned_size = 0; uint8_t *aligned_buf = NULL; + uint64_t aligned_buf_size = 0; int retry = 0; // new shared parameters uint64_t shared_inode = 0, shared_offset = 0, shared_alloc = 0; @@ -175,6 +176,7 @@ static void nfs_do_unshare_write(nfs_kv_write_state *st, int state) uint64_t aligned_size = align_up(size); nfs_do_write(st->ino, 0, aligned_size, [&](cluster_op_t *op) { + assert(size <= st->aligned_buf_size); op->iov.push_back(st->aligned_buf, size); if (aligned_size > size) op->iov.push_back(st->proxy->kvfs->zero_block.data(), aligned_size-size); @@ -282,6 +284,7 @@ static void nfs_do_shared_read(nfs_kv_write_state *st, int state) } assert(!st->aligned_buf); st->aligned_buf = (uint8_t*)malloc_or_die(data_size); + st->aligned_buf_size = data_size; uint64_t shared_offset = st->ientry["shared_offset"].uint64_value(); auto op = new cluster_op_t; op->opcode = OSD_OP_READ; @@ -294,6 +297,7 @@ static void nfs_do_shared_read(nfs_kv_write_state *st, int state) op->iov.push_back(st->proxy->kvfs->scrap_block.data(), pre); } op->iov.push_back(&st->shdr, sizeof(shared_file_header_t)); + assert(data_size <= st->aligned_buf_size); op->iov.push_back(st->aligned_buf, data_size); auto post = (shared_offset+sizeof(shared_file_header_t)+data_size); post = align_up(post) - post; @@ -418,7 +422,15 @@ static void nfs_do_shared_write(nfs_kv_write_state *st, int state) if (has_old) { // old data - op->iov.push_back(st->aligned_buf, st->offset); + assert(st->ientry["size"].uint64_value() == st->aligned_buf_size); + if (st->offset > st->ientry["size"].uint64_value()) + { + if (st->ientry["size"].uint64_value() > 0) + op->iov.push_back(st->aligned_buf, st->ientry["size"].uint64_value()); + add_zero(op, st->offset - st->ientry["size"].uint64_value(), st->proxy->kvfs->zero_block); + } + else + op->iov.push_back(st->aligned_buf, st->offset); } else add_zero(op, st->offset, st->proxy->kvfs->zero_block); @@ -433,7 +445,16 @@ static void nfs_do_shared_write(nfs_kv_write_state *st, int state) if (has_old) { // old data - op->iov.push_back(st->aligned_buf+st->offset+st->size, st->new_size-(st->offset+st->size)); + assert(st->ientry["size"].uint64_value() == st->aligned_buf_size); + if (st->new_size <= st->aligned_buf_size) + op->iov.push_back(st->aligned_buf+st->offset+st->size, st->new_size-(st->offset+st->size)); + else if (st->offset+st->size < st->aligned_buf_size) + { + op->iov.push_back(st->aligned_buf+st->offset+st->size, st->aligned_buf_size-(st->offset+st->size)); + add_zero(op, st->new_size - st->aligned_buf_size, st->proxy->kvfs->zero_block); + } + else + add_zero(op, st->new_size-(st->offset+st->size), st->proxy->kvfs->zero_block); } else add_zero(op, st->offset, st->proxy->kvfs->zero_block); diff --git a/tests/test_nfs.sh b/tests/test_nfs.sh index 665f85c0..42415419 100755 --- a/tests/test_nfs.sh +++ b/tests/test_nfs.sh @@ -17,6 +17,7 @@ trap "sudo umount -f $MNT"' || true; kill -9 $(jobs -p)' EXIT chown 1000:1000 ./testdata/nfs +# check readability by root touch ./testdata/nfs/f1 chown 1000:1000 ./testdata/nfs/f1 chmod 600 ./testdata/nfs/f1 @@ -195,4 +196,13 @@ sudo rm ./testdata/nfs/settings.jsonLGNmGn build/src/kv/vitastor-kv --config_path $VITASTOR_CFG fsmeta get d11/settings.jsonLGNmGn 2>&1 | grep '(code -2)' ls -l ./testdata/nfs +# check small shared file extend-write (it was reading unallocated memory but it's hard to actually make it differ from 0) +dd if=/dev/urandom of=./testdata/shared_beyond_ref bs=7k count=1 +dd if=/dev/urandom of=./testdata/shared_beyond_ref seek=8 bs=1k count=1 conv=notrunc +dd if=./testdata/shared_beyond_ref of=./testdata/nfs/shared_beyond_end bs=7k count=1 +dd if=./testdata/shared_beyond_ref of=./testdata/nfs/shared_beyond_end skip=8 seek=8 bs=1k count=1 conv=notrunc +cp ./testdata/nfs/shared_beyond_end ./testdata/ +diff ./testdata/shared_beyond_end ./testdata/shared_beyond_ref +rm ./testdata/nfs/shared_beyond_end + format_green OK