DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E74A420855
From:   Chris Perl <cperl@janestreet.com>
To:     linux-nfs@vger.kernel.org
Cc:     Chris Perl <cperl@janestreet.com>
Subject: [PATCH 0/1] Fix incorrect sharing of AUTH_SYS and AUTH_GSS_KRB5 data structures
Date:   Fri,  9 Nov 2018 04:02:50 -0500
Message-Id: <20181109090251.14145-1-cperl@janestreet.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

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