| Message ID | 1481821992-77583-1-git-send-email-seth.forshee@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote: > Since 4.8 follow_automount() overrides the credentials with > &init_cred before calling d_automount(). When > rpcauth_lookupcred() is called in this context it is now using > fs[ug]id from the override creds instead of from the user's > creds, which can cause authentication to fail. To fix this, take > the ids from current_real_cred() instead. > > Cc: stable@vger.kernel.org # v4.8+ > CC: Eric W. Biederman <ebiederm@xmission.com> > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems > creds") > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > net/sunrpc/auth.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c > index 2bff63a73cf8..e6197b2bda86 100644 > --- a/net/sunrpc/auth.c > +++ b/net/sunrpc/auth.c > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int > flags) > { > struct auth_cred acred; > struct rpc_cred *ret; > - const struct cred *cred = current_cred(); > + const struct cred *cred = current_real_cred(); > > dprintk("RPC: looking up %s cred\n", > auth->au_ops->au_name); Among other things, this will break the access() syscall. It's completely the wrong level in which to override credentials. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
On Thu, Dec 15, 2016 at 11:01:41PM +0000, Trond Myklebust wrote: > On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote: > > Since 4.8 follow_automount() overrides the credentials with > > &init_cred before calling d_automount(). When > > rpcauth_lookupcred() is called in this context it is now using > > fs[ug]id from the override creds instead of from the user's > > creds, which can cause authentication to fail. To fix this, take > > the ids from current_real_cred() instead. > > > > Cc: stable@vger.kernel.org # v4.8+ > > CC: Eric W. Biederman <ebiederm@xmission.com> > > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems > > creds") > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > --- > > net/sunrpc/auth.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c > > index 2bff63a73cf8..e6197b2bda86 100644 > > --- a/net/sunrpc/auth.c > > +++ b/net/sunrpc/auth.c > > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int > > flags) > > { > > struct auth_cred acred; > > struct rpc_cred *ret; > > - const struct cred *cred = current_cred(); > > + const struct cred *cred = current_real_cred(); > > > > dprintk("RPC: looking up %s cred\n", > > auth->au_ops->au_name); > > Among other things, this will break the access() syscall. Okay, I see that now. > It's completely the wrong level in which to override credentials. The reason for it is that sget() now has a capability check which will fail on automount if current doesn't have CAP_SYS_ADMIN. So what are the alternatives? A few ideas: - Instead of using a completely differnet set of creds, we could copy the current creds and raise CAP_SYS_ADMIN. This won't work if curreent is in a different user ns however. - Filesystems could get around the capability check by using sget_userns() during automount. - We could add a mount flag, say MS_AUTOMOUNT, and skip the capability check if that is set. Any opinions or other ideas? Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 16, 2016 at 07:06:09AM -0600, Seth Forshee wrote: > On Thu, Dec 15, 2016 at 11:01:41PM +0000, Trond Myklebust wrote: > > On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote: > > > Since 4.8 follow_automount() overrides the credentials with > > > &init_cred before calling d_automount(). When > > > rpcauth_lookupcred() is called in this context it is now using > > > fs[ug]id from the override creds instead of from the user's > > > creds, which can cause authentication to fail. To fix this, take > > > the ids from current_real_cred() instead. > > > > > > Cc: stable@vger.kernel.org # v4.8+ > > > CC: Eric W. Biederman <ebiederm@xmission.com> > > > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems > > > creds") > > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > > --- > > > net/sunrpc/auth.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c > > > index 2bff63a73cf8..e6197b2bda86 100644 > > > --- a/net/sunrpc/auth.c > > > +++ b/net/sunrpc/auth.c > > > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int > > > flags) > > > { > > > struct auth_cred acred; > > > struct rpc_cred *ret; > > > - const struct cred *cred = current_cred(); > > > + const struct cred *cred = current_real_cred(); > > > > > > dprintk("RPC: looking up %s cred\n", > > > auth->au_ops->au_name); > > > > Among other things, this will break the access() syscall. > > Okay, I see that now. > > > It's completely the wrong level in which to override credentials. > > The reason for it is that sget() now has a capability check which will > fail on automount if current doesn't have CAP_SYS_ADMIN. So what are the > alternatives? A few ideas: > > - Instead of using a completely differnet set of creds, we could copy > the current creds and raise CAP_SYS_ADMIN. This won't work if > curreent is in a different user ns however. > > - Filesystems could get around the capability check by using > sget_userns() during automount. > > - We could add a mount flag, say MS_AUTOMOUNT, and skip the capability > check if that is set. > > Any opinions or other ideas? I haven't seen any responses, possibly just got lost in the shuffle during the holidays (I know it slipped my mind for a while). Eric, what do you think about the last option above? From what I can see looking up rpc credentials just isn't going to work with current_cred overridden as we're doing for automount. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Seth Forshee <seth.forshee@canonical.com> writes: > On Fri, Dec 16, 2016 at 07:06:09AM -0600, Seth Forshee wrote: >> On Thu, Dec 15, 2016 at 11:01:41PM +0000, Trond Myklebust wrote: >> > On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote: >> > > Since 4.8 follow_automount() overrides the credentials with >> > > &init_cred before calling d_automount(). When >> > > rpcauth_lookupcred() is called in this context it is now using >> > > fs[ug]id from the override creds instead of from the user's >> > > creds, which can cause authentication to fail. To fix this, take >> > > the ids from current_real_cred() instead. >> > > >> > > Cc: stable@vger.kernel.org # v4.8+ >> > > CC: Eric W. Biederman <ebiederm@xmission.com> >> > > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems >> > > creds") >> > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> >> > > --- >> > > net/sunrpc/auth.c | 2 +- >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> > > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c >> > > index 2bff63a73cf8..e6197b2bda86 100644 >> > > --- a/net/sunrpc/auth.c >> > > +++ b/net/sunrpc/auth.c >> > > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int >> > > flags) >> > > { >> > > struct auth_cred acred; >> > > struct rpc_cred *ret; >> > > - const struct cred *cred = current_cred(); >> > > + const struct cred *cred = current_real_cred(); >> > > >> > > dprintk("RPC: looking up %s cred\n", >> > > auth->au_ops->au_name); >> > >> > Among other things, this will break the access() syscall. >> >> Okay, I see that now. >> >> > It's completely the wrong level in which to override credentials. >> >> The reason for it is that sget() now has a capability check which will >> fail on automount if current doesn't have CAP_SYS_ADMIN. So what are the >> alternatives? A few ideas: >> >> - Instead of using a completely differnet set of creds, we could copy >> the current creds and raise CAP_SYS_ADMIN. This won't work if >> curreent is in a different user ns however. >> >> - Filesystems could get around the capability check by using >> sget_userns() during automount. >> >> - We could add a mount flag, say MS_AUTOMOUNT, and skip the capability >> check if that is set. >> >> Any opinions or other ideas? > > I haven't seen any responses, possibly just got lost in the shuffle > during the holidays (I know it slipped my mind for a while). > > Eric, what do you think about the last option above? From what I can see > looking up rpc credentials just isn't going to work with current_cred > overridden as we're doing for automount. I got as far as there wasn't a correct thing to apply, and I have been bogged down in enough other things that I haven't gotten back to this one. My gut feel is that we propbably want to do a little more reworking on the autmount path. But I don't exactly have a concrete proposal for you at the moment. I just found another 10 year old bug in the mount code... Eric -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 11, 2017 at 01:21:57PM +1300, Eric W. Biederman wrote: > Seth Forshee <seth.forshee@canonical.com> writes: > > > On Fri, Dec 16, 2016 at 07:06:09AM -0600, Seth Forshee wrote: > >> On Thu, Dec 15, 2016 at 11:01:41PM +0000, Trond Myklebust wrote: > >> > On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote: > >> > > Since 4.8 follow_automount() overrides the credentials with > >> > > &init_cred before calling d_automount(). When > >> > > rpcauth_lookupcred() is called in this context it is now using > >> > > fs[ug]id from the override creds instead of from the user's > >> > > creds, which can cause authentication to fail. To fix this, take > >> > > the ids from current_real_cred() instead. > >> > > > >> > > Cc: stable@vger.kernel.org # v4.8+ > >> > > CC: Eric W. Biederman <ebiederm@xmission.com> > >> > > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems > >> > > creds") > >> > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > >> > > --- > >> > > net/sunrpc/auth.c | 2 +- > >> > > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > > >> > > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c > >> > > index 2bff63a73cf8..e6197b2bda86 100644 > >> > > --- a/net/sunrpc/auth.c > >> > > +++ b/net/sunrpc/auth.c > >> > > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int > >> > > flags) > >> > > { > >> > > struct auth_cred acred; > >> > > struct rpc_cred *ret; > >> > > - const struct cred *cred = current_cred(); > >> > > + const struct cred *cred = current_real_cred(); > >> > > > >> > > dprintk("RPC: looking up %s cred\n", > >> > > auth->au_ops->au_name); > >> > > >> > Among other things, this will break the access() syscall. > >> > >> Okay, I see that now. > >> > >> > It's completely the wrong level in which to override credentials. > >> > >> The reason for it is that sget() now has a capability check which will > >> fail on automount if current doesn't have CAP_SYS_ADMIN. So what are the > >> alternatives? A few ideas: > >> > >> - Instead of using a completely differnet set of creds, we could copy > >> the current creds and raise CAP_SYS_ADMIN. This won't work if > >> curreent is in a different user ns however. > >> > >> - Filesystems could get around the capability check by using > >> sget_userns() during automount. > >> > >> - We could add a mount flag, say MS_AUTOMOUNT, and skip the capability > >> check if that is set. > >> > >> Any opinions or other ideas? > > > > I haven't seen any responses, possibly just got lost in the shuffle > > during the holidays (I know it slipped my mind for a while). > > > > Eric, what do you think about the last option above? From what I can see > > looking up rpc credentials just isn't going to work with current_cred > > overridden as we're doing for automount. > > I got as far as there wasn't a correct thing to apply, and I have been > bogged down in enough other things that I haven't gotten back to this > one. > > My gut feel is that we propbably want to do a little more reworking on > the autmount path. But I don't exactly have a concrete proposal for > you at the moment. I just found another 10 year old bug in the mount > code... With automounts we're mounting based on the credentials used when mounting the parent. That's what your patch "fs: Call d_automount with the filesystems creds" did, but it's overriding the credentials too early in the call stack and causing these rpcauth problems. But I think we should also be setting the submount's s_user_ns to be the same as the parent's, not to current_user_ns. At that point we'd be using the same credentials and user ns as when mounting the parent super block, so we know the capability check would pass (since it passed for the original mount) and don't really need to do it. So if we could pass down through the call stack that a given mount request is an automount from super block s (or at dentry d) then the fix would be trivial. I don't see any way of passing that information through currently though, without doing something undesirable like adding arguments to the mount filesystem op. Seth -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Seth Forshee <seth.forshee@canonical.com> writes: > On Wed, Jan 11, 2017 at 01:21:57PM +1300, Eric W. Biederman wrote: >> Seth Forshee <seth.forshee@canonical.com> writes: >> >> > On Fri, Dec 16, 2016 at 07:06:09AM -0600, Seth Forshee wrote: >> >> On Thu, Dec 15, 2016 at 11:01:41PM +0000, Trond Myklebust wrote: >> >> > On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote: >> >> > > Since 4.8 follow_automount() overrides the credentials with >> >> > > &init_cred before calling d_automount(). When >> >> > > rpcauth_lookupcred() is called in this context it is now using >> >> > > fs[ug]id from the override creds instead of from the user's >> >> > > creds, which can cause authentication to fail. To fix this, take >> >> > > the ids from current_real_cred() instead. >> >> > > >> >> > > Cc: stable@vger.kernel.org # v4.8+ >> >> > > CC: Eric W. Biederman <ebiederm@xmission.com> >> >> > > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems >> >> > > creds") >> >> > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> >> >> > > --- >> >> > > net/sunrpc/auth.c | 2 +- >> >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> > > >> >> > > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c >> >> > > index 2bff63a73cf8..e6197b2bda86 100644 >> >> > > --- a/net/sunrpc/auth.c >> >> > > +++ b/net/sunrpc/auth.c >> >> > > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int >> >> > > flags) >> >> > > { >> >> > > struct auth_cred acred; >> >> > > struct rpc_cred *ret; >> >> > > - const struct cred *cred = current_cred(); >> >> > > + const struct cred *cred = current_real_cred(); >> >> > > >> >> > > dprintk("RPC: looking up %s cred\n", >> >> > > auth->au_ops->au_name); >> >> > >> >> > Among other things, this will break the access() syscall. >> >> >> >> Okay, I see that now. >> >> >> >> > It's completely the wrong level in which to override credentials. >> >> >> >> The reason for it is that sget() now has a capability check which will >> >> fail on automount if current doesn't have CAP_SYS_ADMIN. So what are the >> >> alternatives? A few ideas: >> >> >> >> - Instead of using a completely differnet set of creds, we could copy >> >> the current creds and raise CAP_SYS_ADMIN. This won't work if >> >> curreent is in a different user ns however. >> >> >> >> - Filesystems could get around the capability check by using >> >> sget_userns() during automount. >> >> >> >> - We could add a mount flag, say MS_AUTOMOUNT, and skip the capability >> >> check if that is set. >> >> >> >> Any opinions or other ideas? >> > >> > I haven't seen any responses, possibly just got lost in the shuffle >> > during the holidays (I know it slipped my mind for a while). >> > >> > Eric, what do you think about the last option above? From what I can see >> > looking up rpc credentials just isn't going to work with current_cred >> > overridden as we're doing for automount. >> >> I got as far as there wasn't a correct thing to apply, and I have been >> bogged down in enough other things that I haven't gotten back to this >> one. >> >> My gut feel is that we propbably want to do a little more reworking on >> the autmount path. But I don't exactly have a concrete proposal for >> you at the moment. I just found another 10 year old bug in the mount >> code... > > With automounts we're mounting based on the credentials used when > mounting the parent. That's what your patch "fs: Call d_automount with > the filesystems creds" did, but it's overriding the credentials too > early in the call stack and causing these rpcauth problems. > > But I think we should also be setting the submount's s_user_ns to be the > same as the parent's, not to current_user_ns. At that point we'd be > using the same credentials and user ns as when mounting the parent super > block, so we know the capability check would pass (since it passed for > the original mount) and don't really need to do it. If we special case the submount code that would be fine. Normal sget needs to continue to use current_user_ns. > So if we could pass down through the call stack that a given mount > request is an automount from super block s (or at dentry d) then the fix > would be trivial. I don't see any way of passing that information > through currently though, without doing something undesirable like > adding arguments to the mount filesystem op. So here are the pieces I have in my thinking about this. 1) We need to capture the cred of the mounter of the filesystem like overlayfs does but in general. Call it s_cred or possibly s_mounter_cred. That would allow us to remove the hard coded cred in the automount path, and would allow unprivileged mounts to eventually use this functionality. 2) Al Viro mentioned to me that it would be very nice if the automount could coud run in a separate stack because this is one place where the stack can get very deep in the vfs. My gut feel is that solving for those two constraints: the code running in a separate thread, and the capturing not just s_user_ns but the cred of the mounter should give us enough constraints to figure out how to structure the code for long term maintenance. I especially think the part about using the mounters creds will likely solve the sunrpc problem. I could of course be wrong but using the creds of the process that happened to walk across the mount point seems completely the wrong thing. In general it feels wrong to expose which user triggered the automount to me. As that means the semantics can very per user. Now when we looked at autofs the semantics were in fact varying per user (ugh). So NFS may have the same legacy requirements. But as a general maintainability principle I don't like vfs state that varies depending upon the user. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
With respect to nfs and automounts. Does NFS have different automount behavior based on the user performing the automount? If NFS does not have different automount behavior depending on the user we just use the creds of the original mounter of NFS? If NFS does have different automount behavior depending on the user (ouch!) we need to go through the call path and see where it makes sense to over ride things and where it does not. Seth the fundamental problem with your patch was that you were patching a location that is used for more just mounts. I am strongly wishing that we could just change follow_automount from: old_cred = override_creds(&init_cred); mnt = path->dentry->d_op->d_automount(path); revert_creds(old_cred); to: old_cred = override_creds(path->mnt->mnt_sb->s_cred); mnt = path->dentry->d_op->d_automount(path); revert_creds(old_cred); And all will be well with nfs. That does remain possible. But looking at the code path you touched it seems to lookup the cred based purely on the local uid, gid, and groups. Which suggests to me that even the original mounters creds may not be enough :( At which point I am not certain of the solution. But I fear that like autofs NFS actually cares which user is transition the magic mountpoint, and may return different data depending on who transitions the mountpoint first. Ick! Nasty Nasty Ick! Eric -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-01-25 at 12:28 +1300, Eric W. Biederman wrote: > With respect to nfs and automounts. > > Does NFS have different automount behavior based on the user > performing the automount? > > If NFS does not have different automount behavior depending on the > user > we just use the creds of the original mounter of NFS? > > If NFS does have different automount behavior depending on the user > (ouch!) we need to go through the call path and see where it makes > sense to over ride things and where it does not. The reason why the NFS client creates a mountpoint is because on entering a directory, it detects that there is either a similar mountpoint on the server, or there is a referral (which acts sort of like a symlink, except it points to a path on one or more different NFS servers). Without that mountpoint, most things would work, but the user would end up seeing nasty non-posix features like duplicate inode numbers. We do not want to use any creds other than user creds here, because as far as the security model is concerned, the process is just crossing into an existing directory. > > > > Seth the fundamental problem with your patch was that you were > patching > a location that is used for more just mounts. > > I am strongly wishing that we could just change follow_automount > from: > > > old_cred = override_creds(&init_cred); > mnt = path->dentry->d_op->d_automount(path); > revert_creds(old_cred); > > to: > > old_cred = override_creds(path->mnt->mnt_sb->s_cred); > mnt = path->dentry->d_op->d_automount(path); > revert_creds(old_cred); > > And all will be well with nfs. That does remain possible. No. That would break permissions checking. See above. > > But looking at the code path you touched it seems to lookup the cred > based purely on the local uid, gid, and groups. Which suggests to > me that even the original mounters creds may not be enough :( > > At which point I am not certain of the solution. But I fear that > like > autofs NFS actually cares which user is transition the magic > mountpoint, > and may return different data depending on who transitions the > mountpoint first. Ick! Nasty Nasty Ick! > The security model is the same as that of an ordinary directory. The only difference is that we create a new superblock. Why is that "Ick"? -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
Trond Myklebust <trondmy@primarydata.com> writes: > On Wed, 2017-01-25 at 12:28 +1300, Eric W. Biederman wrote: >> With respect to nfs and automounts. >> >> Does NFS have different automount behavior based on the user >> performing the automount? >> >> If NFS does not have different automount behavior depending on the >> user >> we just use the creds of the original mounter of NFS? >> >> If NFS does have different automount behavior depending on the user >> (ouch!) we need to go through the call path and see where it makes >> sense to over ride things and where it does not. > > The reason why the NFS client creates a mountpoint is because on > entering a directory, it detects that there is either a similar > mountpoint on the server, or there is a referral (which acts sort of > like a symlink, except it points to a path on one or more different NFS > servers). > Without that mountpoint, most things would work, but the user would end > up seeing nasty non-posix features like duplicate inode numbers. > > We do not want to use any creds other than user creds here, because as > far as the security model is concerned, the process is just crossing > into an existing directory. But sget needs different creds. Because the user who authorizes the mounting of the filesystem is different than the user who is crossing into the new filesystem. The local system now cares about that distinction even if the nfs server does not. >> Seth the fundamental problem with your patch was that you were >> patching >> a location that is used for more just mounts. >> >> I am strongly wishing that we could just change follow_automount >> from: >> >> >> old_cred = override_creds(&init_cred); >> mnt = path->dentry->d_op->d_automount(path); >> revert_creds(old_cred); >> >> to: >> >> old_cred = override_creds(path->mnt->mnt_sb->s_cred); >> mnt = path->dentry->d_op->d_automount(path); >> revert_creds(old_cred); >> >> And all will be well with nfs. That does remain possible. > > No. That would break permissions checking. See above. Then we need to look much harder at the permission checking model of d_automount because we need to permission check against both sets of creds. >> But looking at the code path you touched it seems to lookup the cred >> based purely on the local uid, gid, and groups. Which suggests to >> me that even the original mounters creds may not be enough :( >> >> At which point I am not certain of the solution. But I fear that >> like >> autofs NFS actually cares which user is transition the magic >> mountpoint, >> and may return different data depending on who transitions the >> mountpoint first. Ick! Nasty Nasty Ick! >> > > The security model is the same as that of an ordinary directory. The > only difference is that we create a new superblock. Why is that "Ick"? The Ick is if the superblock that is created depends on the user crossing the mountpoint. AKA Do we get a different mountpoint if user A triggers the automount vs user B triggering the automount? If the problem is just root squash and not letting root trigger the automount it is much less ick. Weird but not ick. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
QWRkaW5nIERhdmlkIEhvd2VsbHMgYW5kIFN0ZXZlIEZyZW5jaCBhcyBJIGJlbGlldmUgYm90aCBB RlMgYW5kIENJRlMNCmhhdmUgdGhlIGV4YWN0IHNhbWUgcmVxdWlyZW1lbnRzIGFuZCBORlMgaGVy ZS4NCg0KT24gV2VkLCAyMDE3LTAxLTI1IGF0IDEyOjU2ICsxMzAwLCBFcmljIFcuIEJpZWRlcm1h biB3cm90ZToNCj4gVHJvbmQgTXlrbGVidXN0IDx0cm9uZG15QHByaW1hcnlkYXRhLmNvbT4gd3Jp dGVzOg0KPiANCj4gPiBPbiBXZWQsIDIwMTctMDEtMjUgYXQgMTI6MjggKzEzMDAsIEVyaWMgVy4g QmllZGVybWFuIHdyb3RlOg0KPiA+ID4gV2l0aCByZXNwZWN0IHRvIG5mcyBhbmQgYXV0b21vdW50 cy4NCj4gPiA+IA0KPiA+ID4gRG9lcyBORlMgaGF2ZSBkaWZmZXJlbnQgYXV0b21vdW50IGJlaGF2 aW9yIGJhc2VkIG9uIHRoZSB1c2VyDQo+ID4gPiBwZXJmb3JtaW5nIHRoZSBhdXRvbW91bnQ/DQo+ ID4gPiANCj4gPiA+IElmIE5GUyBkb2VzIG5vdCBoYXZlIGRpZmZlcmVudCBhdXRvbW91bnQgYmVo YXZpb3IgZGVwZW5kaW5nIG9uDQo+ID4gPiB0aGUNCj4gPiA+IHVzZXINCj4gPiA+IHdlIGp1c3Qg dXNlIHRoZSBjcmVkcyBvZiB0aGUgb3JpZ2luYWwgbW91bnRlciBvZiBORlM/DQo+ID4gPiANCj4g PiA+IElmIE5GUyBkb2VzIGhhdmUgZGlmZmVyZW50IGF1dG9tb3VudCBiZWhhdmlvciBkZXBlbmRp bmcgb24gdGhlDQo+ID4gPiB1c2VyDQo+ID4gPiAob3VjaCEpIHdlIG5lZWQgdG8gZ28gdGhyb3Vn aCB0aGUgY2FsbCBwYXRoIGFuZCBzZWUgd2hlcmUgaXQNCj4gPiA+IG1ha2VzDQo+ID4gPiBzZW5z ZSB0byBvdmVyIHJpZGUgdGhpbmdzIGFuZCB3aGVyZSBpdCBkb2VzIG5vdC4NCj4gPiANCj4gPiBU aGUgcmVhc29uIHdoeSB0aGUgTkZTIGNsaWVudCBjcmVhdGVzIGEgbW91bnRwb2ludCBpcyBiZWNh dXNlIG9uDQo+ID4gZW50ZXJpbmcgYSBkaXJlY3RvcnksIGl0IGRldGVjdHMgdGhhdCB0aGVyZSBp cyBlaXRoZXIgYSBzaW1pbGFyDQo+ID4gbW91bnRwb2ludCBvbiB0aGUgc2VydmVyLCBvciB0aGVy ZSBpcyBhIHJlZmVycmFsICh3aGljaCBhY3RzIHNvcnQNCj4gPiBvZg0KPiA+IGxpa2UgYSBzeW1s aW5rLCBleGNlcHQgaXQgcG9pbnRzIHRvIGEgcGF0aCBvbiBvbmUgb3IgbW9yZSBkaWZmZXJlbnQN Cj4gPiBORlMNCj4gPiBzZXJ2ZXJzKS4NCj4gPiBXaXRob3V0IHRoYXQgbW91bnRwb2ludCwgbW9z dCB0aGluZ3Mgd291bGQgd29yaywgYnV0IHRoZSB1c2VyIHdvdWxkDQo+ID4gZW5kDQo+ID4gdXAg c2VlaW5nIG5hc3R5IG5vbi1wb3NpeCBmZWF0dXJlcyBsaWtlIGR1cGxpY2F0ZSBpbm9kZSBudW1i ZXJzLg0KPiA+IA0KPiA+IFdlIGRvIG5vdCB3YW50IHRvIHVzZSBhbnkgY3JlZHMgb3RoZXIgdGhh biB1c2VyIGNyZWRzIGhlcmUsIGJlY2F1c2UNCj4gPiBhcw0KPiA+IGZhciBhcyB0aGUgc2VjdXJp dHkgbW9kZWwgaXMgY29uY2VybmVkLCB0aGUgcHJvY2VzcyBpcyBqdXN0DQo+ID4gY3Jvc3NpbmcN Cj4gPiBpbnRvIGFuIGV4aXN0aW5nIGRpcmVjdG9yeS4NCj4gDQo+IEJ1dCBzZ2V0IG5lZWRzIGRp ZmZlcmVudCBjcmVkcy4NCj4gDQo+IEJlY2F1c2UgdGhlIHVzZXIgd2hvIGF1dGhvcml6ZXMgdGhl IG1vdW50aW5nIG9mIHRoZSBmaWxlc3lzdGVtIGlzDQo+IGRpZmZlcmVudCB0aGFuIHRoZSB1c2Vy IHdobyBpcyBjcm9zc2luZyBpbnRvIHRoZSBuZXcgZmlsZXN5c3RlbS4NCj4gDQo+IFRoZSBsb2Nh bCBzeXN0ZW0gbm93IGNhcmVzIGFib3V0IHRoYXQgZGlzdGluY3Rpb24gZXZlbiBpZiB0aGUgbmZz DQo+IHNlcnZlcg0KPiBkb2VzIG5vdC4NCg0KV2h5PyBUaGUgZmlsZXN5c3RlbSBpcyBhbHJlYWR5 IG1vdW50ZWQuIFdlJ3JlIGNyZWF0aW5nIGEgbmV3DQptb3VudHBvaW50LCBidXQgd2UgY291bGQg ZXF1YWxseSB3ZWxsIGp1c3Qgc2F5ICdzb2QgdGhhdCcgYW5kIGNyZWF0ZSBhbg0Kb3JkaW5hcnkg ZGlyZWN0b3J5LiBUaGUgcGVuYWx0eSB3b3VsZCBiZSBhZm9yZW1lbnRpb25lZCBub24tcG9zaXgN CndlaXJkbmVzcy4NCg0KPiANCj4gPiA+IFNldGggdGhlIGZ1bmRhbWVudGFsIHByb2JsZW0gd2l0 aCB5b3VyIHBhdGNoIHdhcyB0aGF0IHlvdSB3ZXJlDQo+ID4gPiBwYXRjaGluZw0KPiA+ID4gYSBs b2NhdGlvbiB0aGF0IGlzIHVzZWQgZm9yIG1vcmUganVzdCBtb3VudHMuDQo+ID4gPiANCj4gPiA+ IEkgYW0gc3Ryb25nbHkgd2lzaGluZyB0aGF0IHdlIGNvdWxkIGp1c3QgY2hhbmdlIGZvbGxvd19h dXRvbW91bnQNCj4gPiA+IGZyb206DQo+ID4gPiANCj4gPiA+IA0KPiA+ID4gCW9sZF9jcmVkID0g b3ZlcnJpZGVfY3JlZHMoJmluaXRfY3JlZCk7DQo+ID4gPiAJbW50ID0gcGF0aC0+ZGVudHJ5LT5k X29wLT5kX2F1dG9tb3VudChwYXRoKTsNCj4gPiA+IAlyZXZlcnRfY3JlZHMob2xkX2NyZWQpOw0K PiA+ID4gDQo+ID4gPiB0bzoNCj4gPiA+IA0KPiA+ID4gCW9sZF9jcmVkID0gb3ZlcnJpZGVfY3Jl ZHMocGF0aC0+bW50LT5tbnRfc2ItPnNfY3JlZCk7DQo+ID4gPiAJbW50ID0gcGF0aC0+ZGVudHJ5 LT5kX29wLT5kX2F1dG9tb3VudChwYXRoKTsNCj4gPiA+IAlyZXZlcnRfY3JlZHMob2xkX2NyZWQp Ow0KPiA+ID4gDQo+ID4gPiBBbmQgYWxsIHdpbGwgYmUgd2VsbCB3aXRoIG5mcy7CoMKgVGhhdCBk b2VzIHJlbWFpbiBwb3NzaWJsZS4NCj4gPiANCj4gPiBOby4gVGhhdCB3b3VsZCBicmVhayBwZXJt aXNzaW9ucyBjaGVja2luZy4gU2VlIGFib3ZlLg0KPiANCj4gVGhlbiB3ZSBuZWVkIHRvIGxvb2sg bXVjaCBoYXJkZXIgYXQgdGhlIHBlcm1pc3Npb24gY2hlY2tpbmcNCj4gbW9kZWwgb2YgZF9hdXRv bW91bnQgYmVjYXVzZSB3ZSBuZWVkIHRvIHBlcm1pc3Npb24gY2hlY2sgYWdhaW5zdA0KPiBib3Ro IHNldHMgb2YgY3JlZHMuDQo+IA0KPiA+ID4gQnV0IGxvb2tpbmcgYXQgdGhlIGNvZGUgcGF0aCB5 b3UgdG91Y2hlZCBpdCBzZWVtcyB0byBsb29rdXAgdGhlDQo+ID4gPiBjcmVkDQo+ID4gPiBiYXNl ZCBwdXJlbHkgb24gdGhlIGxvY2FsIHVpZCwgZ2lkLCBhbmQgZ3JvdXBzLsKgwqBXaGljaCBzdWdn ZXN0cw0KPiA+ID4gdG8NCj4gPiA+IG1lIHRoYXQgZXZlbiB0aGUgb3JpZ2luYWwgbW91bnRlcnMg Y3JlZHMgbWF5IG5vdCBiZSBlbm91Z2ggOigNCj4gPiA+IA0KPiA+ID4gQXQgd2hpY2ggcG9pbnQg SSBhbSBub3QgY2VydGFpbiBvZiB0aGUgc29sdXRpb24uwqDCoEJ1dCBJIGZlYXIgdGhhdA0KPiA+ ID4gbGlrZQ0KPiA+ID4gYXV0b2ZzIE5GUyBhY3R1YWxseSBjYXJlcyB3aGljaCB1c2VyIGlzIHRy YW5zaXRpb24gdGhlIG1hZ2ljDQo+ID4gPiBtb3VudHBvaW50LA0KPiA+ID4gYW5kIG1heSByZXR1 cm4gZGlmZmVyZW50IGRhdGEgZGVwZW5kaW5nIG9uIHdobyB0cmFuc2l0aW9ucyB0aGUNCj4gPiA+ IG1vdW50cG9pbnQgZmlyc3QuwqDCoEljayHCoMKgTmFzdHkgTmFzdHkgSWNrIQ0KPiA+ID4gDQo+ ID4gDQo+ID4gVGhlIHNlY3VyaXR5IG1vZGVsIGlzIHRoZSBzYW1lIGFzIHRoYXQgb2YgYW4gb3Jk aW5hcnkgZGlyZWN0b3J5Lg0KPiA+IFRoZQ0KPiA+IG9ubHkgZGlmZmVyZW5jZSBpcyB0aGF0IHdl IGNyZWF0ZSBhIG5ldyBzdXBlcmJsb2NrLiBXaHkgaXMgdGhhdA0KPiA+ICJJY2siPw0KPiANCj4g VGhlIEljayBpcyBpZiB0aGUgc3VwZXJibG9jayB0aGF0IGlzIGNyZWF0ZWQgZGVwZW5kcyBvbiB0 aGUgdXNlcg0KPiBjcm9zc2luZyB0aGUgbW91bnRwb2ludC4NCj4gDQo+IEFLQSBEbyB3ZSBnZXQg YSBkaWZmZXJlbnQgbW91bnRwb2ludCBpZiB1c2VyIEEgdHJpZ2dlcnMgdGhlIGF1dG9tb3VudA0K PiB2cyB1c2VyIEIgdHJpZ2dlcmluZyB0aGUgYXV0b21vdW50Pw0KDQpZb3UgbWF5IGdldCBhbiBF QUNDRVMuIE90aGVyd2lzZSB0aGUgbW91bnRwb2ludHMgc2hvdWxkIGJlIHRoZSBzYW1lLg0KDQo+ IElmIHRoZSBwcm9ibGVtIGlzIGp1c3Qgcm9vdCBzcXVhc2ggYW5kIG5vdCBsZXR0aW5nIHJvb3Qg dHJpZ2dlciB0aGUNCj4gYXV0b21vdW50IGl0IGlzIG11Y2ggbGVzcyBpY2suwqDCoFdlaXJkIGJ1 dCBub3QgaWNrLg0KDQpOb3QganVzdCByb290IHNxdWFzaCwgYnV0IGdlbmVyYWwgcGVybWlzc2lv bnMgY2hlY2tpbmcuIFdlIGRvbid0IHdhbnQNCnRvIGdpdmUgdXNlciBCIHJvb3QgcHJpdmlsZWdl cyBhbGxvd2luZyBjcm9zc2luZyBpbnRvIGEgZGlyZWN0b3J5IHRvDQp3aGljaCBzL2hlIHdvdWxk IG9yZGluYXJpbHkgaGF2ZSBubyByaWdodHMgZWl0aGVyLg0KDQotLSANClRyb25kIE15a2xlYnVz dA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlrbGVi dXN0QHByaW1hcnlkYXRhLmNvbQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c index 2bff63a73cf8..e6197b2bda86 100644 --- a/net/sunrpc/auth.c +++ b/net/sunrpc/auth.c @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int flags) { struct auth_cred acred; struct rpc_cred *ret; - const struct cred *cred = current_cred(); + const struct cred *cred = current_real_cred(); dprintk("RPC: looking up %s cred\n", auth->au_ops->au_name);
Since 4.8 follow_automount() overrides the credentials with &init_cred before calling d_automount(). When rpcauth_lookupcred() is called in this context it is now using fs[ug]id from the override creds instead of from the user's creds, which can cause authentication to fail. To fix this, take the ids from current_real_cred() instead. Cc: stable@vger.kernel.org # v4.8+ CC: Eric W. Biederman <ebiederm@xmission.com> Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems creds") Signed-off-by: Seth Forshee <seth.forshee@canonical.com> --- net/sunrpc/auth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)