From dd43a09a41c2e3072546a10f8483f811d78c02e7 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 3 Jul 2026 23:20:37 +0800 Subject: [PATCH] Do signal-aware waits and meta snapshot dnsfs_query_record no longer re-waits uninterruptibly after a signal wakes wait_for_completion_interruptible. A signaled foreground waiter now returns -ERESTARTSYS at once, drops its reference, and reads no req fields (the worker may still be writing them). Safe by the existing refcount contract: the worker holds its own reference until after complete_all, so the shared coalesced request stays queued, still resolves, fills the cache, and wakes the other waiters; the last reference drop frees it. Adds test_signal_interrupt: a 3s-delayed resolver, SIGINT a cat once the wire query is observed, expect it to die within 1s. dnsfs_storage_assemble_once and dnsfs_storage_read_range_once snapshot storage->meta and storage->epoch into locals before sizing buffers. The read path runs without storage->lock, so a concurrent re-pin could grow chunk_count or size between the allocation and the loop bound, a heap OOB write; the snapshot closes it. A torn generation is caught by epoch/CRC and retried. dnsfs_query_tcp loops kernel_sendmsg until the whole framed query is sent (TCP is a byte stream) and fails a zero-length send; dnsfs_query_udp_once requires the datagram send to report the ful length. --- src/dns.c | 61 ++++++++++++++++++++++++++++++++++++++----------- src/main.c | 30 +++++++++++++++++++++--- tests/driver.sh | 54 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 16 deletions(-) diff --git a/src/dns.c b/src/dns.c index 2b7e62b..8332af2 100644 --- a/src/dns.c +++ b/src/dns.c @@ -603,11 +603,29 @@ static int dnsfs_query_tcp(struct dnsfs_config *cfg, request[0] = query_len >> 8; request[1] = query_len; memcpy(request + 2, query, query_len); - vec.iov_base = request; - vec.iov_len = query_len + 2; - ret = kernel_sendmsg(sock, &msg, &vec, 1, vec.iov_len); - if (ret < 0) - goto out_sock; + /* TCP is a byte stream, so one kernel_sendmsg can send fewer bytes than the + * whole framed query. Loop until all of it goes out, and treat a + * zero-length send as a stuck connection rather than spinning. + */ + { + size_t total_sent = 0; + size_t to_send = query_len + 2; + + while (total_sent < to_send) { + struct msghdr send_msg = {.msg_name = NULL}; + + vec.iov_base = request + total_sent; + vec.iov_len = to_send - total_sent; + ret = kernel_sendmsg(sock, &send_msg, &vec, 1, vec.iov_len); + if (ret < 0) + goto out_sock; + if (ret == 0) { + ret = -EIO; + goto out_sock; + } + total_sent += ret; + } + } atomic64_inc(&dnsfs_wire_queries); vec.iov_base = reply; @@ -700,6 +718,13 @@ static int dnsfs_query_udp_once(struct dnsfs_config *cfg, send_vec.iov_len); if (ret < 0) break; + /* A UDP datagram sends whole or not at all; a short count means the + * query never left intact, so fail rather than wait for a reply to it. + */ + if (ret != send_vec.iov_len) { + ret = -EIO; + break; + } atomic64_inc(&dnsfs_wire_queries); ret = kernel_recvmsg(cfg->sock, &recv_msg, &recv_vec, 1, DNSFS_MAX_PACKET, 0); @@ -1214,15 +1239,25 @@ int dnsfs_query_record(struct dnsfs_config *cfg, return ret; wait: + /* A signal aborts the wait immediately instead of blocking out the whole + * query. The shared request stays queued, so the worker still resolves it, + * fills the cache, and wakes the other coalesced waiters; this caller just + * drops its reference and returns -ERESTARTSYS. Whoever drops the last + * reference frees the request, so leaving early is use-after-free safe. Do + * not read req->ret or req->text on the signal path: the worker may still + * be writing them, and only a completed request has them stable. + */ wait_ret = wait_for_completion_interruptible(&req->done); - if (wait_ret) - wait_for_completion(&req->done); - ret = wait_ret ? -ERESTARTSYS : req->ret; - if (ret > 0) { - if (req->len > out_len) - ret = -ENOSPC; - else - memcpy(out, req->text, req->len); + if (wait_ret) { + ret = -ERESTARTSYS; + } else { + ret = req->ret; + if (ret > 0) { + if (req->len > out_len) + ret = -ENOSPC; + else + memcpy(out, req->text, req->len); + } } if (refcount_dec_and_test(&req->refs)) kfree(req); diff --git a/src/main.c b/src/main.c index 53fc924..afb5a6d 100644 --- a/src/main.c +++ b/src/main.c @@ -606,7 +606,9 @@ static int dnsfs_storage_assemble_once(struct file *file, { struct dentry *dentry = file_dentry(file); struct dnsfs_config *cfg = dentry->d_sb->s_fs_info; - struct dnsfs_file_meta *meta = &storage->meta; + struct dnsfs_file_meta local_meta; + char local_epoch[DNSFS_MAX_LABEL + 1]; + struct dnsfs_file_meta *meta = &local_meta; struct dnsfs_decoded_chunk *chunks = NULL; char (*labels)[DNSFS_MAX_LABEL + 1] = NULL; u8 *chunk_data = NULL; @@ -615,6 +617,16 @@ static int dnsfs_storage_assemble_once(struct file *file, int ret; u32 i; + /* Snapshot meta + epoch into locals. The read path runs without + * storage->lock, so a concurrent re-pin must not grow chunk_count or size + * between the allocations and the loop bounds that use them. A torn + * generation is caught later by the epoch/CRC checks and retried, so it can + * never overrun the buffers sized here. + */ + local_meta = storage->meta; + memcpy(local_epoch, storage->epoch, sizeof(local_epoch)); + local_meta.epoch = local_epoch; + line = kmalloc(DNSFS_RECORD_TEXT_MAX, GFP_KERNEL); if (!line) { ret = -ENOMEM; @@ -728,7 +740,9 @@ static int dnsfs_storage_read_range_once(struct file *file, { struct dentry *dentry = file_dentry(file); struct dnsfs_config *cfg = dentry->d_sb->s_fs_info; - struct dnsfs_file_meta *meta = &storage->meta; + struct dnsfs_file_meta local_meta; + char local_epoch[DNSFS_MAX_LABEL + 1]; + struct dnsfs_file_meta *meta = &local_meta; u32 first = start / DNSFS_CHUNK_SIZE; u32 last = (start + out_len - 1) / DNSFS_CHUNK_SIZE; char label[DNSFS_MAX_LABEL + 1]; @@ -737,12 +751,22 @@ static int dnsfs_storage_read_range_once(struct file *file, char *line = NULL; int ret; u32 i; + bool verify_full; + + /* Snapshot meta + epoch into locals; see dnsfs_storage_assemble_once. The + * lockless read path must not let a concurrent re-pin change size or + * chunk_count under the allocations and bounds below. + */ + local_meta = storage->meta; + memcpy(local_epoch, storage->epoch, sizeof(local_epoch)); + local_meta.epoch = local_epoch; + /* A read covering the whole file also gets a whole-file CRC check; pull the * decoded bytes into one buffer so dnsfs_crc32c_verify can validate them. * This second buffer (TODO B6) is bounded by DNSFS_MAX_STORAGE_SIZE, capped * in dnsfs_storage_validate_meta, so it can never exceed that ceiling. */ - bool verify_full = (start == 0 && out_len >= meta->size); + verify_full = (start == 0 && out_len >= meta->size); if (!out_len) return 0; diff --git a/tests/driver.sh b/tests/driver.sh index f5b8a08..493b49e 100755 --- a/tests/driver.sh +++ b/tests/driver.sh @@ -30,6 +30,7 @@ dns_storage_port=$((23000 + ($$ % 5000))) dns_bad_index_port=$((22000 + ($$ % 5000))) dns_publisher_port=$((29000 + ($$ % 5000))) dns_multi_port=$((21000 + ($$ % 5000))) +dns_signal_port=$((46000 + ($$ % 3000))) dns_count=${TMPDIR:-/tmp}/dnsfs-count.$$ parallel_dir=${TMPDIR:-/tmp}/dnsfs-parallel.$$ storage_out=${TMPDIR:-/tmp}/dnsfs-storage.$$ @@ -724,6 +725,58 @@ PY stop_dns } +# A signal must abort a read blocked on a slow resolver promptly, not wait out +# the whole query (E1). The resolver delays every answer 3s; a cat is +# interrupted ~0.3s in and must die from the signal well before that, proving +# the foreground waiter no longer falls into an uninterruptible wait after the +# signal. +test_signal_interrupt() +{ + start_dns "$dns_signal_port" --delay-ms=3000 --count-file "$dns_count" + rm -f "$dns_count" + mount_dnsfs -o nameserver=127.0.0.1,port="$dns_signal_port",timeout=5000,retries=1 example.org + python3 - "$mnt/TXT" "$dns_count" <<'PY' +import pathlib +import signal +import subprocess +import sys +import time + +path = sys.argv[1] +count_file = pathlib.Path(sys.argv[2]) +proc = subprocess.Popen(["cat", path], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) +# The fixture bumps the count before its 3s delay, so count>=1 proves the worker +# already sent the wire query and cat is parked in the interruptible wait -- a +# fixed sleep could race and signal cat before it ever blocks in the kernel. +deadline = time.monotonic() + 2.0 +while time.monotonic() < deadline: + try: + if int(count_file.read_text().strip() or "0") >= 1: + break + except (FileNotFoundError, ValueError): + pass + time.sleep(0.01) +else: + proc.kill() + raise SystemExit("resolver never saw the query; cat never blocked") +start = time.monotonic() +proc.send_signal(signal.SIGINT) +try: + proc.wait(timeout=2.0) +except subprocess.TimeoutExpired: + proc.kill() + raise SystemExit("interrupted read did not return (blocked >2s on the query)") +elapsed = time.monotonic() - start +if elapsed > 1.0: + raise SystemExit(f"interrupted read returned slowly: {elapsed:.3f}s") +if proc.returncode != -signal.SIGINT: + raise SystemExit(f"cat exit {proc.returncode}, expected SIGINT termination") +PY + unmount_dnsfs + stop_dns +} + # A truncated UDP response forces a TCP retry (exactly two transport queries). test_tcp_fallback() { @@ -1411,6 +1464,7 @@ test_cache_eviction test_cache_reclaim test_positive_ttl test_async_ttl_refresh +test_signal_interrupt test_tcp_fallback test_multi_record test_storage