Thread (49 messages) 49 messages, 8 authors, 2026-01-30

Re: [PATCH v2 1/3] NFSD: Add a key for signing filehandles

From: Benjamin Coddington <hidden>
Date: 2026-01-22 15:17:35
Also in: linux-fsdevel, linux-nfs

On 22 Jan 2026, at 9:49, Chuck Lever wrote:
On Wed, Jan 21, 2026, at 8:22 PM, Benjamin Coddington wrote:
quoted
On 21 Jan 2026, at 18:55, Chuck Lever wrote:
quoted
On 1/21/26 5:56 PM, Benjamin Coddington wrote:
quoted
On 21 Jan 2026, at 17:17, Chuck Lever wrote:
quoted
On 1/21/26 3:54 PM, Benjamin Coddington wrote:
quoted
On 21 Jan 2026, at 15:43, Chuck Lever wrote:
quoted
On Wed, Jan 21, 2026, at 3:24 PM, Benjamin Coddington wrote:
quoted
A future patch will enable NFSD to sign filehandles by appending a Message
Authentication Code(MAC).  To do this, NFSD requires a secret 128-bit key
that can persist across reboots.  A persisted key allows the server to
accept filehandles after a restart.  Enable NFSD to be configured with this
key via both the netlink and nfsd filesystem interfaces.

Since key changes will break existing filehandles, the key can only be set
once.  After it has been set any attempts to set it will return -EEXIST.

Link:
https://lore.kernel.org/linux-nfs/cover.1769026777.git.bcodding@hammerspace.com (local)
Signed-off-by: Benjamin Coddington <redacted>
---
 Documentation/netlink/specs/nfsd.yaml |  6 ++
 fs/nfsd/netlink.c                     |  5 +-
 fs/nfsd/netns.h                       |  2 +
 fs/nfsd/nfsctl.c                      | 94 +++++++++++++++++++++++++++
 fs/nfsd/trace.h                       | 25 +++++++
 include/uapi/linux/nfsd_netlink.h     |  1 +
 6 files changed, 131 insertions(+), 2 deletions(-)
diff --git a/Documentation/netlink/specs/nfsd.yaml
b/Documentation/netlink/specs/nfsd.yaml
index badb2fe57c98..d348648033d9 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -81,6 +81,11 @@ attribute-sets:
       -
         name: min-threads
         type: u32
+      -
+        name: fh-key
+        type: binary
+        checks:
+            exact-len: 16
   -
     name: version
     attributes:
@@ -163,6 +168,7 @@ operations:
             - leasetime
             - scope
             - min-threads
+            - fh-key
     -
       name: threads-get
       doc: get the number of running threads
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 887525964451..81c943345d13 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -24,12 +24,13 @@ const struct nla_policy
nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1] = {
 };

 /* NFSD_CMD_THREADS_SET - do */
