7:16 a.m.
Hi,
We ran into what looks like a bug in the fs recovery backend on V15.1. We
have a
one-line change that fixes it for us, and I'd like to check with you
whether it
is the right fix before we rely on it.
In fs_reclaim_complete() (src/SAL/recovery/recovery_fs.c) the marker name is
copied into an uninitialised path[PATH_MAX] without its terminator:
path[dstpos] = '/';
memcpy(path + dstpos + 1, reclaim_complete_marker, marker_len);
fd = creat(path, 0700);
so creat() picks up whatever followed on the stack. On our builds that is a
display_client_id_rec() dump from an earlier frame, giving entries like
v4recov/(27:Linux NFSv4.1 node-1)/reclaim_completenf = (nil) {NULL}
unconf = ...
fs_check_reclaim_complete() then looks for the exact name and misses it, so
check_clid() refuses CLAIM_PREVIOUS for every NFSv4.1 client: locks are
dropped
without the client noticing, and grace always runs to Grace_Period because
reclaim_completes never reaches the clid count. It reads as intermittent,
since
whether the trailing bytes happen to start with a NUL is luck.
Measured on three nodes, one fcntl(F_WRLCK) holder each, NFSv4.1 only:
across
three restarts the marker name predicted the outcome every time, and the
client-visible I/O pause was ~118s instead of ~34s.
What we are running is marker_len + 1 in that memcpy, so the terminator
comes
with it — the bounds check just above already reserves the byte. That gives
us
3/3 reclaims and grace lifting early.
Is that the correct fix in your view, or would you rather see it done
another way
— an snprintf here would match the reader side. Also happy to hear if you
think
we are misreading the cause.
Thanks,
Abhinandan
On Fri, 21 Aug 2026 at 21:16, Frank Filz <ffilzlnx(a)mindspring.com> wrote:
I think you’re good. And the implementation bits you are relying on
are
stable and won’t go away. You have a novel idea of using an export to hide
an internal use directory, but the idea of having different access
(including hiding) directories via exports is an intended feature of
Ganesha exports. Some filesystems implement sub-volumes in a way that even
improves the security to eliminate the risk of handle guessing.
Frank
*From:* Abhinandan Purkait [mailto:purkaitabhinandan@gmail.com]
*Sent:* Thursday, August 20, 2026 9:00 PM
*To:* Martin Schwenke <martin(a)meltin.net>
*Cc:* Frank Filz <ffilzlnx(a)mindspring.com>; devel(a)lists.nfs-ganesha.org
*Subject:* [NFS-Ganesha-Devel] Re: Guidance: keeping `RecoveryRoot`
inside an exported tree via a nested export with `Access_Type = None`
Thanks, both of you. I came in with four fairly narrow questions and got
much more than that out of it, which I appreciate.
Quick update on where we landed. We've moved off 6.5 onto 15.1 and dropped
the patch we were carrying. The config that replaces it turned out to be
RecoverySkipIp rather than RecoveryBackendIPBased — I had assumed those
were
two names for the same thing, and that was my own misreading rather than
anything either of you said. As far as I can tell SkipIp is what drops the
client address from the tag in nfs4_create_clid_name, while IPBased groups
the database into ip_<addr> directories keyed on cr_server_addr. Two
different halves, and I only worked that out after testing.
Martin, I did try to follow your lead on IPBased before I understood that,
so it may be worth mentioning why it didn't fit us — not a criticism of the
advice, which is clearly right for your setup. You've got a service IP that
clients follow, so whichever node takes the IP reads that IP's directory
and
everyone reclaims. We don't have a floating address at all: clients reach a
per-volume ClusterIP, and the server ends up recording its own pod IP as
cr_server_addr, which changes whenever the pod is rescheduled. So IPBased
would have sharded our database by the one address guaranteed to move.
We're
running it off with SkipIp on instead.
Otherwise we're staying on fs, and keeping the nested Access_Type = None
export for the recovery directory. Frank, your point about wrapping an
inode
in an export ID and FSID you do have access to was the useful correction
there — we've stopped treating that export as any kind of boundary and now
describe it as simply keeping the directory out of the way. And Martin,
thanks for coming back on fs_ng; the FIXME is exactly what I was after,
rather than avoiding it on reputation alone.
If any of that reading looks off to either of you, I'd be glad to hear it —
particularly the SkipIp/IPBased split, and whether fs plus a denied nested
export is a combination you'd expect to keep working. No rush, and no need
to reply if it all sounds reasonable. Otherwise I'll stop taking up list
time. Thanks again, genuinely helpful.
cheers,
Abhinandan
On Fri, 21 Aug 2026 at 08:22, Martin Schwenke <martin(a)meltin.net> wrote:
On Thu, 20 Aug 2026 17:57:44 -0700, "Frank Filz"
<ffilzlnx(a)mindspring.com> wrote:
> How does FSID_Override prevent handle guessing?
>
> You can take the inode portion of a handle, and wrap it in export ID and
> FSID that match an export you have access to. The open_by_handle_at
system
> call will then happily access the file or directory.
Good point! That is still possible...
To secure it properly, we would have to use a subdirectory mount of the
cluster filesystem per export. With Lustre we would then need the
distinct FSID on each export to allow NFS-Ganesha to find the
"filesystem" associated with the export to get the mount_fd to do
open_by_handle_at().
I know this will be secure because resolving a handle from a subsequent
subdirectory mount doesn't work (ESTALE) if we use the mount_fd for the
first subdirectory mount... because all the subdirectory mounts
have the same default FSID. That's the behaviour we see without
FSID_Override=True.
> Good to know about recovery_fs_ng. We should take a look at that code and
> figure out if it makes any sense and either make it usable or discard it.
Yeah, as per my other reply, it wasn't a very educated guess. I took a
fairly quick look and gave up early...
peace & happiness,
martin