[PATCH V2 0/1] Fix incorrect sharing of AUTH_SYS and AUTH_GSS_KRB5 data structures

[PATCH V2 0/1] Fix incorrect sharing of AUTH_SYS and AUTH_GSS_KRB5 data structures

[Chris Perl]

I've marked this as V2, even though it is really the same exact patch
as last time.  The only change is the addition of a Signed-off-by in
the commit message.  Original email below.

Hi!

I explained most of the details in the commit, but the gist is that
mounts that don't have an explicit `sec' option passed will never get
through the first part of the if statement I removed, meaning a mount
which has its auth flavor discovered at mount time (e.g. in
`nfs_try_mount_request') cannot pass that test.

This means auth flavors are not compared and can lead to incorrect
sharing of data structures via `nfs_fs_mount_common'.

I am not the most familiar with all this code, so I might be missing
something about why that check is needed.  Please correct me if it
needs to remain.

In addition to testing this patch on a real system I tested something
almost equivalent to this patch by using systemtap to force
`b->auth_info.flavor_len' to 1 on every invocation if it was 0 (so the
if would always succeed) and back to 0 on exit.  Doing both of these
things caused my issues to go away and helped to reinforce my notion
that this was the right fix.

In case you're interested, that script is below.  Note that it hooks
onto `nfs_compare_super' and not `nfs_compare_mount_options' because
the latter is inlined.

Please copy me directly on any replies, as I'm not a member of the
list.

global revert;

probe module("nfs").function("nfs_compare_super").call
{
	server = @cast($data, "struct nfs_sb_mountdata")->server;
	if (server->auth_info->flavor_len == 0) {
	   server->auth_info->flavor_len = 1;
	   				 revert = 1;
					 }
}

probe module("nfs").function("nfs_compare_super").return
{
	server = @cast($data, "struct nfs_sb_mountdata")->server;
	if (revert) {
	   revert = 0;
	   	  server->auth_info->flavor_len = 0;
		  }
}

Chris Perl (1):
  NFS: nfs_compare_mount_options always compare auth flavors.

 fs/nfs/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

-- 
2.17.1

[PATCH V2 1/1] NFS: nfs_compare_mount_options always compare auth flavors.

[Chris Perl]

This patch removes the check from nfs_compare_mount_options to see if a
`sec' option was passed for the current mount before comparing auth
flavors and instead just always compares auth flavors.

Consider the following scenario:

You have a server with the address 192.168.1.1 and two exports /export/a
and /export/b.  The first export supports `sys' and `krb5' security, the
second just `sys'.

Assume you start with no mounts from the server.

The following results in EIOs being returned as the kernel nfs client
incorrectly thinks it can share the underlying `struct nfs_server's:

$ mkdir /tmp/{a,b}
$ sudo mount -t nfs -o vers=3,sec=krb5 192.168.1.1:/export/a /tmp/a
$ sudo mount -t nfs -o vers=3          192.168.1.1:/export/b /tmp/b
$ df >/dev/null
df: ‘/tmp/b’: Input/output error

Signed-off-by: Chris Perl <cperl@janestreet.com>
---
 fs/nfs/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ac4b2f005778..5ef2c71348bd 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2409,8 +2409,7 @@ static int nfs_compare_mount_options(const struct super_block *s, const struct n
 		goto Ebusy;
 	if (a->acdirmax != b->acdirmax)
 		goto Ebusy;
-	if (b->auth_info.flavor_len > 0 &&
-	   clnt_a->cl_auth->au_flavor != clnt_b->cl_auth->au_flavor)
+	if (clnt_a->cl_auth->au_flavor != clnt_b->cl_auth->au_flavor)
 		goto Ebusy;
 	return 1;
 Ebusy:
-- 
2.17.1