-static const struct nla_policy
nfsd_threads_set_nl_policy[NFSD_A_SERVER_MIN_THREADS + 1] = {
+static const struct nla_policy
nfsd_threads_set_nl_policy[NFSD_A_SERVER_FH_KEY + 1] = {
 	[NFSD_A_SERVER_THREADS] = { .type = NLA_U32, },
 	[NFSD_A_SERVER_GRACETIME] = { .type = NLA_U32, },
 	[NFSD_A_SERVER_LEASETIME] = { .type = NLA_U32, },
 	[NFSD_A_SERVER_SCOPE] = { .type = NLA_NUL_STRING, },
 	[NFSD_A_SERVER_MIN_THREADS] = { .type = NLA_U32, },
+	[NFSD_A_SERVER_FH_KEY] = NLA_POLICY_EXACT_LEN(16),
 };

 /* NFSD_CMD_VERSION_SET - do */
@@ -58,7 +59,7 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
 		.cmd		= NFSD_CMD_THREADS_SET,
 		.doit		= nfsd_nl_threads_set_doit,
 		.policy		= nfsd_threads_set_nl_policy,
-		.maxattr	= NFSD_A_SERVER_MIN_THREADS,
+		.maxattr	= NFSD_A_SERVER_FH_KEY,
 		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
 	},
 	{
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 9fa600602658..c8ed733240a0 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -16,6 +16,7 @@
 #include <linux/percpu-refcount.h>
 #include <linux/siphash.h>
 #include <linux/sunrpc/stats.h>
+#include <linux/siphash.h>

 /* Hash tables for nfs4_clientid state */
 #define CLIENT_HASH_BITS                 4
@@ -224,6 +225,7 @@ struct nfsd_net {
 	spinlock_t              local_clients_lock;
 	struct list_head	local_clients;
 #endif
+	siphash_key_t		*fh_key;
 };

 /* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 30caefb2522f..e59639efcf5c 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -49,6 +49,7 @@ enum {
 	NFSD_Ports,
 	NFSD_MaxBlkSize,
 	NFSD_MinThreads,
+	NFSD_Fh_Key,
 	NFSD_Filecache,
 	NFSD_Leasetime,
 	NFSD_Gracetime,
@@ -69,6 +70,7 @@ static ssize_t write_versions(struct file *file, char
*buf, size_t size);
 static ssize_t write_ports(struct file *file, char *buf, size_t size);
 static ssize_t write_maxblksize(struct file *file, char *buf, size_t
size);
 static ssize_t write_minthreads(struct file *file, char *buf, size_t
size);
+static ssize_t write_fh_key(struct file *file, char *buf, size_t size);
 #ifdef CONFIG_NFSD_V4
 static ssize_t write_leasetime(struct file *file, char *buf, size_t
size);
 static ssize_t write_gracetime(struct file *file, char *buf, size_t
size);
@@ -88,6 +90,7 @@ static ssize_t (*const write_op[])(struct file *,
char *, size_t) = {
 	[NFSD_Ports] = write_ports,
 	[NFSD_MaxBlkSize] = write_maxblksize,
 	[NFSD_MinThreads] = write_minthreads,
+	[NFSD_Fh_Key] = write_fh_key,
 #ifdef CONFIG_NFSD_V4
 	[NFSD_Leasetime] = write_leasetime,
 	[NFSD_Gracetime] = write_gracetime,
@@ -950,6 +953,60 @@ static ssize_t write_minthreads(struct file *file,
char *buf, size_t size)
 	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%u\n", minthreads);
 }

+/*
+ * write_fh_key - Set or report the current NFS filehandle key, the key
+ * 		can only be set once, else -EEXIST because changing the key
+ * 		will break existing filehandles.
Do you really need both a /proc/fs/nfsd API and a netlink API? I
think one or the other would be sufficient, unless you have
something else in mind (in which case, please elaborate in the
patch description).
Yes, some distros use one or the other.  Some try to use both!  Until you
guys deprecate one of the interfaces I think we're stuck expanding them
both.
Neil has said he wants to keep /proc/fs/nfsd rather indefinitely, and
we have publicly stated we will add only to netlink unless it's
unavoidable. I prefer not growing the legacy API.
Having both is more complete, and doesn't introduce any conflicts or
problems.
That doesn't tell me why you need it. It just says you want things to
be "tidy".

quoted
quoted
We generally don't backport new features like this one to stable
kernels, so IMO tucking this into only netlink is defensible.
Why only netlink for this one besides your preference?
You might be channeling one of your kids there.
That's unnecessary.
Is it? There's no point in asking that question other than as the
kind of jab a kid makes when trying to catch a parent in a
contradiction (which is exactly what you continue with below).
Wow, yes.  It's personal, and unprofessional. It has nothing to do with the
merits of the argument at hand.

I'm not trying to catch you in a contradiction Chuck.  You stated it was
your preference, and your reasons had (IMO) a valid counter-argument.
It doesn't make a difference whether it's my preference or not, and
frankly, as a contributor, it's not your role to decide whether an
interface goes in procfs or not.
It does make a difference, because this is community software.

I'm not trying to decide, I'm responding to your words and actions.  I was
having trouble resolving your desire to make sure the server is never
started w/o a key, while you also ask to remove an interface that can create
exactly that situation.
Don't argue with me.
Got it - I'm really sad to read this.
Just answer my questions. All I'm asking here is why are you adding it.
You got it, I'm trying to do that.  Let's take it down a notch.
quoted
quoted
As I stated before: we have said we don't want to continue adding
new APIs to procfs. It's not just NFSD that prefers this, it's a long
term project across the kernel. If you have a clear technical reason
that a new procfs API is needed, let's hear it.
You've just added one to your nfsd-testing branch two weeks ago that you
asked me to rebase onto.
Sorry for being human. Sometimes I don't notice things. That one doesn't
belong there either. But each one of these is decided on a case-by-case
basis. It's not appropriate for you to compare your procfs addition to
any other as a basis for "permission to add the API".
No need to apologize, it's just confusing.  I have a right to point it out.
Again, this is argumentative, not constructive. You're not answering
a direct question from a reviewer/maintainer. What is the reason you
need this API?
Sorry - I am making a best effort trying to do that.  When I see you write
"I prefer not growing the legacy API" right after you grew the legacy API it
creates dissonance, so I'm pointing it out.  Which is good apparently,
because now you'll have more of what you want.
quoted
quoted
quoted
There's a very good reason for both interfaces - there's been no work to
deprecate the old interface or co-ordination with distros to ensure they
have fully adopted the netlink interface.  Up until now new features have
been added to both interfaces.
I'm not seeing how this is a strong and specific argument for including
a procfs version of this specific interface. It's still saying "tidy" to
me and not explaining why we must have the extra clutter.

An example of a strong technical reason would be "We have legacy user
space applications that expect to find this API in procfs."
The systemd startup for the nfs-server in RHEL falls back to rpc.nfsd on
nfsdctl failure.  Without the additional interface you can have systems that
start the nfs-server via rpc.nfsd without setting the key - exactly the
situation you're so adamant should never happen in your below argument..
If your intention is to get distributions to backport signed FHs
to kernels that do not have the NFSD netlink interface, you need
to have stated that up front. That is the rationale I'm looking
for here, and I really should not have to work this hard getting
that answer from you.
I'm doing my best, I understand you're unhappy with me.
I might have deleted this while editing my reply yesterday, but
as a policy, LTS kernels do not get new features like this. So
it was never my intention to plan to target upstream backports
for this feature.

If this is part of your vision for this feature, you will need
to make a similar case to the stable@ folks.
We'll probably work with what we can from what you've decided you want.

As I've already let Jeff know - I'll remove that interface on the next
posting.

Ben
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help