Tom reported seeing some NFSv3 WRITE calls going to a mirror DS that
were using the MDS credentials instead of the ones that were provided
in the LAYOUTGET response.
The problem turns out to be an issue in how mirror creds were being
set up. Creds are associated with the mirror, not the DS, but we'd
skip trying to set up the credentials in the mirror when we found that
the DS was already connected.
While I was in there, I spotted a potential credential leak too and
fixed that as well.
Jeff Layton (2):
nfs: fix potential credential leak in ff_layout_update_mirror_cred
nfs: always update creds in mirror, even when we have an already
connected ds
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
--
2.4.3
A ds can be associated with more than one mirror, but we currently skip
setting a mirror's credentials if we find that it's already set up with
a connected client.
The upshot is that we can end up sending DS writes with MDS credentials
instead of properly setting them up. Fix nfs4_ff_layout_prepare_ds to
always verify that the mirror's credentials are set up, even when we
have a DS that's already connected.
Reported-by: Tom Haynes <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index c19b9a88f748..f13e1969eedd 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -387,7 +387,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx,
/* matching smp_wmb() in _nfs4_pnfs_v3/4_ds_connect */
smp_rmb();
if (ds->ds_clp)
- goto out;
+ goto out_update_creds;
flavor = nfs4_ff_layout_choose_authflavor(mirror);
@@ -431,7 +431,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx,
}
}
}
-
+out_update_creds:
if (ff_layout_update_mirror_cred(mirror, ds))
ds = NULL;
out:
--
2.4.3
If we have two tasks racing to update a mirror's credentials, then they
can end up leaking one (or more) sets of credentials. The first task
will set mirror->cred and then the second task will just overwrite it.
Use a cmpxchg to ensure that the creds are only set once. If we get to
the point where we would set mirror->cred and find that they're already
set, then we just release the creds that were just found.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 77a2d026aa12..c19b9a88f748 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -324,7 +324,8 @@ static int ff_layout_update_mirror_cred(struct nfs4_ff_layout_mirror *mirror,
__func__, PTR_ERR(cred));
return PTR_ERR(cred);
} else {
- mirror->cred = cred;
+ if (cmpxchg(&mirror->cred, NULL, cred))
+ put_rpccred(cred);
}
}
return 0;
--
2.4.3
On Thu, Jun 25, 2015 at 12:10 AM, Jeff Layton <[email protected]> wrote:
> Tom reported seeing some NFSv3 WRITE calls going to a mirror DS that
> were using the MDS credentials instead of the ones that were provided
> in the LAYOUTGET response.
>
> The problem turns out to be an issue in how mirror creds were being
> set up. Creds are associated with the mirror, not the DS, but we'd
> skip trying to set up the credentials in the mirror when we found that
> the DS was already connected.
>
> While I was in there, I spotted a potential credential leak too and
> fixed that as well.
>
> Jeff Layton (2):
> nfs: fix potential credential leak in ff_layout_update_mirror_cred
> nfs: always update creds in mirror, even when we have an already
> connected ds
Both looks good. Thanks! And we can mark them for stable IMO.
Cheers,
Tao
>
> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html