* [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials @ 2016-12-15 17:13 Seth Forshee 2016-12-15 23:01 ` Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Seth Forshee @ 2016-12-15 17:13 UTC (permalink / raw) To: Trond Myklebust, Anna Schumaker, J. Bruce Fields Cc: Eric W . Biederman, linux-nfs 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); -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2016-12-15 17:13 [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials Seth Forshee @ 2016-12-15 23:01 ` Trond Myklebust 2016-12-16 13:06 ` Seth Forshee 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2016-12-15 23:01 UTC (permalink / raw) To: bfields, anna.schumaker, seth.forshee; +Cc: ebiederm, linux-nfs T24gVGh1LCAyMDE2LTEyLTE1IGF0IDExOjEzIC0wNjAwLCBTZXRoIEZvcnNoZWUgd3JvdGU6DQo+ IFNpbmNlIDQuOCBmb2xsb3dfYXV0b21vdW50KCkgb3ZlcnJpZGVzIHRoZSBjcmVkZW50aWFscyB3 aXRoDQo+ICZpbml0X2NyZWQgYmVmb3JlIGNhbGxpbmcgZF9hdXRvbW91bnQoKS4gV2hlbg0KPiBy cGNhdXRoX2xvb2t1cGNyZWQoKSBpcyBjYWxsZWQgaW4gdGhpcyBjb250ZXh0IGl0IGlzIG5vdyB1 c2luZw0KPiBmc1t1Z11pZCBmcm9tIHRoZSBvdmVycmlkZSBjcmVkcyBpbnN0ZWFkIG9mIGZyb20g dGhlIHVzZXIncw0KPiBjcmVkcywgd2hpY2ggY2FuIGNhdXNlIGF1dGhlbnRpY2F0aW9uIHRvIGZh aWwuIFRvIGZpeCB0aGlzLCB0YWtlDQo+IHRoZSBpZHMgZnJvbSBjdXJyZW50X3JlYWxfY3JlZCgp IGluc3RlYWQuDQo+IA0KPiBDYzogc3RhYmxlQHZnZXIua2VybmVsLm9yZyAjIHY0LjgrDQo+IEND OiBFcmljIFcuIEJpZWRlcm1hbiA8ZWJpZWRlcm1AeG1pc3Npb24uY29tPg0KPiBGaXhlczogYWVh YTRhNzlmZjZhICgiZnM6IENhbGwgZF9hdXRvbW91bnQgd2l0aCB0aGUgZmlsZXN5c3RlbXMNCj4g Y3JlZHMiKQ0KPiBTaWduZWQtb2ZmLWJ5OiBTZXRoIEZvcnNoZWUgPHNldGguZm9yc2hlZUBjYW5v bmljYWwuY29tPg0KPiAtLS0NCj4gwqBuZXQvc3VucnBjL2F1dGguYyB8IDIgKy0NCj4gwqAxIGZp bGUgY2hhbmdlZCwgMSBpbnNlcnRpb24oKyksIDEgZGVsZXRpb24oLSkNCj4gDQo+IGRpZmYgLS1n aXQgYS9uZXQvc3VucnBjL2F1dGguYyBiL25ldC9zdW5ycGMvYXV0aC5jDQo+IGluZGV4IDJiZmY2 M2E3M2NmOC4uZTYxOTdiMmJkYTg2IDEwMDY0NA0KPiAtLS0gYS9uZXQvc3VucnBjL2F1dGguYw0K PiArKysgYi9uZXQvc3VucnBjL2F1dGguYw0KPiBAQCAtNjIyLDcgKzYyMiw3IEBAIHJwY2F1dGhf bG9va3VwY3JlZChzdHJ1Y3QgcnBjX2F1dGggKmF1dGgsIGludA0KPiBmbGFncykNCj4gwqB7DQo+ IMKgCXN0cnVjdCBhdXRoX2NyZWQgYWNyZWQ7DQo+IMKgCXN0cnVjdCBycGNfY3JlZCAqcmV0Ow0K PiAtCWNvbnN0IHN0cnVjdCBjcmVkICpjcmVkID0gY3VycmVudF9jcmVkKCk7DQo+ICsJY29uc3Qg c3RydWN0IGNyZWQgKmNyZWQgPSBjdXJyZW50X3JlYWxfY3JlZCgpOw0KPiDCoA0KPiDCoAlkcHJp bnRrKCJSUEM6wqDCoMKgwqDCoMKgwqBsb29raW5nIHVwICVzIGNyZWRcbiIsDQo+IMKgCQlhdXRo LT5hdV9vcHMtPmF1X25hbWUpOw0KDQpBbW9uZyBvdGhlciB0aGluZ3MsIHRoaXMgd2lsbCBicmVh ayB0aGUgYWNjZXNzKCkgc3lzY2FsbC4gSXQncw0KY29tcGxldGVseSB0aGUgd3JvbmcgbGV2ZWwg aW4gd2hpY2ggdG8gb3ZlcnJpZGUgY3JlZGVudGlhbHMuDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0 DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1 c3RAcHJpbWFyeWRhdGEuY29tDQo= ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2016-12-15 23:01 ` Trond Myklebust @ 2016-12-16 13:06 ` Seth Forshee 2017-01-10 14:55 ` Seth Forshee 0 siblings, 1 reply; 19+ messages in thread From: Seth Forshee @ 2016-12-16 13:06 UTC (permalink / raw) To: Trond Myklebust; +Cc: bfields, anna.schumaker, ebiederm, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2016-12-16 13:06 ` Seth Forshee @ 2017-01-10 14:55 ` Seth Forshee 2017-01-11 0:21 ` Eric W. Biederman 0 siblings, 1 reply; 19+ messages in thread From: Seth Forshee @ 2017-01-10 14:55 UTC (permalink / raw) To: Trond Myklebust, ebiederm; +Cc: bfields, anna.schumaker, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-10 14:55 ` Seth Forshee @ 2017-01-11 0:21 ` Eric W. Biederman 2017-01-24 15:17 ` Seth Forshee 0 siblings, 1 reply; 19+ messages in thread From: Eric W. Biederman @ 2017-01-11 0:21 UTC (permalink / raw) To: Seth Forshee; +Cc: Trond Myklebust, bfields, anna.schumaker, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-11 0:21 ` Eric W. Biederman @ 2017-01-24 15:17 ` Seth Forshee 2017-01-24 22:55 ` Eric W. Biederman 0 siblings, 1 reply; 19+ messages in thread From: Seth Forshee @ 2017-01-24 15:17 UTC (permalink / raw) To: Eric W. Biederman; +Cc: Trond Myklebust, bfields, anna.schumaker, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-24 15:17 ` Seth Forshee @ 2017-01-24 22:55 ` Eric W. Biederman 2017-01-24 23:28 ` Eric W. Biederman 0 siblings, 1 reply; 19+ messages in thread From: Eric W. Biederman @ 2017-01-24 22:55 UTC (permalink / raw) To: Seth Forshee; +Cc: Trond Myklebust, bfields, anna.schumaker, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-24 22:55 ` Eric W. Biederman @ 2017-01-24 23:28 ` Eric W. Biederman 2017-01-24 23:46 ` Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Eric W. Biederman @ 2017-01-24 23:28 UTC (permalink / raw) To: Seth Forshee; +Cc: Trond Myklebust, bfields, anna.schumaker, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-24 23:28 ` Eric W. Biederman @ 2017-01-24 23:46 ` Trond Myklebust 2017-01-24 23:56 ` Eric W. Biederman 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2017-01-24 23:46 UTC (permalink / raw) To: ebiederm, seth.forshee; +Cc: bfields, anna.schumaker, linux-nfs T24gV2VkLCAyMDE3LTAxLTI1IGF0IDEyOjI4ICsxMzAwLCBFcmljIFcuIEJpZWRlcm1hbiB3cm90 ZToNCj4gV2l0aCByZXNwZWN0IHRvIG5mcyBhbmQgYXV0b21vdW50cy4NCj4gDQo+IERvZXMgTkZT IGhhdmUgZGlmZmVyZW50IGF1dG9tb3VudCBiZWhhdmlvciBiYXNlZCBvbiB0aGUgdXNlcg0KPiBw ZXJmb3JtaW5nIHRoZSBhdXRvbW91bnQ/DQo+IA0KPiBJZiBORlMgZG9lcyBub3QgaGF2ZSBkaWZm ZXJlbnQgYXV0b21vdW50IGJlaGF2aW9yIGRlcGVuZGluZyBvbiB0aGUNCj4gdXNlcg0KPiB3ZSBq dXN0IHVzZSB0aGUgY3JlZHMgb2YgdGhlIG9yaWdpbmFsIG1vdW50ZXIgb2YgTkZTPw0KPiANCj4g SWYgTkZTIGRvZXMgaGF2ZSBkaWZmZXJlbnQgYXV0b21vdW50IGJlaGF2aW9yIGRlcGVuZGluZyBv biB0aGUgdXNlcg0KPiAob3VjaCEpIHdlIG5lZWQgdG8gZ28gdGhyb3VnaCB0aGUgY2FsbCBwYXRo IGFuZCBzZWUgd2hlcmUgaXQgbWFrZXMNCj4gc2Vuc2UgdG8gb3ZlciByaWRlIHRoaW5ncyBhbmQg d2hlcmUgaXQgZG9lcyBub3QuDQoNClRoZSByZWFzb24gd2h5IHRoZSBORlMgY2xpZW50IGNyZWF0 ZXMgYSBtb3VudHBvaW50IGlzIGJlY2F1c2Ugb24NCmVudGVyaW5nIGEgZGlyZWN0b3J5LCBpdCBk ZXRlY3RzIHRoYXQgdGhlcmUgaXMgZWl0aGVyIGEgc2ltaWxhcg0KbW91bnRwb2ludCBvbiB0aGUg c2VydmVyLCBvciB0aGVyZSBpcyBhIHJlZmVycmFsICh3aGljaCBhY3RzIHNvcnQgb2YNCmxpa2Ug YSBzeW1saW5rLCBleGNlcHQgaXQgcG9pbnRzIHRvIGEgcGF0aCBvbiBvbmUgb3IgbW9yZSBkaWZm ZXJlbnQgTkZTDQpzZXJ2ZXJzKS4NCldpdGhvdXQgdGhhdCBtb3VudHBvaW50LCBtb3N0IHRoaW5n cyB3b3VsZCB3b3JrLCBidXQgdGhlIHVzZXIgd291bGQgZW5kDQp1cCBzZWVpbmcgbmFzdHkgbm9u LXBvc2l4IGZlYXR1cmVzIGxpa2UgZHVwbGljYXRlIGlub2RlIG51bWJlcnMuDQoNCldlIGRvIG5v dCB3YW50IHRvIHVzZSBhbnkgY3JlZHMgb3RoZXIgdGhhbiB1c2VyIGNyZWRzIGhlcmUsIGJlY2F1 c2UgYXMNCmZhciBhcyB0aGUgc2VjdXJpdHkgbW9kZWwgaXMgY29uY2VybmVkLCB0aGUgcHJvY2Vz cyBpcyBqdXN0IGNyb3NzaW5nDQppbnRvIGFuIGV4aXN0aW5nIGRpcmVjdG9yeS4NCj4gDQo+IA0K PiANCj4gU2V0aCB0aGUgZnVuZGFtZW50YWwgcHJvYmxlbSB3aXRoIHlvdXIgcGF0Y2ggd2FzIHRo YXQgeW91IHdlcmUNCj4gcGF0Y2hpbmcNCj4gYSBsb2NhdGlvbiB0aGF0IGlzIHVzZWQgZm9yIG1v cmUganVzdCBtb3VudHMuDQo+IA0KPiBJIGFtIHN0cm9uZ2x5IHdpc2hpbmcgdGhhdCB3ZSBjb3Vs ZCBqdXN0IGNoYW5nZSBmb2xsb3dfYXV0b21vdW50DQo+IGZyb206DQo+IA0KPiANCj4gCW9sZF9j cmVkID0gb3ZlcnJpZGVfY3JlZHMoJmluaXRfY3JlZCk7DQo+IAltbnQgPSBwYXRoLT5kZW50cnkt PmRfb3AtPmRfYXV0b21vdW50KHBhdGgpOw0KPiAJcmV2ZXJ0X2NyZWRzKG9sZF9jcmVkKTsNCj4g DQo+IHRvOg0KPiANCj4gCW9sZF9jcmVkID0gb3ZlcnJpZGVfY3JlZHMocGF0aC0+bW50LT5tbnRf c2ItPnNfY3JlZCk7DQo+IAltbnQgPSBwYXRoLT5kZW50cnktPmRfb3AtPmRfYXV0b21vdW50KHBh dGgpOw0KPiAJcmV2ZXJ0X2NyZWRzKG9sZF9jcmVkKTsNCj4gDQo+IEFuZCBhbGwgd2lsbCBiZSB3 ZWxsIHdpdGggbmZzLsKgwqBUaGF0IGRvZXMgcmVtYWluIHBvc3NpYmxlLg0KDQpOby4gVGhhdCB3 b3VsZCBicmVhayBwZXJtaXNzaW9ucyBjaGVja2luZy4gU2VlIGFib3ZlLg0KDQo+IA0KPiBCdXQg bG9va2luZyBhdCB0aGUgY29kZSBwYXRoIHlvdSB0b3VjaGVkIGl0IHNlZW1zIHRvIGxvb2t1cCB0 aGUgY3JlZA0KPiBiYXNlZCBwdXJlbHkgb24gdGhlIGxvY2FsIHVpZCwgZ2lkLCBhbmQgZ3JvdXBz LsKgwqBXaGljaCBzdWdnZXN0cyB0bw0KPiBtZSB0aGF0IGV2ZW4gdGhlIG9yaWdpbmFsIG1vdW50 ZXJzIGNyZWRzIG1heSBub3QgYmUgZW5vdWdoIDooDQo+IA0KPiBBdCB3aGljaCBwb2ludCBJIGFt IG5vdCBjZXJ0YWluIG9mIHRoZSBzb2x1dGlvbi7CoMKgQnV0IEkgZmVhciB0aGF0DQo+IGxpa2UN Cj4gYXV0b2ZzIE5GUyBhY3R1YWxseSBjYXJlcyB3aGljaCB1c2VyIGlzIHRyYW5zaXRpb24gdGhl IG1hZ2ljDQo+IG1vdW50cG9pbnQsDQo+IGFuZCBtYXkgcmV0dXJuIGRpZmZlcmVudCBkYXRhIGRl cGVuZGluZyBvbiB3aG8gdHJhbnNpdGlvbnMgdGhlDQo+IG1vdW50cG9pbnQgZmlyc3QuwqDCoElj ayHCoMKgTmFzdHkgTmFzdHkgSWNrIQ0KPiANCg0KVGhlIHNlY3VyaXR5IG1vZGVsIGlzIHRoZSBz YW1lIGFzIHRoYXQgb2YgYW4gb3JkaW5hcnkgZGlyZWN0b3J5LiBUaGUNCm9ubHkgZGlmZmVyZW5j ZSBpcyB0aGF0IHdlIGNyZWF0ZSBhIG5ldyBzdXBlcmJsb2NrLiBXaHkgaXMgdGhhdCAiSWNrIj8N Cg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgUHJp bWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg== ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-24 23:46 ` Trond Myklebust @ 2017-01-24 23:56 ` Eric W. Biederman 2017-01-25 0:14 ` Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Eric W. Biederman @ 2017-01-24 23:56 UTC (permalink / raw) To: Trond Myklebust; +Cc: seth.forshee, bfields, anna.schumaker, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-24 23:56 ` Eric W. Biederman @ 2017-01-25 0:14 ` Trond Myklebust 2017-01-25 14:52 ` Seth Forshee 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2017-01-25 0:14 UTC (permalink / raw) To: Steve French, ebiederm, dhowells Cc: bfields, anna.schumaker, seth.forshee, linux-nfs 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-25 0:14 ` Trond Myklebust @ 2017-01-25 14:52 ` Seth Forshee 2017-01-25 15:51 ` Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Seth Forshee @ 2017-01-25 14:52 UTC (permalink / raw) To: Trond Myklebust, ebiederm Cc: Steve French, dhowells, bfields, anna.schumaker, linux-nfs On Wed, Jan 25, 2017 at 12:14:26AM +0000, Trond Myklebust wrote: > Adding David Howells and Steve French as I believe both AFS and CIFS > have the exact same requirements and NFS here. > > On Wed, 2017-01-25 at 12:56 +1300, Eric W. Biederman wrote: > > 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. > > Why? The filesystem is already mounted. We're creating a new > mountpoint, but we could equally well just say 'sod that' and create an > ordinary directory. The penalty would be aforementioned non-posix > weirdness. > > > > > > > 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. How about something like this? Essentially, stash the creds used at mount time in the super block, then create a vfs_kern_automount() function which overrides the currend creds then calls vfs_kern_mount(). Only compile tested so far, and probably it should be split up into several patches. diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c index 81dd075356b9..4455e64610d3 100644 --- a/fs/afs/mntpt.c +++ b/fs/afs/mntpt.c @@ -202,7 +202,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt) /* try and do the mount */ _debug("--- attempting mount %s -o %s ---", devname, options); - mnt = vfs_kern_mount(&afs_fs_type, 0, devname, options); + mnt = vfs_kern_automount(mntpt, &afs_fs_type, 0, devname, options); _debug("--- mount result %p ---", mnt); free_page((unsigned long) devname); diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c index ec9dbbcca3b9..d378f88e8630 100644 --- a/fs/cifs/cifs_dfs_ref.c +++ b/fs/cifs/cifs_dfs_ref.c @@ -245,9 +245,10 @@ char *cifs_compose_mount_options(const char *sb_mountdata, * @fullpath: full path in UNC format * @ref: server's referral */ -static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb, +static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt, const char *fullpath, const struct dfs_info3_param *ref) { + struct cifs_sb_info *cifs_sb = CIFS_SB(mntpt->d_sb); struct vfsmount *mnt; char *mountdata; char *devname = NULL; @@ -259,7 +260,7 @@ static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb, if (IS_ERR(mountdata)) return (struct vfsmount *)mountdata; - mnt = vfs_kern_mount(&cifs_fs_type, 0, devname, mountdata); + mnt = vfs_kern_automount(mntpt, &cifs_fs_type, 0, devname, mountdata); kfree(mountdata); kfree(devname); return mnt; @@ -334,7 +335,7 @@ static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt) mnt = ERR_PTR(-EINVAL); break; } - mnt = cifs_dfs_do_refmount(cifs_sb, + mnt = cifs_dfs_do_refmount(mntpt, full_path, referrals + i); cifs_dbg(FYI, "%s: cifs_dfs_do_refmount:%s , mnt:%p\n", __func__, referrals[i].node_name, mnt); diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index f17fcf89e18e..d45f131af869 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -187,9 +187,9 @@ static const struct super_operations debugfs_super_operations = { static struct vfsmount *debugfs_automount(struct path *path) { - struct vfsmount *(*f)(void *); - f = (struct vfsmount *(*)(void *))path->dentry->d_fsdata; - return f(d_inode(path->dentry)->i_private); + struct vfsmount *(*f)(struct dentry *, void *); + f = (struct vfsmount *(*)(struct dentry *, void *))path->dentry->d_fsdata; + return f(path->dentry, d_inode(path->dentry)->i_private); } static const struct dentry_operations debugfs_dops = { @@ -504,7 +504,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir); */ struct dentry *debugfs_create_automount(const char *name, struct dentry *parent, - struct vfsmount *(*f)(void *), + struct vfsmount *(*f)(struct dentry *, void *), void *data) { struct dentry *dentry = start_creating(name, parent); diff --git a/fs/namespace.c b/fs/namespace.c index 487ba30bb5c6..da7e6dfa56cb 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void } EXPORT_SYMBOL_GPL(vfs_kern_mount); +struct vfsmount * +vfs_kern_automount(struct dentry *dentry, struct file_system_type *type, + int flags, const char *name, void *data) +{ + const struct cred *old_cred; + struct vfsmount *mnt; + + old_cred = override_creds(dentry->d_sb->s_cred); + mnt = vfs_kern_mount(type, flags, name, data); + revert_creds(old_cred); + + return mnt; +} +EXPORT_SYMBOL_GPL(vfs_kern_automount); + static struct mount *clone_mnt(struct mount *old, struct dentry *root, int flag) { diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c index 5551e8ef67fd..dce529c50cbd 100644 --- a/fs/nfs/namespace.c +++ b/fs/nfs/namespace.c @@ -222,11 +222,11 @@ void nfs_release_automount_timer(void) /* * Clone a mountpoint of the appropriate type */ -static struct vfsmount *nfs_do_clone_mount(struct nfs_server *server, +static struct vfsmount *nfs_do_clone_mount(struct dentry *dentry, const char *devname, struct nfs_clone_mount *mountdata) { - return vfs_kern_mount(&nfs_xdev_fs_type, 0, devname, mountdata); + return vfs_kern_automount(dentry, &nfs_xdev_fs_type, 0, devname, mountdata); } /** @@ -261,7 +261,7 @@ struct vfsmount *nfs_do_submount(struct dentry *dentry, struct nfs_fh *fh, mnt = (struct vfsmount *)devname; if (IS_ERR(devname)) goto free_page; - mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname, &mountdata); + mnt = nfs_do_clone_mount(dentry, devname, &mountdata); free_page: free_page((unsigned long)page); out: diff --git a/fs/super.c b/fs/super.c index 1709ed029a2c..b33b7d606aec 100644 --- a/fs/super.c +++ b/fs/super.c @@ -166,6 +166,7 @@ static void destroy_super(struct super_block *s) list_lru_destroy(&s->s_inode_lru); security_sb_free(s); WARN_ON(!list_empty(&s->s_mounts)); + put_cred(s->s_cred); put_user_ns(s->s_user_ns); kfree(s->s_subtype); kfree(s->s_options); @@ -193,6 +194,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags, INIT_LIST_HEAD(&s->s_mounts); s->s_user_ns = get_user_ns(user_ns); + s->s_cred = get_current_cred(); if (security_sb_alloc(s)) goto fail; diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 014cc564d1c4..67ac9f10e7b7 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -99,7 +99,7 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent, struct dentry *debugfs_create_automount(const char *name, struct dentry *parent, - struct vfsmount *(*f)(void *), + struct vfsmount *(*f)(struct dentry *, void *), void *data); void debugfs_remove(struct dentry *dentry); @@ -211,7 +211,7 @@ static inline struct dentry *debugfs_create_symlink(const char *name, static inline struct dentry *debugfs_create_automount(const char *name, struct dentry *parent, - struct vfsmount *(*f)(void *), + struct vfsmount *(*f)(struct dentry *, void *), void *data) { return ERR_PTR(-ENODEV); diff --git a/include/linux/fs.h b/include/linux/fs.h index 2ba074328894..cae845944d1d 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1374,6 +1374,9 @@ struct super_block { */ struct user_namespace *s_user_ns; + /* Credentials of the user who mounted the filesystem */ + const struct cred *s_cred; + /* * Keep the lru lists last in the structure so they always sit on their * own individual cachelines. diff --git a/include/linux/mount.h b/include/linux/mount.h index c6f55158d5e5..b9cdca0c6b1a 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -90,6 +90,9 @@ struct file_system_type; extern struct vfsmount *vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void *data); +struct vfsmount *vfs_kern_automount(struct dentry *dentry, + struct file_system_type *type, + int flags, const char *name, void *data); extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list); extern void mark_mounts_for_expiry(struct list_head *mounts); diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index d7449783987a..7bb959abd5f1 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -7503,7 +7503,7 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer) ftrace_init_tracefs(tr, d_tracer); } -static struct vfsmount *trace_automount(void *ingore) +static struct vfsmount *trace_automount(struct dentry *dentry, void *ingore) { struct vfsmount *mnt; struct file_system_type *type; @@ -7516,7 +7516,7 @@ static struct vfsmount *trace_automount(void *ingore) type = get_fs_type("tracefs"); if (!type) return NULL; - mnt = vfs_kern_mount(type, 0, "tracefs", NULL); + mnt = vfs_kern_automount(dentry, type, 0, "tracefs", NULL); put_filesystem(type); if (IS_ERR(mnt)) return NULL; ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-25 14:52 ` Seth Forshee @ 2017-01-25 15:51 ` Trond Myklebust 2017-01-25 16:28 ` Seth Forshee 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2017-01-25 15:51 UTC (permalink / raw) To: seth.forshee, ebiederm Cc: bfields, anna.schumaker, Steve French, linux-nfs, dhowells T24gV2VkLCAyMDE3LTAxLTI1IGF0IDA4OjUyIC0wNjAwLCBTZXRoIEZvcnNoZWUgd3JvdGU6DQo+ IE9uIFdlZCwgSmFuIDI1LCAyMDE3IGF0IDEyOjE0OjI2QU0gKzAwMDAsIFRyb25kIE15a2xlYnVz dCB3cm90ZToNCj4gPiBBZGRpbmcgRGF2aWQgSG93ZWxscyBhbmQgU3RldmUgRnJlbmNoIGFzIEkg YmVsaWV2ZSBib3RoIEFGUyBhbmQNCj4gPiBDSUZTDQo+ID4gaGF2ZSB0aGUgZXhhY3Qgc2FtZSBy ZXF1aXJlbWVudHMgYW5kIE5GUyBoZXJlLg0KPiA+IA0KPiA+IE9uIFdlZCwgMjAxNy0wMS0yNSBh dCAxMjo1NiArMTMwMCwgRXJpYyBXLiBCaWVkZXJtYW4gd3JvdGU6DQo+ID4gPiBUcm9uZCBNeWts ZWJ1c3QgPHRyb25kbXlAcHJpbWFyeWRhdGEuY29tPiB3cml0ZXM6DQo+ID4gPiANCj4gPiA+ID4g T24gV2VkLCAyMDE3LTAxLTI1IGF0IDEyOjI4ICsxMzAwLCBFcmljIFcuIEJpZWRlcm1hbiB3cm90 ZToNCj4gPiA+ID4gPiBXaXRoIHJlc3BlY3QgdG8gbmZzIGFuZCBhdXRvbW91bnRzLg0KPiA+ID4g PiA+IA0KPiA+ID4gPiA+IERvZXMgTkZTIGhhdmUgZGlmZmVyZW50IGF1dG9tb3VudCBiZWhhdmlv ciBiYXNlZCBvbiB0aGUgdXNlcg0KPiA+ID4gPiA+IHBlcmZvcm1pbmcgdGhlIGF1dG9tb3VudD8N Cj4gPiA+ID4gPiANCj4gPiA+ID4gPiBJZiBORlMgZG9lcyBub3QgaGF2ZSBkaWZmZXJlbnQgYXV0 b21vdW50IGJlaGF2aW9yIGRlcGVuZGluZw0KPiA+ID4gPiA+IG9uDQo+ID4gPiA+ID4gdGhlDQo+ ID4gPiA+ID4gdXNlcg0KPiA+ID4gPiA+IHdlIGp1c3QgdXNlIHRoZSBjcmVkcyBvZiB0aGUgb3Jp Z2luYWwgbW91bnRlciBvZiBORlM/DQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gSWYgTkZTIGRvZXMg aGF2ZSBkaWZmZXJlbnQgYXV0b21vdW50IGJlaGF2aW9yIGRlcGVuZGluZyBvbg0KPiA+ID4gPiA+ IHRoZQ0KPiA+ID4gPiA+IHVzZXINCj4gPiA+ID4gPiAob3VjaCEpIHdlIG5lZWQgdG8gZ28gdGhy b3VnaCB0aGUgY2FsbCBwYXRoIGFuZCBzZWUgd2hlcmUgaXQNCj4gPiA+ID4gPiBtYWtlcw0KPiA+ ID4gPiA+IHNlbnNlIHRvIG92ZXIgcmlkZSB0aGluZ3MgYW5kIHdoZXJlIGl0IGRvZXMgbm90Lg0K PiA+ID4gPiANCj4gPiA+ID4gVGhlIHJlYXNvbiB3aHkgdGhlIE5GUyBjbGllbnQgY3JlYXRlcyBh IG1vdW50cG9pbnQgaXMgYmVjYXVzZQ0KPiA+ID4gPiBvbg0KPiA+ID4gPiBlbnRlcmluZyBhIGRp cmVjdG9yeSwgaXQgZGV0ZWN0cyB0aGF0IHRoZXJlIGlzIGVpdGhlciBhIHNpbWlsYXINCj4gPiA+ ID4gbW91bnRwb2ludCBvbiB0aGUgc2VydmVyLCBvciB0aGVyZSBpcyBhIHJlZmVycmFsICh3aGlj aCBhY3RzDQo+ID4gPiA+IHNvcnQNCj4gPiA+ID4gb2YNCj4gPiA+ID4gbGlrZSBhIHN5bWxpbmss IGV4Y2VwdCBpdCBwb2ludHMgdG8gYSBwYXRoIG9uIG9uZSBvciBtb3JlDQo+ID4gPiA+IGRpZmZl cmVudA0KPiA+ID4gPiBORlMNCj4gPiA+ID4gc2VydmVycykuDQo+ID4gPiA+IFdpdGhvdXQgdGhh dCBtb3VudHBvaW50LCBtb3N0IHRoaW5ncyB3b3VsZCB3b3JrLCBidXQgdGhlIHVzZXINCj4gPiA+ ID4gd291bGQNCj4gPiA+ID4gZW5kDQo+ID4gPiA+IHVwIHNlZWluZyBuYXN0eSBub24tcG9zaXgg ZmVhdHVyZXMgbGlrZSBkdXBsaWNhdGUgaW5vZGUNCj4gPiA+ID4gbnVtYmVycy4NCj4gPiA+ID4g DQo+ID4gPiA+IFdlIGRvIG5vdCB3YW50IHRvIHVzZSBhbnkgY3JlZHMgb3RoZXIgdGhhbiB1c2Vy IGNyZWRzIGhlcmUsDQo+ID4gPiA+IGJlY2F1c2UNCj4gPiA+ID4gYXMNCj4gPiA+ID4gZmFyIGFz IHRoZSBzZWN1cml0eSBtb2RlbCBpcyBjb25jZXJuZWQsIHRoZSBwcm9jZXNzIGlzIGp1c3QNCj4g PiA+ID4gY3Jvc3NpbmcNCj4gPiA+ID4gaW50byBhbiBleGlzdGluZyBkaXJlY3RvcnkuDQo+ID4g PiANCj4gPiA+IEJ1dCBzZ2V0IG5lZWRzIGRpZmZlcmVudCBjcmVkcy4NCj4gPiA+IA0KPiA+ID4g QmVjYXVzZSB0aGUgdXNlciB3aG8gYXV0aG9yaXplcyB0aGUgbW91bnRpbmcgb2YgdGhlIGZpbGVz eXN0ZW0gaXMNCj4gPiA+IGRpZmZlcmVudCB0aGFuIHRoZSB1c2VyIHdobyBpcyBjcm9zc2luZyBp bnRvIHRoZSBuZXcgZmlsZXN5c3RlbS4NCj4gPiA+IA0KPiA+ID4gVGhlIGxvY2FsIHN5c3RlbSBu b3cgY2FyZXMgYWJvdXQgdGhhdCBkaXN0aW5jdGlvbiBldmVuIGlmIHRoZSBuZnMNCj4gPiA+IHNl cnZlcg0KPiA+ID4gZG9lcyBub3QuDQo+ID4gDQo+ID4gV2h5PyBUaGUgZmlsZXN5c3RlbSBpcyBh bHJlYWR5IG1vdW50ZWQuIFdlJ3JlIGNyZWF0aW5nIGEgbmV3DQo+ID4gbW91bnRwb2ludCwgYnV0 IHdlIGNvdWxkIGVxdWFsbHkgd2VsbCBqdXN0IHNheSAnc29kIHRoYXQnIGFuZA0KPiA+IGNyZWF0 ZSBhbg0KPiA+IG9yZGluYXJ5IGRpcmVjdG9yeS4gVGhlIHBlbmFsdHkgd291bGQgYmUgYWZvcmVt ZW50aW9uZWQgbm9uLXBvc2l4DQo+ID4gd2VpcmRuZXNzLg0KPiA+IA0KPiA+ID4gDQo+ID4gPiA+ ID4gU2V0aCB0aGUgZnVuZGFtZW50YWwgcHJvYmxlbSB3aXRoIHlvdXIgcGF0Y2ggd2FzIHRoYXQg eW91DQo+ID4gPiA+ID4gd2VyZQ0KPiA+ID4gPiA+IHBhdGNoaW5nDQo+ID4gPiA+ID4gYSBsb2Nh dGlvbiB0aGF0IGlzIHVzZWQgZm9yIG1vcmUganVzdCBtb3VudHMuDQo+ID4gPiA+ID4gDQo+ID4g PiA+ID4gSSBhbSBzdHJvbmdseSB3aXNoaW5nIHRoYXQgd2UgY291bGQganVzdCBjaGFuZ2UNCj4g PiA+ID4gPiBmb2xsb3dfYXV0b21vdW50DQo+ID4gPiA+ID4gZnJvbToNCj4gPiA+ID4gPiANCj4g PiA+ID4gPiANCj4gPiA+ID4gPiAJb2xkX2NyZWQgPSBvdmVycmlkZV9jcmVkcygmaW5pdF9jcmVk KTsNCj4gPiA+ID4gPiAJbW50ID0gcGF0aC0+ZGVudHJ5LT5kX29wLT5kX2F1dG9tb3VudChwYXRo KTsNCj4gPiA+ID4gPiAJcmV2ZXJ0X2NyZWRzKG9sZF9jcmVkKTsNCj4gPiA+ID4gPiANCj4gPiA+ ID4gPiB0bzoNCj4gPiA+ID4gPiANCj4gPiA+ID4gPiAJb2xkX2NyZWQgPSBvdmVycmlkZV9jcmVk cyhwYXRoLT5tbnQtPm1udF9zYi0+c19jcmVkKTsNCj4gPiA+ID4gPiAJbW50ID0gcGF0aC0+ZGVu dHJ5LT5kX29wLT5kX2F1dG9tb3VudChwYXRoKTsNCj4gPiA+ID4gPiAJcmV2ZXJ0X2NyZWRzKG9s ZF9jcmVkKTsNCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBBbmQgYWxsIHdpbGwgYmUgd2VsbCB3aXRo IG5mcy7CoMKgVGhhdCBkb2VzIHJlbWFpbiBwb3NzaWJsZS4NCj4gPiA+ID4gDQo+ID4gPiA+IE5v LiBUaGF0IHdvdWxkIGJyZWFrIHBlcm1pc3Npb25zIGNoZWNraW5nLiBTZWUgYWJvdmUuDQo+ID4g PiANCj4gPiA+IFRoZW4gd2UgbmVlZCB0byBsb29rIG11Y2ggaGFyZGVyIGF0IHRoZSBwZXJtaXNz aW9uIGNoZWNraW5nDQo+ID4gPiBtb2RlbCBvZiBkX2F1dG9tb3VudCBiZWNhdXNlIHdlIG5lZWQg dG8gcGVybWlzc2lvbiBjaGVjayBhZ2FpbnN0DQo+ID4gPiBib3RoIHNldHMgb2YgY3JlZHMuDQo+ IA0KPiBIb3cgYWJvdXQgc29tZXRoaW5nIGxpa2UgdGhpcz8gRXNzZW50aWFsbHksIHN0YXNoIHRo ZSBjcmVkcyB1c2VkIGF0DQo+IG1vdW50IHRpbWUgaW4gdGhlIHN1cGVyIGJsb2NrLCB0aGVuIGNy ZWF0ZSBhIHZmc19rZXJuX2F1dG9tb3VudCgpDQo+IGZ1bmN0aW9uIHdoaWNoIG92ZXJyaWRlcyB0 aGUgY3VycmVuZCBjcmVkcyB0aGVuIGNhbGxzDQo+IHZmc19rZXJuX21vdW50KCkuDQo+IA0KPiBP bmx5IGNvbXBpbGUgdGVzdGVkIHNvIGZhciwgYW5kIHByb2JhYmx5IGl0IHNob3VsZCBiZSBzcGxp dCB1cCBpbnRvDQo+IHNldmVyYWwgcGF0Y2hlcy4NCg0KPiBkaWZmIC0tZ2l0IGEvZnMvbmFtZXNw YWNlLmMgYi9mcy9uYW1lc3BhY2UuYw0KPiBpbmRleCA0ODdiYTMwYmI1YzYuLmRhN2U2ZGZhNTZj YiAxMDA2NDQNCj4gLS0tIGEvZnMvbmFtZXNwYWNlLmMNCj4gKysrIGIvZnMvbmFtZXNwYWNlLmMN Cj4gQEAgLTk4OSw2ICs5ODksMjEgQEAgdmZzX2tlcm5fbW91bnQoc3RydWN0IGZpbGVfc3lzdGVt X3R5cGUgKnR5cGUsDQo+IGludCBmbGFncywgY29uc3QgY2hhciAqbmFtZSwgdm9pZA0KPiDCoH0N Cj4gwqBFWFBPUlRfU1lNQk9MX0dQTCh2ZnNfa2Vybl9tb3VudCk7DQo+IMKgDQo+ICtzdHJ1Y3Qg dmZzbW91bnQgKg0KPiArdmZzX2tlcm5fYXV0b21vdW50KHN0cnVjdCBkZW50cnkgKmRlbnRyeSwg c3RydWN0IGZpbGVfc3lzdGVtX3R5cGUNCj4gKnR5cGUsDQo+ICsJCcKgwqDCoGludCBmbGFncywg Y29uc3QgY2hhciAqbmFtZSwgdm9pZCAqZGF0YSkNCj4gK3sNCj4gKwljb25zdCBzdHJ1Y3QgY3Jl ZCAqb2xkX2NyZWQ7DQo+ICsJc3RydWN0IHZmc21vdW50ICptbnQ7DQo+ICsNCj4gKwlvbGRfY3Jl ZCA9IG92ZXJyaWRlX2NyZWRzKGRlbnRyeS0+ZF9zYi0+c19jcmVkKTsNCj4gKwltbnQgPSB2ZnNf a2Vybl9tb3VudCh0eXBlLCBmbGFncywgbmFtZSwgZGF0YSk7DQo+ICsJcmV2ZXJ0X2NyZWRzKG9s ZF9jcmVkKTsNCj4gKw0KPiArCXJldHVybiBtbnQ7DQo+ICt9DQo+ICtFWFBPUlRfU1lNQk9MX0dQ TCh2ZnNfa2Vybl9hdXRvbW91bnQpOw0KPiArDQoNCk5vcGUuIEFzIEkgc2FpZCwgdGhlIGNhbGwg aW50byB0aGUgZmlsZXN5c3RlbSBuZWVkcyB0aGUgX3VzZXJfIGNyZWRzLg0KDQotLSA= ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-25 15:51 ` Trond Myklebust @ 2017-01-25 16:28 ` Seth Forshee 2017-02-01 6:36 ` Eric W. Biederman 0 siblings, 1 reply; 19+ messages in thread From: Seth Forshee @ 2017-01-25 16:28 UTC (permalink / raw) To: Trond Myklebust Cc: ebiederm, bfields, anna.schumaker, Steve French, linux-nfs, dhowells On Wed, Jan 25, 2017 at 03:51:30PM +0000, Trond Myklebust wrote: > On Wed, 2017-01-25 at 08:52 -0600, Seth Forshee wrote: > > On Wed, Jan 25, 2017 at 12:14:26AM +0000, Trond Myklebust wrote: > > > Adding David Howells and Steve French as I believe both AFS and > > > CIFS > > > have the exact same requirements and NFS here. > > > > > > On Wed, 2017-01-25 at 12:56 +1300, Eric W. Biederman wrote: > > > > 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. > > > > > > Why? The filesystem is already mounted. We're creating a new > > > mountpoint, but we could equally well just say 'sod that' and > > > create an > > > ordinary directory. The penalty would be aforementioned non-posix > > > weirdness. > > > > > > > > > > > > > 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. > > > > How about something like this? Essentially, stash the creds used at > > mount time in the super block, then create a vfs_kern_automount() > > function which overrides the currend creds then calls > > vfs_kern_mount(). > > > > Only compile tested so far, and probably it should be split up into > > several patches. > > > diff --git a/fs/namespace.c b/fs/namespace.c > > index 487ba30bb5c6..da7e6dfa56cb 100644 > > --- a/fs/namespace.c > > +++ b/fs/namespace.c > > @@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, > > int flags, const char *name, void > > } > > EXPORT_SYMBOL_GPL(vfs_kern_mount); > > > > +struct vfsmount * > > +vfs_kern_automount(struct dentry *dentry, struct file_system_type > > *type, > > + int flags, const char *name, void *data) > > +{ > > + const struct cred *old_cred; > > + struct vfsmount *mnt; > > + > > + old_cred = override_creds(dentry->d_sb->s_cred); > > + mnt = vfs_kern_mount(type, flags, name, data); > > + revert_creds(old_cred); > > + > > + return mnt; > > +} > > +EXPORT_SYMBOL_GPL(vfs_kern_automount); > > + > > Nope. As I said, the call into the filesystem needs the _user_ creds. Okay. I thought that perhaps the user's creds were only needed when looking up the mountpoint, and that after that it might be okay to switch the creds. I guess not. Thanks, Seth ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials 2017-01-25 16:28 ` Seth Forshee @ 2017-02-01 6:36 ` Eric W. Biederman 2017-02-01 6:38 ` [REVIEW][PATCH] fs: Better permission checking for submounts Eric W. Biederman 0 siblings, 1 reply; 19+ messages in thread From: Eric W. Biederman @ 2017-02-01 6:36 UTC (permalink / raw) To: Seth Forshee Cc: Trond Myklebust, bfields, anna.schumaker, Steve French, linux-nfs, dhowells Seth Forshee <seth.forshee@canonical.com> writes: > On Wed, Jan 25, 2017 at 03:51:30PM +0000, Trond Myklebust wrote: >> On Wed, 2017-01-25 at 08:52 -0600, Seth Forshee wrote: >> > On Wed, Jan 25, 2017 at 12:14:26AM +0000, Trond Myklebust wrote: >> > > Adding David Howells and Steve French as I believe both AFS and >> > > CIFS >> > > have the exact same requirements and NFS here. >> > > >> > > On Wed, 2017-01-25 at 12:56 +1300, Eric W. Biederman wrote: >> > > > 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. >> > > >> > > Why? The filesystem is already mounted. We're creating a new >> > > mountpoint, but we could equally well just say 'sod that' and >> > > create an >> > > ordinary directory. The penalty would be aforementioned non-posix >> > > weirdness. >> > > >> > > > >> > > > > > 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. >> > >> > How about something like this? Essentially, stash the creds used at >> > mount time in the super block, then create a vfs_kern_automount() >> > function which overrides the currend creds then calls >> > vfs_kern_mount(). >> > >> > Only compile tested so far, and probably it should be split up into >> > several patches. >> >> > diff --git a/fs/namespace.c b/fs/namespace.c >> > index 487ba30bb5c6..da7e6dfa56cb 100644 >> > --- a/fs/namespace.c >> > +++ b/fs/namespace.c >> > @@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, >> > int flags, const char *name, void >> > } >> > EXPORT_SYMBOL_GPL(vfs_kern_mount); >> > >> > +struct vfsmount * >> > +vfs_kern_automount(struct dentry *dentry, struct file_system_type >> > *type, >> > + int flags, const char *name, void *data) >> > +{ >> > + const struct cred *old_cred; >> > + struct vfsmount *mnt; >> > + >> > + old_cred = override_creds(dentry->d_sb->s_cred); >> > + mnt = vfs_kern_mount(type, flags, name, data); >> > + revert_creds(old_cred); >> > + >> > + return mnt; >> > +} >> > +EXPORT_SYMBOL_GPL(vfs_kern_automount); >> > + >> >> Nope. As I said, the call into the filesystem needs the _user_ creds. > > Okay. I thought that perhaps the user's creds were only needed when > looking up the mountpoint, and that after that it might be okay to > switch the creds. I guess not. So I agree that for the case of submounts that we don't need an additional permission check as the initial mount of the filesystem is that permission check already. I can see some interesting corner cases where a permission check might be required at the point when we have unprivileged mounts with submounts in the mix. As sget might find a filesystem that was already mounted elsewhere. But we don't have to face that case today. What is needs to be fixed are the permission checks cause problems for existing filesystems with submounts. What we need to fix those is knowledge the mount is happening for a submount/automount. At which point it is simple to turn off the permission check. Knowledge that a mount is a submount and not the ordinary kind of mount is going to require something like Seth's patch. So after having gone off to a corner and thought about this I believe I have come up with a minimal patch that makes things good for everyone. Eric ^ permalink raw reply [flat|nested] 19+ messages in thread
* [REVIEW][PATCH] fs: Better permission checking for submounts 2017-02-01 6:36 ` Eric W. Biederman @ 2017-02-01 6:38 ` Eric W. Biederman 2017-02-01 13:28 ` Trond Myklebust 2017-02-01 13:38 ` Seth Forshee 0 siblings, 2 replies; 19+ messages in thread From: Eric W. Biederman @ 2017-02-01 6:38 UTC (permalink / raw) To: Seth Forshee Cc: Trond Myklebust, bfields, anna.schumaker, Steve French, linux-nfs, dhowells, linux-fsdevel To support unprivileged users mounting filesystems two permission checks have to be performed: a test to see if the user allowed to create a mount in the mount namespace, and a test to see if the user is allowed to access the specified filesystem. The automount case is special in that mounting the original filesystem grants permission to mount the sub-filesystems, to any user who happens to stumble across the their mountpoint and satisfies the ordinary filesystem permission checks. Attempting to handle the automount case by using override_creds almost works. It preserves the idea that permission to mount the original filesystem is permission to mount the sub-filesystem. Unfortunately using override_creds messes up the filesystems ordinary permission checks. Solve this by being explicit that a mount is a submount by introducing vfs_submount, and using it where appropriate. vfs_submount uses a new mount internal mount flags MS_SUBMOUNT, to let sget and friends know that a mount is a submount so they can take appropriate action. sget and sget_userns are modified to not perform any permission checks on submounts. follow_automount is modified to stop using override_creds as that has proven problemantic. do_mount is modified to always remove the new MS_SUBMOUNT flag so that we know userspace will never by able to specify it. autofs4 is modified to stop using current_real_cred that was put in there to handle the previous version of submount permission checking. cifs is modified to pass the mountpoint all of the way down to vfs_submount. debugfs is modified to pass the mountpoint all of the way down to trace_automount by adding a new parameter. To make this change easier a new typedef debugfs_automount_t is introduced to capture the type of the debugfs automount function. Cc: stable@vger.kernel.org Fixes: 069d5ac9ae0d ("autofs: Fix automounts by using current_real_cred()->uid") Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems creds") Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- fs/afs/mntpt.c | 2 +- fs/autofs4/waitq.c | 4 ++-- fs/cifs/cifs_dfs_ref.c | 7 ++++--- fs/debugfs/inode.c | 8 ++++---- fs/namei.c | 3 --- fs/namespace.c | 17 ++++++++++++++++- fs/nfs/namespace.c | 2 +- fs/nfs/nfs4namespace.c | 2 +- fs/super.c | 13 ++++++++++--- include/linux/debugfs.h | 3 ++- include/linux/mount.h | 3 +++ include/uapi/linux/fs.h | 1 + kernel/trace/trace.c | 4 ++-- 13 files changed, 47 insertions(+), 22 deletions(-) diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c index 81dd075356b9..d4fb0afc0097 100644 --- a/fs/afs/mntpt.c +++ b/fs/afs/mntpt.c @@ -202,7 +202,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt) /* try and do the mount */ _debug("--- attempting mount %s -o %s ---", devname, options); - mnt = vfs_kern_mount(&afs_fs_type, 0, devname, options); + mnt = vfs_submount(mntpt, &afs_fs_type, devname, options); _debug("--- mount result %p ---", mnt); free_page((unsigned long) devname); diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c index 1278335ce366..79fbd85db4ba 100644 --- a/fs/autofs4/waitq.c +++ b/fs/autofs4/waitq.c @@ -436,8 +436,8 @@ int autofs4_wait(struct autofs_sb_info *sbi, memcpy(&wq->name, &qstr, sizeof(struct qstr)); wq->dev = autofs4_get_dev(sbi); wq->ino = autofs4_get_ino(sbi); - wq->uid = current_real_cred()->uid; - wq->gid = current_real_cred()->gid; + wq->uid = current_cred()->uid; + wq->gid = current_cred()->gid; wq->pid = pid; wq->tgid = tgid; wq->status = -EINTR; /* Status return if interrupted */ diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c index ec9dbbcca3b9..9156be545b0f 100644 --- a/fs/cifs/cifs_dfs_ref.c +++ b/fs/cifs/cifs_dfs_ref.c @@ -245,7 +245,8 @@ char *cifs_compose_mount_options(const char *sb_mountdata, * @fullpath: full path in UNC format * @ref: server's referral */ -static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb, +static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt, + struct cifs_sb_info *cifs_sb, const char *fullpath, const struct dfs_info3_param *ref) { struct vfsmount *mnt; @@ -259,7 +260,7 @@ static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb, if (IS_ERR(mountdata)) return (struct vfsmount *)mountdata; - mnt = vfs_kern_mount(&cifs_fs_type, 0, devname, mountdata); + mnt = vfs_submount(mntpt, &cifs_fs_type, devname, mountdata); kfree(mountdata); kfree(devname); return mnt; @@ -334,7 +335,7 @@ static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt) mnt = ERR_PTR(-EINVAL); break; } - mnt = cifs_dfs_do_refmount(cifs_sb, + mnt = cifs_dfs_do_refmount(mntpt, cifs_sb, full_path, referrals + i); cifs_dbg(FYI, "%s: cifs_dfs_do_refmount:%s , mnt:%p\n", __func__, referrals[i].node_name, mnt); diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index f17fcf89e18e..1e30f74a9527 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -187,9 +187,9 @@ static const struct super_operations debugfs_super_operations = { static struct vfsmount *debugfs_automount(struct path *path) { - struct vfsmount *(*f)(void *); - f = (struct vfsmount *(*)(void *))path->dentry->d_fsdata; - return f(d_inode(path->dentry)->i_private); + debugfs_automount_t f; + f = (debugfs_automount_t)path->dentry->d_fsdata; + return f(path->dentry, d_inode(path->dentry)->i_private); } static const struct dentry_operations debugfs_dops = { @@ -504,7 +504,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir); */ struct dentry *debugfs_create_automount(const char *name, struct dentry *parent, - struct vfsmount *(*f)(void *), + debugfs_automount_t f, void *data) { struct dentry *dentry = start_creating(name, parent); diff --git a/fs/namei.c b/fs/namei.c index 6fa3e9138fe4..da689c9c005e 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1100,7 +1100,6 @@ static int follow_automount(struct path *path, struct nameidata *nd, bool *need_mntput) { struct vfsmount *mnt; - const struct cred *old_cred; int err; if (!path->dentry->d_op || !path->dentry->d_op->d_automount) @@ -1129,9 +1128,7 @@ static int follow_automount(struct path *path, struct nameidata *nd, if (nd->total_link_count >= 40) return -ELOOP; - old_cred = override_creds(&init_cred); mnt = path->dentry->d_op->d_automount(path); - revert_creds(old_cred); if (IS_ERR(mnt)) { /* * The filesystem is allowed to return -EISDIR here to indicate diff --git a/fs/namespace.c b/fs/namespace.c index 487ba30bb5c6..089a6b23135a 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void } EXPORT_SYMBOL_GPL(vfs_kern_mount); +struct vfsmount * +vfs_submount(const struct dentry *mountpoint, struct file_system_type *type, + const char *name, void *data) +{ + /* Until it is worked out how to pass the user namespace + * through from the parent mount to the submount don't support + * unprivileged mounts with submounts. + */ + if (mountpoint->d_sb->s_user_ns != &init_user_ns) + return ERR_PTR(-EPERM); + + return vfs_kern_mount(type, MS_SUBMOUNT, name, data); +} +EXPORT_SYMBOL_GPL(vfs_submount); + static struct mount *clone_mnt(struct mount *old, struct dentry *root, int flag) { @@ -2794,7 +2809,7 @@ long do_mount(const char *dev_name, const char __user *dir_name, flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_BORN | MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT | - MS_STRICTATIME | MS_NOREMOTELOCK); + MS_STRICTATIME | MS_NOREMOTELOCK | MS_SUBMOUNT); if (flags & MS_REMOUNT) retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags, diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c index 5551e8ef67fd..e49d831c4e85 100644 --- a/fs/nfs/namespace.c +++ b/fs/nfs/namespace.c @@ -226,7 +226,7 @@ static struct vfsmount *nfs_do_clone_mount(struct nfs_server *server, const char *devname, struct nfs_clone_mount *mountdata) { - return vfs_kern_mount(&nfs_xdev_fs_type, 0, devname, mountdata); + return vfs_submount(mountdata->dentry, &nfs_xdev_fs_type, devname, mountdata); } /** diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c index d21104912676..d8b040bd9814 100644 --- a/fs/nfs/nfs4namespace.c +++ b/fs/nfs/nfs4namespace.c @@ -279,7 +279,7 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata, mountdata->hostname, mountdata->mnt_path); - mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata); + mnt = vfs_submount(mountdata->dentry, &nfs4_referral_fs_type, page, mountdata); if (!IS_ERR(mnt)) break; } diff --git a/fs/super.c b/fs/super.c index 1709ed029a2c..4185844f7a12 100644 --- a/fs/super.c +++ b/fs/super.c @@ -469,7 +469,7 @@ struct super_block *sget_userns(struct file_system_type *type, struct super_block *old; int err; - if (!(flags & MS_KERNMOUNT) && + if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT)) && !(type->fs_flags & FS_USERNS_MOUNT) && !capable(CAP_SYS_ADMIN)) return ERR_PTR(-EPERM); @@ -499,7 +499,7 @@ struct super_block *sget_userns(struct file_system_type *type, } if (!s) { spin_unlock(&sb_lock); - s = alloc_super(type, flags, user_ns); + s = alloc_super(type, (flags & ~MS_SUBMOUNT), user_ns); if (!s) return ERR_PTR(-ENOMEM); goto retry; @@ -540,8 +540,15 @@ struct super_block *sget(struct file_system_type *type, { struct user_namespace *user_ns = current_user_ns(); + /* We don't yet pass the user namespace of the parent + * mount through to here so always use &init_user_ns + * until that changes. + */ + if (flags & MS_SUBMOUNT) + user_ns = &init_user_ns; + /* Ensure the requestor has permissions over the target filesystem */ - if (!(flags & MS_KERNMOUNT) && !ns_capable(user_ns, CAP_SYS_ADMIN)) + if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT)) && !ns_capable(user_ns, CAP_SYS_ADMIN)) return ERR_PTR(-EPERM); return sget_userns(type, test, set, flags, user_ns, data); diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 014cc564d1c4..233006be30aa 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -97,9 +97,10 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent); struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent, const char *dest); +typedef struct vfsmount *(*debugfs_automount_t)(struct dentry *, void *); struct dentry *debugfs_create_automount(const char *name, struct dentry *parent, - struct vfsmount *(*f)(void *), + debugfs_automount_t f, void *data); void debugfs_remove(struct dentry *dentry); diff --git a/include/linux/mount.h b/include/linux/mount.h index c6f55158d5e5..8e0352af06b7 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -90,6 +90,9 @@ struct file_system_type; extern struct vfsmount *vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void *data); +extern struct vfsmount *vfs_submount(const struct dentry *mountpoint, + struct file_system_type *type, + const char *name, void *data); extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list); extern void mark_mounts_for_expiry(struct list_head *mounts); diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index 36da93fbf188..048a85e9f017 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -132,6 +132,7 @@ struct inodes_stat_t { #define MS_LAZYTIME (1<<25) /* Update the on-disk [acm]times lazily */ /* These sb flags are internal to the kernel */ +#define MS_SUBMOUNT (1<<26) #define MS_NOREMOTELOCK (1<<27) #define MS_NOSEC (1<<28) #define MS_BORN (1<<29) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index d7449783987a..310f0ea0d1a2 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -7503,7 +7503,7 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer) ftrace_init_tracefs(tr, d_tracer); } -static struct vfsmount *trace_automount(void *ingore) +static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore) { struct vfsmount *mnt; struct file_system_type *type; @@ -7516,7 +7516,7 @@ static struct vfsmount *trace_automount(void *ingore) type = get_fs_type("tracefs"); if (!type) return NULL; - mnt = vfs_kern_mount(type, 0, "tracefs", NULL); + mnt = vfs_submount(mntpt, type, "tracefs", NULL); put_filesystem(type); if (IS_ERR(mnt)) return NULL; -- 2.10.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [REVIEW][PATCH] fs: Better permission checking for submounts 2017-02-01 6:38 ` [REVIEW][PATCH] fs: Better permission checking for submounts Eric W. Biederman @ 2017-02-01 13:28 ` Trond Myklebust 2017-02-01 13:38 ` Seth Forshee 1 sibling, 0 replies; 19+ messages in thread From: Trond Myklebust @ 2017-02-01 13:28 UTC (permalink / raw) To: ebiederm, seth.forshee Cc: bfields, anna.schumaker, Steve French, linux-nfs, dhowells, linux-fsdevel On Wed, 2017-02-01 at 19:38 +1300, Eric W. Biederman wrote: > To support unprivileged users mounting filesystems two permission > checks have to be performed: a test to see if the user allowed to > create a mount in the mount namespace, and a test to see if > the user is allowed to access the specified filesystem. > > The automount case is special in that mounting the original > filesystem > grants permission to mount the sub-filesystems, to any user who > happens to stumble across the their mountpoint and satisfies the > ordinary filesystem permission checks. > > Attempting to handle the automount case by using override_creds > almost works. It preserves the idea that permission to mount > the original filesystem is permission to mount the sub-filesystem. > Unfortunately using override_creds messes up the filesystems > ordinary permission checks. > > Solve this by being explicit that a mount is a submount by > introducing > vfs_submount, and using it where appropriate. > > vfs_submount uses a new mount internal mount flags MS_SUBMOUNT, to > let > sget and friends know that a mount is a submount so they can take > appropriate > action. > > sget and sget_userns are modified to not perform any permission > checks > on submounts. > > follow_automount is modified to stop using override_creds as that > has proven problemantic. > > do_mount is modified to always remove the new MS_SUBMOUNT flag so > that we know userspace will never by able to specify it. > > autofs4 is modified to stop using current_real_cred that was put in > there to handle the previous version of submount permission checking. > > cifs is modified to pass the mountpoint all of the way down to > vfs_submount. > > debugfs is modified to pass the mountpoint all of the way down to > trace_automount by adding a new parameter. To make this change > easier > a new typedef debugfs_automount_t is introduced to capture the type > of > the debugfs automount function. > > Cc: stable@vger.kernel.org > Fixes: 069d5ac9ae0d ("autofs: Fix automounts by using > current_real_cred()->uid") > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems > creds") > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > fs/afs/mntpt.c | 2 +- > fs/autofs4/waitq.c | 4 ++-- > fs/cifs/cifs_dfs_ref.c | 7 ++++--- > fs/debugfs/inode.c | 8 ++++---- > fs/namei.c | 3 --- > fs/namespace.c | 17 ++++++++++++++++- > fs/nfs/namespace.c | 2 +- > fs/nfs/nfs4namespace.c | 2 +- > fs/super.c | 13 ++++++++++--- > include/linux/debugfs.h | 3 ++- > include/linux/mount.h | 3 +++ > include/uapi/linux/fs.h | 1 + > kernel/trace/trace.c | 4 ++-- > 13 files changed, 47 insertions(+), 22 deletions(-) > > diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c > index 81dd075356b9..d4fb0afc0097 100644 > --- a/fs/afs/mntpt.c > +++ b/fs/afs/mntpt.c > @@ -202,7 +202,7 @@ static struct vfsmount > *afs_mntpt_do_automount(struct dentry *mntpt) > > /* try and do the mount */ > _debug("--- attempting mount %s -o %s ---", devname, > options); > - mnt = vfs_kern_mount(&afs_fs_type, 0, devname, options); > + mnt = vfs_submount(mntpt, &afs_fs_type, devname, options); > _debug("--- mount result %p ---", mnt); > > free_page((unsigned long) devname); > diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c > index 1278335ce366..79fbd85db4ba 100644 > --- a/fs/autofs4/waitq.c > +++ b/fs/autofs4/waitq.c > @@ -436,8 +436,8 @@ int autofs4_wait(struct autofs_sb_info *sbi, > memcpy(&wq->name, &qstr, sizeof(struct qstr)); > wq->dev = autofs4_get_dev(sbi); > wq->ino = autofs4_get_ino(sbi); > - wq->uid = current_real_cred()->uid; > - wq->gid = current_real_cred()->gid; > + wq->uid = current_cred()->uid; > + wq->gid = current_cred()->gid; > wq->pid = pid; > wq->tgid = tgid; > wq->status = -EINTR; /* Status return if interrupted > */ > diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c > index ec9dbbcca3b9..9156be545b0f 100644 > --- a/fs/cifs/cifs_dfs_ref.c > +++ b/fs/cifs/cifs_dfs_ref.c > @@ -245,7 +245,8 @@ char *cifs_compose_mount_options(const char > *sb_mountdata, > * @fullpath: full path in UNC format > * @ref: server's referral > */ > -static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info > *cifs_sb, > +static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt, > + struct cifs_sb_info *cifs_sb, > const char *fullpath, const struct dfs_info3_param > *ref) > { > struct vfsmount *mnt; > @@ -259,7 +260,7 @@ static struct vfsmount > *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb, > if (IS_ERR(mountdata)) > return (struct vfsmount *)mountdata; > > - mnt = vfs_kern_mount(&cifs_fs_type, 0, devname, mountdata); > + mnt = vfs_submount(mntpt, &cifs_fs_type, devname, > mountdata); > kfree(mountdata); > kfree(devname); > return mnt; > @@ -334,7 +335,7 @@ static struct vfsmount > *cifs_dfs_do_automount(struct dentry *mntpt) > mnt = ERR_PTR(-EINVAL); > break; > } > - mnt = cifs_dfs_do_refmount(cifs_sb, > + mnt = cifs_dfs_do_refmount(mntpt, cifs_sb, > full_path, referrals + i); > cifs_dbg(FYI, "%s: cifs_dfs_do_refmount:%s , > mnt:%p\n", > __func__, referrals[i].node_name, mnt); > diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c > index f17fcf89e18e..1e30f74a9527 100644 > --- a/fs/debugfs/inode.c > +++ b/fs/debugfs/inode.c > @@ -187,9 +187,9 @@ static const struct super_operations > debugfs_super_operations = { > > static struct vfsmount *debugfs_automount(struct path *path) > { > - struct vfsmount *(*f)(void *); > - f = (struct vfsmount *(*)(void *))path->dentry->d_fsdata; > - return f(d_inode(path->dentry)->i_private); > + debugfs_automount_t f; > + f = (debugfs_automount_t)path->dentry->d_fsdata; > + return f(path->dentry, d_inode(path->dentry)->i_private); > } > > static const struct dentry_operations debugfs_dops = { > @@ -504,7 +504,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir); > */ > struct dentry *debugfs_create_automount(const char *name, > struct dentry *parent, > - struct vfsmount *(*f)(void > *), > + debugfs_automount_t f, > void *data) > { > struct dentry *dentry = start_creating(name, parent); > diff --git a/fs/namei.c b/fs/namei.c > index 6fa3e9138fe4..da689c9c005e 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -1100,7 +1100,6 @@ static int follow_automount(struct path *path, > struct nameidata *nd, > bool *need_mntput) > { > struct vfsmount *mnt; > - const struct cred *old_cred; > int err; > > if (!path->dentry->d_op || !path->dentry->d_op->d_automount) > @@ -1129,9 +1128,7 @@ static int follow_automount(struct path *path, > struct nameidata *nd, > if (nd->total_link_count >= 40) > return -ELOOP; > > - old_cred = override_creds(&init_cred); > mnt = path->dentry->d_op->d_automount(path); > - revert_creds(old_cred); > if (IS_ERR(mnt)) { > /* > * The filesystem is allowed to return -EISDIR here > to indicate > diff --git a/fs/namespace.c b/fs/namespace.c > index 487ba30bb5c6..089a6b23135a 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, > int flags, const char *name, void > } > EXPORT_SYMBOL_GPL(vfs_kern_mount); > > +struct vfsmount * > +vfs_submount(const struct dentry *mountpoint, struct > file_system_type *type, > + const char *name, void *data) > +{ > + /* Until it is worked out how to pass the user namespace > + * through from the parent mount to the submount don't > support > + * unprivileged mounts with submounts. > + */ > + if (mountpoint->d_sb->s_user_ns != &init_user_ns) > + return ERR_PTR(-EPERM); > + > + return vfs_kern_mount(type, MS_SUBMOUNT, name, data); > +} > +EXPORT_SYMBOL_GPL(vfs_submount); > + > static struct mount *clone_mnt(struct mount *old, struct dentry > *root, > int flag) > { > @@ -2794,7 +2809,7 @@ long do_mount(const char *dev_name, const char > __user *dir_name, > > flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | > MS_BORN | > MS_NOATIME | MS_NODIRATIME | MS_RELATIME| > MS_KERNMOUNT | > - MS_STRICTATIME | MS_NOREMOTELOCK); > + MS_STRICTATIME | MS_NOREMOTELOCK | MS_SUBMOUNT); > > if (flags & MS_REMOUNT) > retval = do_remount(&path, flags & ~MS_REMOUNT, > mnt_flags, > diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c > index 5551e8ef67fd..e49d831c4e85 100644 > --- a/fs/nfs/namespace.c > +++ b/fs/nfs/namespace.c > @@ -226,7 +226,7 @@ static struct vfsmount *nfs_do_clone_mount(struct > nfs_server *server, > const char *devname, > struct nfs_clone_mount > *mountdata) > { > - return vfs_kern_mount(&nfs_xdev_fs_type, 0, devname, > mountdata); > + return vfs_submount(mountdata->dentry, &nfs_xdev_fs_type, > devname, mountdata); > } > > /** > diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c > index d21104912676..d8b040bd9814 100644 > --- a/fs/nfs/nfs4namespace.c > +++ b/fs/nfs/nfs4namespace.c > @@ -279,7 +279,7 @@ static struct vfsmount *try_location(struct > nfs_clone_mount *mountdata, > mountdata->hostname, > mountdata->mnt_path); > > - mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, > page, mountdata); > + mnt = vfs_submount(mountdata->dentry, > &nfs4_referral_fs_type, page, mountdata); > if (!IS_ERR(mnt)) > break; > } > diff --git a/fs/super.c b/fs/super.c > index 1709ed029a2c..4185844f7a12 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -469,7 +469,7 @@ struct super_block *sget_userns(struct > file_system_type *type, > struct super_block *old; > int err; > > - if (!(flags & MS_KERNMOUNT) && > + if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT)) && > !(type->fs_flags & FS_USERNS_MOUNT) && > !capable(CAP_SYS_ADMIN)) > return ERR_PTR(-EPERM); > @@ -499,7 +499,7 @@ struct super_block *sget_userns(struct > file_system_type *type, > } > if (!s) { > spin_unlock(&sb_lock); > - s = alloc_super(type, flags, user_ns); > + s = alloc_super(type, (flags & ~MS_SUBMOUNT), > user_ns); > if (!s) > return ERR_PTR(-ENOMEM); > goto retry; > @@ -540,8 +540,15 @@ struct super_block *sget(struct file_system_type > *type, > { > struct user_namespace *user_ns = current_user_ns(); > > + /* We don't yet pass the user namespace of the parent > + * mount through to here so always use &init_user_ns > + * until that changes. > + */ > + if (flags & MS_SUBMOUNT) > + user_ns = &init_user_ns; > + > /* Ensure the requestor has permissions over the target > filesystem */ > - if (!(flags & MS_KERNMOUNT) && !ns_capable(user_ns, > CAP_SYS_ADMIN)) > + if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT)) && > !ns_capable(user_ns, CAP_SYS_ADMIN)) > return ERR_PTR(-EPERM); > > return sget_userns(type, test, set, flags, user_ns, data); > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h > index 014cc564d1c4..233006be30aa 100644 > --- a/include/linux/debugfs.h > +++ b/include/linux/debugfs.h > @@ -97,9 +97,10 @@ struct dentry *debugfs_create_dir(const char > *name, struct dentry *parent); > struct dentry *debugfs_create_symlink(const char *name, struct > dentry *parent, > const char *dest); > > +typedef struct vfsmount *(*debugfs_automount_t)(struct dentry *, > void *); > struct dentry *debugfs_create_automount(const char *name, > struct dentry *parent, > - struct vfsmount *(*f)(void > *), > + debugfs_automount_t f, > void *data); > > void debugfs_remove(struct dentry *dentry); > diff --git a/include/linux/mount.h b/include/linux/mount.h > index c6f55158d5e5..8e0352af06b7 100644 > --- a/include/linux/mount.h > +++ b/include/linux/mount.h > @@ -90,6 +90,9 @@ struct file_system_type; > extern struct vfsmount *vfs_kern_mount(struct file_system_type > *type, > int flags, const char *name, > void *data); > +extern struct vfsmount *vfs_submount(const struct dentry > *mountpoint, > + struct file_system_type *type, > + const char *name, void *data); > > extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head > *expiry_list); > extern void mark_mounts_for_expiry(struct list_head *mounts); > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > index 36da93fbf188..048a85e9f017 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -132,6 +132,7 @@ struct inodes_stat_t { > #define MS_LAZYTIME (1<<25) /* Update the on-disk [acm]times > lazily */ > > /* These sb flags are internal to the kernel */ > +#define MS_SUBMOUNT (1<<26) > #define MS_NOREMOTELOCK (1<<27) > #define MS_NOSEC (1<<28) > #define MS_BORN (1<<29) > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index d7449783987a..310f0ea0d1a2 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -7503,7 +7503,7 @@ init_tracer_tracefs(struct trace_array *tr, > struct dentry *d_tracer) > ftrace_init_tracefs(tr, d_tracer); > } > > -static struct vfsmount *trace_automount(void *ingore) > +static struct vfsmount *trace_automount(struct dentry *mntpt, void > *ingore) > { > struct vfsmount *mnt; > struct file_system_type *type; > @@ -7516,7 +7516,7 @@ static struct vfsmount *trace_automount(void > *ingore) > type = get_fs_type("tracefs"); > if (!type) > return NULL; > - mnt = vfs_kern_mount(type, 0, "tracefs", NULL); > + mnt = vfs_submount(mntpt, type, "tracefs", NULL); > put_filesystem(type); > if (IS_ERR(mnt)) > return NULL; Yes. That looks like it might be workable. Reviewed-by: Trond Myklebust <trond.myklebust@primarydata.com> -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [REVIEW][PATCH] fs: Better permission checking for submounts @ 2017-02-01 13:28 ` Trond Myklebust 0 siblings, 0 replies; 19+ messages in thread From: Trond Myklebust @ 2017-02-01 13:28 UTC (permalink / raw) To: ebiederm, seth.forshee Cc: bfields, anna.schumaker, Steve French, linux-nfs, dhowells, linux-fsdevel T24gV2VkLCAyMDE3LTAyLTAxIGF0IDE5OjM4ICsxMzAwLCBFcmljIFcuIEJpZWRlcm1hbiB3cm90 ZToNCj4gVG8gc3VwcG9ydCB1bnByaXZpbGVnZWQgdXNlcnMgbW91bnRpbmcgZmlsZXN5c3RlbXMg dHdvIHBlcm1pc3Npb24NCj4gY2hlY2tzIGhhdmUgdG8gYmUgcGVyZm9ybWVkOiBhIHRlc3QgdG8g c2VlIGlmIHRoZSB1c2VyIGFsbG93ZWQgdG8NCj4gY3JlYXRlIGEgbW91bnQgaW4gdGhlIG1vdW50 IG5hbWVzcGFjZSwgYW5kIGEgdGVzdCB0byBzZWUgaWYNCj4gdGhlIHVzZXIgaXMgYWxsb3dlZCB0 byBhY2Nlc3MgdGhlIHNwZWNpZmllZCBmaWxlc3lzdGVtLg0KPiANCj4gVGhlIGF1dG9tb3VudCBj YXNlIGlzIHNwZWNpYWwgaW4gdGhhdCBtb3VudGluZyB0aGUgb3JpZ2luYWwNCj4gZmlsZXN5c3Rl bQ0KPiBncmFudHMgcGVybWlzc2lvbiB0byBtb3VudCB0aGUgc3ViLWZpbGVzeXN0ZW1zLCB0byBh bnkgdXNlciB3aG8NCj4gaGFwcGVucyB0byBzdHVtYmxlIGFjcm9zcyB0aGUgdGhlaXIgbW91bnRw b2ludCBhbmQgc2F0aXNmaWVzIHRoZQ0KPiBvcmRpbmFyeSBmaWxlc3lzdGVtIHBlcm1pc3Npb24g Y2hlY2tzLg0KPiANCj4gQXR0ZW1wdGluZyB0byBoYW5kbGUgdGhlIGF1dG9tb3VudCBjYXNlIGJ5 IHVzaW5nIG92ZXJyaWRlX2NyZWRzDQo+IGFsbW9zdCB3b3Jrcy7CoMKgSXQgcHJlc2VydmVzIHRo ZSBpZGVhIHRoYXQgcGVybWlzc2lvbiB0byBtb3VudA0KPiB0aGUgb3JpZ2luYWwgZmlsZXN5c3Rl bSBpcyBwZXJtaXNzaW9uIHRvIG1vdW50IHRoZSBzdWItZmlsZXN5c3RlbS4NCj4gVW5mb3J0dW5h dGVseSB1c2luZyBvdmVycmlkZV9jcmVkcyBtZXNzZXMgdXAgdGhlIGZpbGVzeXN0ZW1zDQo+IG9y ZGluYXJ5IHBlcm1pc3Npb24gY2hlY2tzLg0KPiANCj4gU29sdmUgdGhpcyBieSBiZWluZyBleHBs aWNpdCB0aGF0IGEgbW91bnQgaXMgYSBzdWJtb3VudCBieQ0KPiBpbnRyb2R1Y2luZw0KPiB2ZnNf c3VibW91bnQsIGFuZCB1c2luZyBpdCB3aGVyZSBhcHByb3ByaWF0ZS4NCj4gDQo+IHZmc19zdWJt b3VudCB1c2VzIGEgbmV3IG1vdW50IGludGVybmFsIG1vdW50IGZsYWdzIE1TX1NVQk1PVU5ULCB0 bw0KPiBsZXQNCj4gc2dldCBhbmQgZnJpZW5kcyBrbm93IHRoYXQgYSBtb3VudCBpcyBhIHN1Ym1v dW50IHNvIHRoZXkgY2FuIHRha2UNCj4gYXBwcm9wcmlhdGUNCj4gYWN0aW9uLg0KPiANCj4gc2dl dCBhbmQgc2dldF91c2VybnMgYXJlIG1vZGlmaWVkIHRvIG5vdCBwZXJmb3JtIGFueSBwZXJtaXNz aW9uDQo+IGNoZWNrcw0KPiBvbiBzdWJtb3VudHMuDQo+IA0KPiBmb2xsb3dfYXV0b21vdW50IGlz IG1vZGlmaWVkIHRvIHN0b3AgdXNpbmcgb3ZlcnJpZGVfY3JlZHMgYXMgdGhhdA0KPiBoYXMgcHJv dmVuIHByb2JsZW1hbnRpYy4NCj4gDQo+IGRvX21vdW50IGlzIG1vZGlmaWVkIHRvIGFsd2F5cyBy ZW1vdmUgdGhlIG5ldyBNU19TVUJNT1VOVCBmbGFnIHNvDQo+IHRoYXQgd2Uga25vdyB1c2Vyc3Bh Y2Ugd2lsbCBuZXZlciBieSBhYmxlIHRvIHNwZWNpZnkgaXQuDQo+IA0KPiBhdXRvZnM0IGlzIG1v ZGlmaWVkIHRvIHN0b3AgdXNpbmcgY3VycmVudF9yZWFsX2NyZWQgdGhhdCB3YXMgcHV0IGluDQo+ IHRoZXJlIHRvIGhhbmRsZSB0aGUgcHJldmlvdXMgdmVyc2lvbiBvZiBzdWJtb3VudCBwZXJtaXNz aW9uIGNoZWNraW5nLg0KPiANCj4gY2lmcyBpcyBtb2RpZmllZCB0byBwYXNzIHRoZSBtb3VudHBv aW50IGFsbCBvZiB0aGUgd2F5IGRvd24gdG8NCj4gdmZzX3N1Ym1vdW50Lg0KPiANCj4gZGVidWdm cyBpcyBtb2RpZmllZCB0byBwYXNzIHRoZSBtb3VudHBvaW50IGFsbCBvZiB0aGUgd2F5IGRvd24g dG8NCj4gdHJhY2VfYXV0b21vdW50IGJ5IGFkZGluZyBhIG5ldyBwYXJhbWV0ZXIuwqDCoFRvIG1h a2UgdGhpcyBjaGFuZ2UNCj4gZWFzaWVyDQo+IGEgbmV3IHR5cGVkZWYgZGVidWdmc19hdXRvbW91 bnRfdCBpcyBpbnRyb2R1Y2VkIHRvIGNhcHR1cmUgdGhlIHR5cGUNCj4gb2YNCj4gdGhlIGRlYnVn ZnMgYXV0b21vdW50IGZ1bmN0aW9uLg0KPiANCj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcN Cj4gRml4ZXM6IDA2OWQ1YWM5YWUwZCAoImF1dG9mczrCoMKgRml4IGF1dG9tb3VudHMgYnkgdXNp bmcNCj4gY3VycmVudF9yZWFsX2NyZWQoKS0+dWlkIikNCj4gRml4ZXM6IGFlYWE0YTc5ZmY2YSAo ImZzOiBDYWxsIGRfYXV0b21vdW50IHdpdGggdGhlIGZpbGVzeXN0ZW1zDQo+IGNyZWRzIikNCj4g U2lnbmVkLW9mZi1ieTogIkVyaWMgVy4gQmllZGVybWFuIiA8ZWJpZWRlcm1AeG1pc3Npb24uY29t Pg0KPiAtLS0NCj4gwqBmcy9hZnMvbW50cHQuY8KgwqDCoMKgwqDCoMKgwqDCoMKgfMKgwqAyICst DQo+IMKgZnMvYXV0b2ZzNC93YWl0cS5jwqDCoMKgwqDCoMKgfMKgwqA0ICsrLS0NCj4gwqBmcy9j aWZzL2NpZnNfZGZzX3JlZi5jwqDCoHzCoMKgNyArKysrLS0tDQo+IMKgZnMvZGVidWdmcy9pbm9k ZS5jwqDCoMKgwqDCoMKgfMKgwqA4ICsrKystLS0tDQo+IMKgZnMvbmFtZWkuY8KgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqB8wqDCoDMgLS0tDQo+IMKgZnMvbmFtZXNwYWNlLmPCoMKgwqDCoMKg wqDCoMKgwqDCoHwgMTcgKysrKysrKysrKysrKysrKy0NCj4gwqBmcy9uZnMvbmFtZXNwYWNlLmPC oMKgwqDCoMKgwqB8wqDCoDIgKy0NCj4gwqBmcy9uZnMvbmZzNG5hbWVzcGFjZS5jwqDCoHzCoMKg MiArLQ0KPiDCoGZzL3N1cGVyLmPCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgfCAxMyArKysr KysrKysrLS0tDQo+IMKgaW5jbHVkZS9saW51eC9kZWJ1Z2ZzLmggfMKgwqAzICsrLQ0KPiDCoGlu Y2x1ZGUvbGludXgvbW91bnQuaMKgwqDCoHzCoMKgMyArKysNCj4gwqBpbmNsdWRlL3VhcGkvbGlu dXgvZnMuaCB8wqDCoDEgKw0KPiDCoGtlcm5lbC90cmFjZS90cmFjZS5jwqDCoMKgwqB8wqDCoDQg KystLQ0KPiDCoDEzIGZpbGVzIGNoYW5nZWQsIDQ3IGluc2VydGlvbnMoKyksIDIyIGRlbGV0aW9u cygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL2Fmcy9tbnRwdC5jIGIvZnMvYWZzL21udHB0LmMN Cj4gaW5kZXggODFkZDA3NTM1NmI5Li5kNGZiMGFmYzAwOTcgMTAwNjQ0DQo+IC0tLSBhL2ZzL2Fm cy9tbnRwdC5jDQo+ICsrKyBiL2ZzL2Fmcy9tbnRwdC5jDQo+IEBAIC0yMDIsNyArMjAyLDcgQEAg c3RhdGljIHN0cnVjdCB2ZnNtb3VudA0KPiAqYWZzX21udHB0X2RvX2F1dG9tb3VudChzdHJ1Y3Qg ZGVudHJ5ICptbnRwdCkNCj4gwqANCj4gwqAJLyogdHJ5IGFuZCBkbyB0aGUgbW91bnQgKi8NCj4g wqAJX2RlYnVnKCItLS0gYXR0ZW1wdGluZyBtb3VudCAlcyAtbyAlcyAtLS0iLCBkZXZuYW1lLA0K PiBvcHRpb25zKTsNCj4gLQltbnQgPSB2ZnNfa2Vybl9tb3VudCgmYWZzX2ZzX3R5cGUsIDAsIGRl dm5hbWUsIG9wdGlvbnMpOw0KPiArCW1udCA9IHZmc19zdWJtb3VudChtbnRwdCwgJmFmc19mc190 eXBlLCBkZXZuYW1lLCBvcHRpb25zKTsNCj4gwqAJX2RlYnVnKCItLS0gbW91bnQgcmVzdWx0ICVw IC0tLSIsIG1udCk7DQo+IMKgDQo+IMKgCWZyZWVfcGFnZSgodW5zaWduZWQgbG9uZykgZGV2bmFt ZSk7DQo+IGRpZmYgLS1naXQgYS9mcy9hdXRvZnM0L3dhaXRxLmMgYi9mcy9hdXRvZnM0L3dhaXRx LmMNCj4gaW5kZXggMTI3ODMzNWNlMzY2Li43OWZiZDg1ZGI0YmEgMTAwNjQ0DQo+IC0tLSBhL2Zz L2F1dG9mczQvd2FpdHEuYw0KPiArKysgYi9mcy9hdXRvZnM0L3dhaXRxLmMNCj4gQEAgLTQzNiw4 ICs0MzYsOCBAQCBpbnQgYXV0b2ZzNF93YWl0KHN0cnVjdCBhdXRvZnNfc2JfaW5mbyAqc2JpLA0K PiDCoAkJbWVtY3B5KCZ3cS0+bmFtZSwgJnFzdHIsIHNpemVvZihzdHJ1Y3QgcXN0cikpOw0KPiDC oAkJd3EtPmRldiA9IGF1dG9mczRfZ2V0X2RldihzYmkpOw0KPiDCoAkJd3EtPmlubyA9IGF1dG9m czRfZ2V0X2lubyhzYmkpOw0KPiAtCQl3cS0+dWlkID0gY3VycmVudF9yZWFsX2NyZWQoKS0+dWlk Ow0KPiAtCQl3cS0+Z2lkID0gY3VycmVudF9yZWFsX2NyZWQoKS0+Z2lkOw0KPiArCQl3cS0+dWlk ID0gY3VycmVudF9jcmVkKCktPnVpZDsNCj4gKwkJd3EtPmdpZCA9IGN1cnJlbnRfY3JlZCgpLT5n aWQ7DQo+IMKgCQl3cS0+cGlkID0gcGlkOw0KPiDCoAkJd3EtPnRnaWQgPSB0Z2lkOw0KPiDCoAkJ d3EtPnN0YXR1cyA9IC1FSU5UUjsgLyogU3RhdHVzIHJldHVybiBpZiBpbnRlcnJ1cHRlZA0KPiAq Lw0KPiBkaWZmIC0tZ2l0IGEvZnMvY2lmcy9jaWZzX2Rmc19yZWYuYyBiL2ZzL2NpZnMvY2lmc19k ZnNfcmVmLmMNCj4gaW5kZXggZWM5ZGJiY2NhM2I5Li45MTU2YmU1NDViMGYgMTAwNjQ0DQo+IC0t LSBhL2ZzL2NpZnMvY2lmc19kZnNfcmVmLmMNCj4gKysrIGIvZnMvY2lmcy9jaWZzX2Rmc19yZWYu Yw0KPiBAQCAtMjQ1LDcgKzI0NSw4IEBAIGNoYXIgKmNpZnNfY29tcG9zZV9tb3VudF9vcHRpb25z KGNvbnN0IGNoYXINCj4gKnNiX21vdW50ZGF0YSwNCj4gwqAgKiBAZnVsbHBhdGg6CQlmdWxsIHBh dGggaW4gVU5DIGZvcm1hdA0KPiDCoCAqIEByZWY6CQlzZXJ2ZXIncyByZWZlcnJhbA0KPiDCoCAq Lw0KPiAtc3RhdGljIHN0cnVjdCB2ZnNtb3VudCAqY2lmc19kZnNfZG9fcmVmbW91bnQoc3RydWN0 IGNpZnNfc2JfaW5mbw0KPiAqY2lmc19zYiwNCj4gK3N0YXRpYyBzdHJ1Y3QgdmZzbW91bnQgKmNp ZnNfZGZzX2RvX3JlZm1vdW50KHN0cnVjdCBkZW50cnkgKm1udHB0LA0KPiArCQlzdHJ1Y3QgY2lm c19zYl9pbmZvICpjaWZzX3NiLA0KPiDCoAkJY29uc3QgY2hhciAqZnVsbHBhdGgsIGNvbnN0IHN0 cnVjdCBkZnNfaW5mbzNfcGFyYW0NCj4gKnJlZikNCj4gwqB7DQo+IMKgCXN0cnVjdCB2ZnNtb3Vu dCAqbW50Ow0KPiBAQCAtMjU5LDcgKzI2MCw3IEBAIHN0YXRpYyBzdHJ1Y3QgdmZzbW91bnQNCj4g KmNpZnNfZGZzX2RvX3JlZm1vdW50KHN0cnVjdCBjaWZzX3NiX2luZm8gKmNpZnNfc2IsDQo+IMKg CWlmIChJU19FUlIobW91bnRkYXRhKSkNCj4gwqAJCXJldHVybiAoc3RydWN0IHZmc21vdW50ICop bW91bnRkYXRhOw0KPiDCoA0KPiAtCW1udCA9IHZmc19rZXJuX21vdW50KCZjaWZzX2ZzX3R5cGUs IDAsIGRldm5hbWUsIG1vdW50ZGF0YSk7DQo+ICsJbW50ID0gdmZzX3N1Ym1vdW50KG1udHB0LCAm Y2lmc19mc190eXBlLCBkZXZuYW1lLA0KPiBtb3VudGRhdGEpOw0KPiDCoAlrZnJlZShtb3VudGRh dGEpOw0KPiDCoAlrZnJlZShkZXZuYW1lKTsNCj4gwqAJcmV0dXJuIG1udDsNCj4gQEAgLTMzNCw3 ICszMzUsNyBAQCBzdGF0aWMgc3RydWN0IHZmc21vdW50DQo+ICpjaWZzX2Rmc19kb19hdXRvbW91 bnQoc3RydWN0IGRlbnRyeSAqbW50cHQpDQo+IMKgCQkJbW50ID0gRVJSX1BUUigtRUlOVkFMKTsN Cj4gwqAJCQlicmVhazsNCj4gwqAJCX0NCj4gLQkJbW50ID0gY2lmc19kZnNfZG9fcmVmbW91bnQo Y2lmc19zYiwNCj4gKwkJbW50ID0gY2lmc19kZnNfZG9fcmVmbW91bnQobW50cHQsIGNpZnNfc2Is DQo+IMKgCQkJCWZ1bGxfcGF0aCwgcmVmZXJyYWxzICsgaSk7DQo+IMKgCQljaWZzX2RiZyhGWUks ICIlczogY2lmc19kZnNfZG9fcmVmbW91bnQ6JXMgLA0KPiBtbnQ6JXBcbiIsDQo+IMKgCQkJwqBf X2Z1bmNfXywgcmVmZXJyYWxzW2ldLm5vZGVfbmFtZSwgbW50KTsNCj4gZGlmZiAtLWdpdCBhL2Zz L2RlYnVnZnMvaW5vZGUuYyBiL2ZzL2RlYnVnZnMvaW5vZGUuYw0KPiBpbmRleCBmMTdmY2Y4OWUx OGUuLjFlMzBmNzRhOTUyNyAxMDA2NDQNCj4gLS0tIGEvZnMvZGVidWdmcy9pbm9kZS5jDQo+ICsr KyBiL2ZzL2RlYnVnZnMvaW5vZGUuYw0KPiBAQCAtMTg3LDkgKzE4Nyw5IEBAIHN0YXRpYyBjb25z dCBzdHJ1Y3Qgc3VwZXJfb3BlcmF0aW9ucw0KPiBkZWJ1Z2ZzX3N1cGVyX29wZXJhdGlvbnMgPSB7 DQo+IMKgDQo+IMKgc3RhdGljIHN0cnVjdCB2ZnNtb3VudCAqZGVidWdmc19hdXRvbW91bnQoc3Ry dWN0IHBhdGggKnBhdGgpDQo+IMKgew0KPiAtCXN0cnVjdCB2ZnNtb3VudCAqKCpmKSh2b2lkICop Ow0KPiAtCWYgPSAoc3RydWN0IHZmc21vdW50ICooKikodm9pZCAqKSlwYXRoLT5kZW50cnktPmRf ZnNkYXRhOw0KPiAtCXJldHVybiBmKGRfaW5vZGUocGF0aC0+ZGVudHJ5KS0+aV9wcml2YXRlKTsN Cj4gKwlkZWJ1Z2ZzX2F1dG9tb3VudF90IGY7DQo+ICsJZiA9IChkZWJ1Z2ZzX2F1dG9tb3VudF90 KXBhdGgtPmRlbnRyeS0+ZF9mc2RhdGE7DQo+ICsJcmV0dXJuIGYocGF0aC0+ZGVudHJ5LCBkX2lu b2RlKHBhdGgtPmRlbnRyeSktPmlfcHJpdmF0ZSk7DQo+IMKgfQ0KPiDCoA0KPiDCoHN0YXRpYyBj b25zdCBzdHJ1Y3QgZGVudHJ5X29wZXJhdGlvbnMgZGVidWdmc19kb3BzID0gew0KPiBAQCAtNTA0 LDcgKzUwNCw3IEBAIEVYUE9SVF9TWU1CT0xfR1BMKGRlYnVnZnNfY3JlYXRlX2Rpcik7DQo+IMKg ICovDQo+IMKgc3RydWN0IGRlbnRyeSAqZGVidWdmc19jcmVhdGVfYXV0b21vdW50KGNvbnN0IGNo YXIgKm5hbWUsDQo+IMKgCQkJCQlzdHJ1Y3QgZGVudHJ5ICpwYXJlbnQsDQo+IC0JCQkJCXN0cnVj dCB2ZnNtb3VudCAqKCpmKSh2b2lkDQo+ICopLA0KPiArCQkJCQlkZWJ1Z2ZzX2F1dG9tb3VudF90 IGYsDQo+IMKgCQkJCQl2b2lkICpkYXRhKQ0KPiDCoHsNCj4gwqAJc3RydWN0IGRlbnRyeSAqZGVu dHJ5ID0gc3RhcnRfY3JlYXRpbmcobmFtZSwgcGFyZW50KTsNCj4gZGlmZiAtLWdpdCBhL2ZzL25h bWVpLmMgYi9mcy9uYW1laS5jDQo+IGluZGV4IDZmYTNlOTEzOGZlNC4uZGE2ODljOWMwMDVlIDEw MDY0NA0KPiAtLS0gYS9mcy9uYW1laS5jDQo+ICsrKyBiL2ZzL25hbWVpLmMNCj4gQEAgLTExMDAs NyArMTEwMCw2IEBAIHN0YXRpYyBpbnQgZm9sbG93X2F1dG9tb3VudChzdHJ1Y3QgcGF0aCAqcGF0 aCwNCj4gc3RydWN0IG5hbWVpZGF0YSAqbmQsDQo+IMKgCQkJwqDCoMKgwqBib29sICpuZWVkX21u dHB1dCkNCj4gwqB7DQo+IMKgCXN0cnVjdCB2ZnNtb3VudCAqbW50Ow0KPiAtCWNvbnN0IHN0cnVj dCBjcmVkICpvbGRfY3JlZDsNCj4gwqAJaW50IGVycjsNCj4gwqANCj4gwqAJaWYgKCFwYXRoLT5k ZW50cnktPmRfb3AgfHwgIXBhdGgtPmRlbnRyeS0+ZF9vcC0+ZF9hdXRvbW91bnQpDQo+IEBAIC0x MTI5LDkgKzExMjgsNyBAQCBzdGF0aWMgaW50IGZvbGxvd19hdXRvbW91bnQoc3RydWN0IHBhdGgg KnBhdGgsDQo+IHN0cnVjdCBuYW1laWRhdGEgKm5kLA0KPiDCoAlpZiAobmQtPnRvdGFsX2xpbmtf Y291bnQgPj0gNDApDQo+IMKgCQlyZXR1cm4gLUVMT09QOw0KPiDCoA0KPiAtCW9sZF9jcmVkID0g b3ZlcnJpZGVfY3JlZHMoJmluaXRfY3JlZCk7DQo+IMKgCW1udCA9IHBhdGgtPmRlbnRyeS0+ZF9v cC0+ZF9hdXRvbW91bnQocGF0aCk7DQo+IC0JcmV2ZXJ0X2NyZWRzKG9sZF9jcmVkKTsNCj4gwqAJ aWYgKElTX0VSUihtbnQpKSB7DQo+IMKgCQkvKg0KPiDCoAkJwqAqIFRoZSBmaWxlc3lzdGVtIGlz IGFsbG93ZWQgdG8gcmV0dXJuIC1FSVNESVIgaGVyZQ0KPiB0byBpbmRpY2F0ZQ0KPiBkaWZmIC0t Z2l0IGEvZnMvbmFtZXNwYWNlLmMgYi9mcy9uYW1lc3BhY2UuYw0KPiBpbmRleCA0ODdiYTMwYmI1 YzYuLjA4OWE2YjIzMTM1YSAxMDA2NDQNCj4gLS0tIGEvZnMvbmFtZXNwYWNlLmMNCj4gKysrIGIv ZnMvbmFtZXNwYWNlLmMNCj4gQEAgLTk4OSw2ICs5ODksMjEgQEAgdmZzX2tlcm5fbW91bnQoc3Ry dWN0IGZpbGVfc3lzdGVtX3R5cGUgKnR5cGUsDQo+IGludCBmbGFncywgY29uc3QgY2hhciAqbmFt ZSwgdm9pZA0KPiDCoH0NCj4gwqBFWFBPUlRfU1lNQk9MX0dQTCh2ZnNfa2Vybl9tb3VudCk7DQo+ IMKgDQo+ICtzdHJ1Y3QgdmZzbW91bnQgKg0KPiArdmZzX3N1Ym1vdW50KGNvbnN0IHN0cnVjdCBk ZW50cnkgKm1vdW50cG9pbnQsIHN0cnVjdA0KPiBmaWxlX3N5c3RlbV90eXBlICp0eXBlLA0KPiAr CcKgwqDCoMKgwqBjb25zdCBjaGFyICpuYW1lLCB2b2lkICpkYXRhKQ0KPiArew0KPiArCS8qIFVu dGlsIGl0IGlzIHdvcmtlZCBvdXQgaG93IHRvIHBhc3MgdGhlIHVzZXIgbmFtZXNwYWNlDQo+ICsJ wqAqIHRocm91Z2ggZnJvbSB0aGUgcGFyZW50IG1vdW50IHRvIHRoZSBzdWJtb3VudCBkb24ndA0K PiBzdXBwb3J0DQo+ICsJwqAqIHVucHJpdmlsZWdlZCBtb3VudHMgd2l0aCBzdWJtb3VudHMuDQo+ ICsJwqAqLw0KPiArCWlmIChtb3VudHBvaW50LT5kX3NiLT5zX3VzZXJfbnMgIT0gJmluaXRfdXNl cl9ucykNCj4gKwkJcmV0dXJuIEVSUl9QVFIoLUVQRVJNKTsNCj4gKw0KPiArCXJldHVybiB2ZnNf a2Vybl9tb3VudCh0eXBlLCBNU19TVUJNT1VOVCwgbmFtZSwgZGF0YSk7DQo+ICt9DQo+ICtFWFBP UlRfU1lNQk9MX0dQTCh2ZnNfc3VibW91bnQpOw0KPiArDQo+IMKgc3RhdGljIHN0cnVjdCBtb3Vu dCAqY2xvbmVfbW50KHN0cnVjdCBtb3VudCAqb2xkLCBzdHJ1Y3QgZGVudHJ5DQo+ICpyb290LA0K PiDCoAkJCQkJaW50IGZsYWcpDQo+IMKgew0KPiBAQCAtMjc5NCw3ICsyODA5LDcgQEAgbG9uZyBk b19tb3VudChjb25zdCBjaGFyICpkZXZfbmFtZSwgY29uc3QgY2hhcg0KPiBfX3VzZXIgKmRpcl9u YW1lLA0KPiDCoA0KPiDCoAlmbGFncyAmPSB+KE1TX05PU1VJRCB8IE1TX05PRVhFQyB8IE1TX05P REVWIHwgTVNfQUNUSVZFIHwNCj4gTVNfQk9STiB8DQo+IMKgCQnCoMKgwqBNU19OT0FUSU1FIHwg TVNfTk9ESVJBVElNRSB8IE1TX1JFTEFUSU1FfA0KPiBNU19LRVJOTU9VTlQgfA0KPiAtCQnCoMKg wqBNU19TVFJJQ1RBVElNRSB8IE1TX05PUkVNT1RFTE9DSyk7DQo+ICsJCcKgwqDCoE1TX1NUUklD VEFUSU1FIHwgTVNfTk9SRU1PVEVMT0NLIHwgTVNfU1VCTU9VTlQpOw0KPiDCoA0KPiDCoAlpZiAo ZmxhZ3MgJiBNU19SRU1PVU5UKQ0KPiDCoAkJcmV0dmFsID0gZG9fcmVtb3VudCgmcGF0aCwgZmxh Z3MgJiB+TVNfUkVNT1VOVCwNCj4gbW50X2ZsYWdzLA0KPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25h bWVzcGFjZS5jIGIvZnMvbmZzL25hbWVzcGFjZS5jDQo+IGluZGV4IDU1NTFlOGVmNjdmZC4uZTQ5 ZDgzMWM0ZTg1IDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvbmFtZXNwYWNlLmMNCj4gKysrIGIvZnMv bmZzL25hbWVzcGFjZS5jDQo+IEBAIC0yMjYsNyArMjI2LDcgQEAgc3RhdGljIHN0cnVjdCB2ZnNt b3VudCAqbmZzX2RvX2Nsb25lX21vdW50KHN0cnVjdA0KPiBuZnNfc2VydmVyICpzZXJ2ZXIsDQo+ IMKgCQkJCQnCoMKgwqBjb25zdCBjaGFyICpkZXZuYW1lLA0KPiDCoAkJCQkJwqDCoMKgc3RydWN0 IG5mc19jbG9uZV9tb3VudA0KPiAqbW91bnRkYXRhKQ0KPiDCoHsNCj4gLQlyZXR1cm4gdmZzX2tl cm5fbW91bnQoJm5mc194ZGV2X2ZzX3R5cGUsIDAsIGRldm5hbWUsDQo+IG1vdW50ZGF0YSk7DQo+ ICsJcmV0dXJuIHZmc19zdWJtb3VudChtb3VudGRhdGEtPmRlbnRyeSwgJm5mc194ZGV2X2ZzX3R5 cGUsDQo+IGRldm5hbWUsIG1vdW50ZGF0YSk7DQo+IMKgfQ0KPiDCoA0KPiDCoC8qKg0KPiBkaWZm IC0tZ2l0IGEvZnMvbmZzL25mczRuYW1lc3BhY2UuYyBiL2ZzL25mcy9uZnM0bmFtZXNwYWNlLmMN Cj4gaW5kZXggZDIxMTA0OTEyNjc2Li5kOGIwNDBiZDk4MTQgMTAwNjQ0DQo+IC0tLSBhL2ZzL25m cy9uZnM0bmFtZXNwYWNlLmMNCj4gKysrIGIvZnMvbmZzL25mczRuYW1lc3BhY2UuYw0KPiBAQCAt Mjc5LDcgKzI3OSw3IEBAIHN0YXRpYyBzdHJ1Y3QgdmZzbW91bnQgKnRyeV9sb2NhdGlvbihzdHJ1 Y3QNCj4gbmZzX2Nsb25lX21vdW50ICptb3VudGRhdGEsDQo+IMKgCQkJCW1vdW50ZGF0YS0+aG9z dG5hbWUsDQo+IMKgCQkJCW1vdW50ZGF0YS0+bW50X3BhdGgpOw0KPiDCoA0KPiAtCQltbnQgPSB2 ZnNfa2Vybl9tb3VudCgmbmZzNF9yZWZlcnJhbF9mc190eXBlLCAwLA0KPiBwYWdlLCBtb3VudGRh dGEpOw0KPiArCQltbnQgPSB2ZnNfc3VibW91bnQobW91bnRkYXRhLT5kZW50cnksDQo+ICZuZnM0 X3JlZmVycmFsX2ZzX3R5cGUsIHBhZ2UsIG1vdW50ZGF0YSk7DQo+IMKgCQlpZiAoIUlTX0VSUiht bnQpKQ0KPiDCoAkJCWJyZWFrOw0KPiDCoAl9DQo+IGRpZmYgLS1naXQgYS9mcy9zdXBlci5jIGIv ZnMvc3VwZXIuYw0KPiBpbmRleCAxNzA5ZWQwMjlhMmMuLjQxODU4NDRmN2ExMiAxMDA2NDQNCj4g LS0tIGEvZnMvc3VwZXIuYw0KPiArKysgYi9mcy9zdXBlci5jDQo+IEBAIC00NjksNyArNDY5LDcg QEAgc3RydWN0IHN1cGVyX2Jsb2NrICpzZ2V0X3VzZXJucyhzdHJ1Y3QNCj4gZmlsZV9zeXN0ZW1f dHlwZSAqdHlwZSwNCj4gwqAJc3RydWN0IHN1cGVyX2Jsb2NrICpvbGQ7DQo+IMKgCWludCBlcnI7 DQo+IMKgDQo+IC0JaWYgKCEoZmxhZ3MgJiBNU19LRVJOTU9VTlQpICYmDQo+ICsJaWYgKCEoZmxh Z3MgJiAoTVNfS0VSTk1PVU5UfE1TX1NVQk1PVU5UKSkgJiYNCj4gwqAJwqDCoMKgwqAhKHR5cGUt PmZzX2ZsYWdzICYgRlNfVVNFUk5TX01PVU5UKSAmJg0KPiDCoAnCoMKgwqDCoCFjYXBhYmxlKENB UF9TWVNfQURNSU4pKQ0KPiDCoAkJcmV0dXJuIEVSUl9QVFIoLUVQRVJNKTsNCj4gQEAgLTQ5OSw3 ICs0OTksNyBAQCBzdHJ1Y3Qgc3VwZXJfYmxvY2sgKnNnZXRfdXNlcm5zKHN0cnVjdA0KPiBmaWxl X3N5c3RlbV90eXBlICp0eXBlLA0KPiDCoAl9DQo+IMKgCWlmICghcykgew0KPiDCoAkJc3Bpbl91 bmxvY2soJnNiX2xvY2spOw0KPiAtCQlzID0gYWxsb2Nfc3VwZXIodHlwZSwgZmxhZ3MsIHVzZXJf bnMpOw0KPiArCQlzID0gYWxsb2Nfc3VwZXIodHlwZSwgKGZsYWdzICYgfk1TX1NVQk1PVU5UKSwN Cj4gdXNlcl9ucyk7DQo+IMKgCQlpZiAoIXMpDQo+IMKgCQkJcmV0dXJuIEVSUl9QVFIoLUVOT01F TSk7DQo+IMKgCQlnb3RvIHJldHJ5Ow0KPiBAQCAtNTQwLDggKzU0MCwxNSBAQCBzdHJ1Y3Qgc3Vw ZXJfYmxvY2sgKnNnZXQoc3RydWN0IGZpbGVfc3lzdGVtX3R5cGUNCj4gKnR5cGUsDQo+IMKgew0K PiDCoAlzdHJ1Y3QgdXNlcl9uYW1lc3BhY2UgKnVzZXJfbnMgPSBjdXJyZW50X3VzZXJfbnMoKTsN Cj4gwqANCj4gKwkvKiBXZSBkb24ndCB5ZXQgcGFzcyB0aGUgdXNlciBuYW1lc3BhY2Ugb2YgdGhl IHBhcmVudA0KPiArCcKgKiBtb3VudCB0aHJvdWdoIHRvIGhlcmUgc28gYWx3YXlzIHVzZSAmaW5p dF91c2VyX25zDQo+ICsJwqAqIHVudGlsIHRoYXQgY2hhbmdlcy4NCj4gKwnCoCovDQo+ICsJaWYg KGZsYWdzICYgTVNfU1VCTU9VTlQpDQo+ICsJCXVzZXJfbnMgPSAmaW5pdF91c2VyX25zOw0KPiAr DQo+IMKgCS8qIEVuc3VyZSB0aGUgcmVxdWVzdG9yIGhhcyBwZXJtaXNzaW9ucyBvdmVyIHRoZSB0 YXJnZXQNCj4gZmlsZXN5c3RlbSAqLw0KPiAtCWlmICghKGZsYWdzICYgTVNfS0VSTk1PVU5UKSAm JiAhbnNfY2FwYWJsZSh1c2VyX25zLA0KPiBDQVBfU1lTX0FETUlOKSkNCj4gKwlpZiAoIShmbGFn cyAmIChNU19LRVJOTU9VTlR8TVNfU1VCTU9VTlQpKSAmJg0KPiAhbnNfY2FwYWJsZSh1c2VyX25z LCBDQVBfU1lTX0FETUlOKSkNCj4gwqAJCXJldHVybiBFUlJfUFRSKC1FUEVSTSk7DQo+IMKgDQo+ IMKgCXJldHVybiBzZ2V0X3VzZXJucyh0eXBlLCB0ZXN0LCBzZXQsIGZsYWdzLCB1c2VyX25zLCBk YXRhKTsNCj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvZGVidWdmcy5oIGIvaW5jbHVkZS9s aW51eC9kZWJ1Z2ZzLmgNCj4gaW5kZXggMDE0Y2M1NjRkMWM0Li4yMzMwMDZiZTMwYWEgMTAwNjQ0 DQo+IC0tLSBhL2luY2x1ZGUvbGludXgvZGVidWdmcy5oDQo+ICsrKyBiL2luY2x1ZGUvbGludXgv ZGVidWdmcy5oDQo+IEBAIC05Nyw5ICs5NywxMCBAQCBzdHJ1Y3QgZGVudHJ5ICpkZWJ1Z2ZzX2Ny ZWF0ZV9kaXIoY29uc3QgY2hhcg0KPiAqbmFtZSwgc3RydWN0IGRlbnRyeSAqcGFyZW50KTsNCj4g wqBzdHJ1Y3QgZGVudHJ5ICpkZWJ1Z2ZzX2NyZWF0ZV9zeW1saW5rKGNvbnN0IGNoYXIgKm5hbWUs IHN0cnVjdA0KPiBkZW50cnkgKnBhcmVudCwNCj4gwqAJCQkJwqDCoMKgwqDCoMKgY29uc3QgY2hh ciAqZGVzdCk7DQo+IMKgDQo+ICt0eXBlZGVmIHN0cnVjdCB2ZnNtb3VudCAqKCpkZWJ1Z2ZzX2F1 dG9tb3VudF90KShzdHJ1Y3QgZGVudHJ5ICosDQo+IHZvaWQgKik7DQo+IMKgc3RydWN0IGRlbnRy eSAqZGVidWdmc19jcmVhdGVfYXV0b21vdW50KGNvbnN0IGNoYXIgKm5hbWUsDQo+IMKgCQkJCQlz dHJ1Y3QgZGVudHJ5ICpwYXJlbnQsDQo+IC0JCQkJCXN0cnVjdCB2ZnNtb3VudCAqKCpmKSh2b2lk DQo+ICopLA0KPiArCQkJCQlkZWJ1Z2ZzX2F1dG9tb3VudF90IGYsDQo+IMKgCQkJCQl2b2lkICpk YXRhKTsNCj4gwqANCj4gwqB2b2lkIGRlYnVnZnNfcmVtb3ZlKHN0cnVjdCBkZW50cnkgKmRlbnRy eSk7DQo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L21vdW50LmggYi9pbmNsdWRlL2xpbnV4 L21vdW50LmgNCj4gaW5kZXggYzZmNTUxNThkNWU1Li44ZTAzNTJhZjA2YjcgMTAwNjQ0DQo+IC0t LSBhL2luY2x1ZGUvbGludXgvbW91bnQuaA0KPiArKysgYi9pbmNsdWRlL2xpbnV4L21vdW50LmgN Cj4gQEAgLTkwLDYgKzkwLDkgQEAgc3RydWN0IGZpbGVfc3lzdGVtX3R5cGU7DQo+IMKgZXh0ZXJu IHN0cnVjdCB2ZnNtb3VudCAqdmZzX2tlcm5fbW91bnQoc3RydWN0IGZpbGVfc3lzdGVtX3R5cGUN Cj4gKnR5cGUsDQo+IMKgCQkJCcKgwqDCoMKgwqDCoGludCBmbGFncywgY29uc3QgY2hhciAqbmFt ZSwNCj4gwqAJCQkJwqDCoMKgwqDCoMKgdm9pZCAqZGF0YSk7DQo+ICtleHRlcm4gc3RydWN0IHZm c21vdW50ICp2ZnNfc3VibW91bnQoY29uc3Qgc3RydWN0IGRlbnRyeQ0KPiAqbW91bnRwb2ludCwN Cj4gKwkJCQnCoMKgwqDCoMKgc3RydWN0IGZpbGVfc3lzdGVtX3R5cGUgKnR5cGUsDQo+ICsJCQkJ wqDCoMKgwqDCoGNvbnN0IGNoYXIgKm5hbWUsIHZvaWQgKmRhdGEpOw0KPiDCoA0KPiDCoGV4dGVy biB2b2lkIG1udF9zZXRfZXhwaXJ5KHN0cnVjdCB2ZnNtb3VudCAqbW50LCBzdHJ1Y3QgbGlzdF9o ZWFkDQo+ICpleHBpcnlfbGlzdCk7DQo+IMKgZXh0ZXJuIHZvaWQgbWFya19tb3VudHNfZm9yX2V4 cGlyeShzdHJ1Y3QgbGlzdF9oZWFkICptb3VudHMpOw0KPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS91 YXBpL2xpbnV4L2ZzLmggYi9pbmNsdWRlL3VhcGkvbGludXgvZnMuaA0KPiBpbmRleCAzNmRhOTNm YmYxODguLjA0OGE4NWU5ZjAxNyAxMDA2NDQNCj4gLS0tIGEvaW5jbHVkZS91YXBpL2xpbnV4L2Zz LmgNCj4gKysrIGIvaW5jbHVkZS91YXBpL2xpbnV4L2ZzLmgNCj4gQEAgLTEzMiw2ICsxMzIsNyBA QCBzdHJ1Y3QgaW5vZGVzX3N0YXRfdCB7DQo+IMKgI2RlZmluZSBNU19MQVpZVElNRQkoMTw8MjUp IC8qIFVwZGF0ZSB0aGUgb24tZGlzayBbYWNtXXRpbWVzDQo+IGxhemlseSAqLw0KPiDCoA0KPiDC oC8qIFRoZXNlIHNiIGZsYWdzIGFyZSBpbnRlcm5hbCB0byB0aGUga2VybmVsICovDQo+ICsjZGVm aW5lIE1TX1NVQk1PVU5UwqDCoMKgwqDCoCgxPDwyNikNCj4gwqAjZGVmaW5lIE1TX05PUkVNT1RF TE9DSwkoMTw8MjcpDQo+IMKgI2RlZmluZSBNU19OT1NFQwkoMTw8MjgpDQo+IMKgI2RlZmluZSBN U19CT1JOCQkoMTw8MjkpDQo+IGRpZmYgLS1naXQgYS9rZXJuZWwvdHJhY2UvdHJhY2UuYyBiL2tl cm5lbC90cmFjZS90cmFjZS5jDQo+IGluZGV4IGQ3NDQ5NzgzOTg3YS4uMzEwZjBlYTBkMWEyIDEw MDY0NA0KPiAtLS0gYS9rZXJuZWwvdHJhY2UvdHJhY2UuYw0KPiArKysgYi9rZXJuZWwvdHJhY2Uv dHJhY2UuYw0KPiBAQCAtNzUwMyw3ICs3NTAzLDcgQEAgaW5pdF90cmFjZXJfdHJhY2VmcyhzdHJ1 Y3QgdHJhY2VfYXJyYXkgKnRyLA0KPiBzdHJ1Y3QgZGVudHJ5ICpkX3RyYWNlcikNCj4gwqAJZnRy YWNlX2luaXRfdHJhY2Vmcyh0ciwgZF90cmFjZXIpOw0KPiDCoH0NCj4gwqANCj4gLXN0YXRpYyBz dHJ1Y3QgdmZzbW91bnQgKnRyYWNlX2F1dG9tb3VudCh2b2lkICppbmdvcmUpDQo+ICtzdGF0aWMg c3RydWN0IHZmc21vdW50ICp0cmFjZV9hdXRvbW91bnQoc3RydWN0IGRlbnRyeSAqbW50cHQsIHZv aWQNCj4gKmluZ29yZSkNCj4gwqB7DQo+IMKgCXN0cnVjdCB2ZnNtb3VudCAqbW50Ow0KPiDCoAlz dHJ1Y3QgZmlsZV9zeXN0ZW1fdHlwZSAqdHlwZTsNCj4gQEAgLTc1MTYsNyArNzUxNiw3IEBAIHN0 YXRpYyBzdHJ1Y3QgdmZzbW91bnQgKnRyYWNlX2F1dG9tb3VudCh2b2lkDQo+ICppbmdvcmUpDQo+ IMKgCXR5cGUgPSBnZXRfZnNfdHlwZSgidHJhY2VmcyIpOw0KPiDCoAlpZiAoIXR5cGUpDQo+IMKg CQlyZXR1cm4gTlVMTDsNCj4gLQltbnQgPSB2ZnNfa2Vybl9tb3VudCh0eXBlLCAwLCAidHJhY2Vm cyIsIE5VTEwpOw0KPiArCW1udCA9IHZmc19zdWJtb3VudChtbnRwdCwgdHlwZSwgInRyYWNlZnMi LCBOVUxMKTsNCj4gwqAJcHV0X2ZpbGVzeXN0ZW0odHlwZSk7DQo+IMKgCWlmIChJU19FUlIobW50 KSkNCj4gwqAJCXJldHVybiBOVUxMOw0KDQpZZXMuIFRoYXQgbG9va3MgbGlrZSBpdCBtaWdodCBi ZSB3b3JrYWJsZS4NCg0KUmV2aWV3ZWQtYnk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVi dXN0QHByaW1hcnlkYXRhLmNvbT4NCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xp ZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEu Y29tDQo= ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [REVIEW][PATCH] fs: Better permission checking for submounts 2017-02-01 6:38 ` [REVIEW][PATCH] fs: Better permission checking for submounts Eric W. Biederman 2017-02-01 13:28 ` Trond Myklebust @ 2017-02-01 13:38 ` Seth Forshee 1 sibling, 0 replies; 19+ messages in thread From: Seth Forshee @ 2017-02-01 13:38 UTC (permalink / raw) To: Eric W. Biederman Cc: Trond Myklebust, bfields, anna.schumaker, Steve French, linux-nfs, dhowells, linux-fsdevel On Wed, Feb 01, 2017 at 07:38:17PM +1300, Eric W. Biederman wrote: > > To support unprivileged users mounting filesystems two permission > checks have to be performed: a test to see if the user allowed to > create a mount in the mount namespace, and a test to see if > the user is allowed to access the specified filesystem. > > The automount case is special in that mounting the original filesystem > grants permission to mount the sub-filesystems, to any user who > happens to stumble across the their mountpoint and satisfies the > ordinary filesystem permission checks. > > Attempting to handle the automount case by using override_creds > almost works. It preserves the idea that permission to mount > the original filesystem is permission to mount the sub-filesystem. > Unfortunately using override_creds messes up the filesystems > ordinary permission checks. > > Solve this by being explicit that a mount is a submount by introducing > vfs_submount, and using it where appropriate. > > vfs_submount uses a new mount internal mount flags MS_SUBMOUNT, to let > sget and friends know that a mount is a submount so they can take appropriate > action. > > sget and sget_userns are modified to not perform any permission checks > on submounts. > > follow_automount is modified to stop using override_creds as that > has proven problemantic. > > do_mount is modified to always remove the new MS_SUBMOUNT flag so > that we know userspace will never by able to specify it. > > autofs4 is modified to stop using current_real_cred that was put in > there to handle the previous version of submount permission checking. > > cifs is modified to pass the mountpoint all of the way down to vfs_submount. > > debugfs is modified to pass the mountpoint all of the way down to > trace_automount by adding a new parameter. To make this change easier > a new typedef debugfs_automount_t is introduced to capture the type of > the debugfs automount function. > > Cc: stable@vger.kernel.org > Fixes: 069d5ac9ae0d ("autofs: Fix automounts by using current_real_cred()->uid") > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems creds") > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Looks good to me. I also got testing from the user who reported the bug to us, and it does fix his nfs submount problem. Reviewed-by: Seth Forshee <seth.forshee@canonical.com> ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-02-01 13:38 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-15 17:13 [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials Seth Forshee 2016-12-15 23:01 ` Trond Myklebust 2016-12-16 13:06 ` Seth Forshee 2017-01-10 14:55 ` Seth Forshee 2017-01-11 0:21 ` Eric W. Biederman 2017-01-24 15:17 ` Seth Forshee 2017-01-24 22:55 ` Eric W. Biederman 2017-01-24 23:28 ` Eric W. Biederman 2017-01-24 23:46 ` Trond Myklebust 2017-01-24 23:56 ` Eric W. Biederman 2017-01-25 0:14 ` Trond Myklebust 2017-01-25 14:52 ` Seth Forshee 2017-01-25 15:51 ` Trond Myklebust 2017-01-25 16:28 ` Seth Forshee 2017-02-01 6:36 ` Eric W. Biederman 2017-02-01 6:38 ` [REVIEW][PATCH] fs: Better permission checking for submounts Eric W. Biederman 2017-02-01 13:28 ` Trond Myklebust 2017-02-01 13:28 ` Trond Myklebust 2017-02-01 13:38 ` Seth Forshee
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.