From fc151100098d2899b7aed99aa1bcfe27bf00d58d Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 21 Apr 2026 15:20:21 -0400 Subject: [PATCH] NFSD: Report whether fh_key was actually updated The nfsd_ctl_fh_key_set tracepoint was introduced to capture operator activity on the filehandle signing key. Earlier revisions logged the key bytes verbatim; the version that landed hashes the 16 key bytes through crc32_le and stores the result. CRC32 is a linear projection of its input rather than a one-way function, and truncating any hash of fixed-size secret material leaves the key recoverable under offline brute force when the threat model includes an attacker with access to the trace ring. The operational question the fingerprint was meant to answer is whether a NFSD_CMD_THREADS_SET call that carries an NFSD_A_SERVER_FH_KEY attribute actually replaced the active key or re-installed the value already in place. Answer that question directly: compare the incoming key bytes against the current nn->fh_key inside nfsd_nl_fh_key_set() and surface a single bit to the tracepoint. The event now prints "updated" when the stored key changed and "unmodified" otherwise. A first set that fails kmalloc reports "unmodified" because no key was installed. Reported-by: jaeyeong Fixes: 62346217fd72 ("NFSD: Add a key for signing filehandles") Cc: Benjamin Coddington Reviewed-by: Benjamin Coddington Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfsctl.c | 18 ++++++++++++++---- fs/nfsd/trace.h | 16 +++++++--------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 39e7012a60d8..04e3954d54bd 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1594,16 +1594,27 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb, static int nfsd_nl_fh_key_set(const struct nlattr *attr, struct nfsd_net *nn) { siphash_key_t *fh_key = nn->fh_key; + u64 k0, k1; + bool changed; + + k0 = get_unaligned_le64(nla_data(attr)); + k1 = get_unaligned_le64(nla_data(attr) + 8); if (!fh_key) { fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL); - if (!fh_key) + if (!fh_key) { + trace_nfsd_ctl_fh_key_set(false, -ENOMEM); return -ENOMEM; + } nn->fh_key = fh_key; + changed = true; + } else { + changed = fh_key->key[0] != k0 || fh_key->key[1] != k1; } - fh_key->key[0] = get_unaligned_le64(nla_data(attr)); - fh_key->key[1] = get_unaligned_le64(nla_data(attr) + 8); + fh_key->key[0] = k0; + fh_key->key[1] = k1; + trace_nfsd_ctl_fh_key_set(changed, 0); return 0; } @@ -1682,7 +1693,6 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info) attr = info->attrs[NFSD_A_SERVER_FH_KEY]; if (attr) { ret = nfsd_nl_fh_key_set(attr, nn); - trace_nfsd_ctl_fh_key_set((const char *)nn->fh_key, ret); if (ret) goto out_unlock; } diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h index 5ad38f50836d..b631a472222b 100644 --- a/fs/nfsd/trace.h +++ b/fs/nfsd/trace.h @@ -2243,23 +2243,21 @@ TRACE_EVENT(nfsd_end_grace, TRACE_EVENT(nfsd_ctl_fh_key_set, TP_PROTO( - const char *key, + bool changed, int result ), - TP_ARGS(key, result), + TP_ARGS(changed, result), TP_STRUCT__entry( - __field(u32, key_hash) + __field(bool, changed) __field(int, result) ), TP_fast_assign( - if (key) - __entry->key_hash = ~crc32_le(0xFFFFFFFF, key, 16); - else - __entry->key_hash = 0; + __entry->changed = changed; __entry->result = result; ), - TP_printk("key=0x%08x result=%d", - __entry->key_hash, __entry->result + TP_printk("key %s, result=%d", + __entry->changed ? "updated" : "unmodified", + __entry->result ) );