4.1 NULL dereference in 2.6.32-rc3

4.1 NULL dereference in 2.6.32-rc3

[J. Bruce Fields]

After mounting and unmounting a 4.1 partition with client and server
both 2.6.32-rc3, I see the following NULL dereference on the client.

I think the only cache lookup there is in unix_gid_find().  Hm.
Maybe it's trying to defer a request without a defer method set?

Of course there's no point to the client's callback server doing this
upcall at all.

--b.

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<(null)>] (null)
*pde = 00000000 
Oops: 0000 [#1] PREEMPT 
last sysfs file: /sys/kernel/uevent_seqnum
Modules linked in:

Pid: 3108, comm: nfsv4.1-svc Tainted: G        W  (2.6.32-rc3 #144) 
EIP: 0060:[<00000000>] EFLAGS: 00010293 CPU: 0
EIP is at 0x0
EAX: c73edd7c EBX: c5d2f8e8 ECX: 00000000 EDX: 00000001
ESI: c5d2f8d8 EDI: 4aca7522 EBP: c71b1e80 ESP: c71b1e58
 DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
Process nfsv4.1-svc (pid: 3108, ti=c71b0000 task=c4800020 task.ti=c71b0000)
Stack:
 c176f01a c71b1e80 c176f695 c71b1e90 c73edd7c c1aca8a0 fffffff5 c73ed000
<0> c5d2f8d8 00000000 c71b1eb8 c1768dcf c71b1f30 00000fc4 c1aca7bc 00000246
<0> c17689e2 00000001 c1aca7bc 00000000 c17c0158 c1aca944 c73ed0c8 00000000
Call Trace:
 [<c176f01a>] ? cache_check+0xea/0x350
 [<c176f695>] ? sunrpc_cache_lookup+0x125/0x140
 [<c1768dcf>] ? svcauth_unix_accept+0x15f/0x2e0
 [<c17689e2>] ? svc_authenticate+0x142/0x1a0
 [<c17c0158>] ? sub_preempt_count+0x8/0x90
 [<c17689f7>] ? svc_authenticate+0x157/0x1a0
 [<c17bd877>] ? _spin_unlock_irq+0x27/0x50
 [<c1764cd3>] ? svc_process_common+0x3f3/0x630
 [<c1764fd2>] ? bc_svc_process+0xc2/0x100
 [<c1059d0b>] ? trace_hardirqs_on+0xb/0x10
 [<c1213487>] ? nfs41_callback_svc+0x87/0x120
 [<c1049c50>] ? autoremove_wake_function+0x0/0x50
 [<c1213400>] ? nfs41_callback_svc+0x0/0x120
 [<c10499a4>] ? kthread+0x74/0x80
 [<c1049930>] ? kthread+0x0/0x80
 [<c100363b>] ? kernel_thread_helper+0x7/0x10
Code:  Bad EIP value.
EIP: [<00000000>] 0x0 SS:ESP 0068:c71b1e58
CR2: 0000000000000000
---[ end trace 39933fa1a06d9d4b ]---

Re: 4.1 NULL dereference in 2.6.32-rc3

[J. Bruce Fields]

On Mon, Oct 05, 2009 at 07:07:36PM -0400, J. Bruce Fields wrote:
> After mounting and unmounting a 4.1 partition with client and server
> both 2.6.32-rc3, I see the following NULL dereference on the client.
> 
> I think the only cache lookup there is in unix_gid_find().  Hm.
> Maybe it's trying to defer a request without a defer method set?

Confirmed.  And I don't see where the client sets any defer method.  (It
shouldn't really have to.)

Anyway, I'll think of some way to bypass this upcall.  I'm mystified as
to why others aren't seeing this, though.

--b.

> 
> Of course there's no point to the client's callback server doing this
> upcall at all.
> 
> --b.
> 
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<(null)>] (null)
> *pde = 00000000 
> Oops: 0000 [#1] PREEMPT 
> last sysfs file: /sys/kernel/uevent_seqnum
> Modules linked in:
> 
> Pid: 3108, comm: nfsv4.1-svc Tainted: G        W  (2.6.32-rc3 #144) 
> EIP: 0060:[<00000000>] EFLAGS: 00010293 CPU: 0
> EIP is at 0x0
> EAX: c73edd7c EBX: c5d2f8e8 ECX: 00000000 EDX: 00000001
> ESI: c5d2f8d8 EDI: 4aca7522 EBP: c71b1e80 ESP: c71b1e58
>  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
> Process nfsv4.1-svc (pid: 3108, ti=c71b0000 task=c4800020 task.ti=c71b0000)
> Stack:
>  c176f01a c71b1e80 c176f695 c71b1e90 c73edd7c c1aca8a0 fffffff5 c73ed000
> <0> c5d2f8d8 00000000 c71b1eb8 c1768dcf c71b1f30 00000fc4 c1aca7bc 00000246
> <0> c17689e2 00000001 c1aca7bc 00000000 c17c0158 c1aca944 c73ed0c8 00000000
> Call Trace:
>  [<c176f01a>] ? cache_check+0xea/0x350
>  [<c176f695>] ? sunrpc_cache_lookup+0x125/0x140
>  [<c1768dcf>] ? svcauth_unix_accept+0x15f/0x2e0
>  [<c17689e2>] ? svc_authenticate+0x142/0x1a0
>  [<c17c0158>] ? sub_preempt_count+0x8/0x90
>  [<c17689f7>] ? svc_authenticate+0x157/0x1a0
>  [<c17bd877>] ? _spin_unlock_irq+0x27/0x50
>  [<c1764cd3>] ? svc_process_common+0x3f3/0x630
>  [<c1764fd2>] ? bc_svc_process+0xc2/0x100
>  [<c1059d0b>] ? trace_hardirqs_on+0xb/0x10
>  [<c1213487>] ? nfs41_callback_svc+0x87/0x120
>  [<c1049c50>] ? autoremove_wake_function+0x0/0x50
>  [<c1213400>] ? nfs41_callback_svc+0x0/0x120
>  [<c10499a4>] ? kthread+0x74/0x80
>  [<c1049930>] ? kthread+0x0/0x80
>  [<c100363b>] ? kernel_thread_helper+0x7/0x10
> Code:  Bad EIP value.
> EIP: [<00000000>] 0x0 SS:ESP 0068:c71b1e58
> CR2: 0000000000000000
> ---[ end trace 39933fa1a06d9d4b ]---
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 4.1 NULL dereference in 2.6.32-rc3

[J. Bruce Fields]

On Thu, Oct 08, 2009 at 08:20:20PM -0400, J. Bruce Fields wrote:
> On Mon, Oct 05, 2009 at 07:07:36PM -0400, J. Bruce Fields wrote:
> > After mounting and unmounting a 4.1 partition with client and server
> > both 2.6.32-rc3, I see the following NULL dereference on the client.
> > 
> > I think the only cache lookup there is in unix_gid_find().  Hm.
> > Maybe it's trying to defer a request without a defer method set?
> 
> Confirmed.  And I don't see where the client sets any defer method.  (It
> shouldn't really have to.)
> 
> Anyway, I'll think of some way to bypass this upcall.

Actually, it seems sort of wrong to have what's really nfs
server-specific credential mapping in generic rpc cred parsing code.
Maybe we should move that unix_gid_find() into pg_authenticate()
(so svcauth_unix_set_client() in this case).

--b.

> I'm mystified as
> to why others aren't seeing this, though.
> 
> --b.
> 
> > 
> > Of course there's no point to the client's callback server doing this
> > upcall at all.
> > 
> > --b.
> > 
> > BUG: unable to handle kernel NULL pointer dereference at (null)
> > IP: [<(null)>] (null)
> > *pde = 00000000 
> > Oops: 0000 [#1] PREEMPT 
> > last sysfs file: /sys/kernel/uevent_seqnum
> > Modules linked in:
> > 
> > Pid: 3108, comm: nfsv4.1-svc Tainted: G        W  (2.6.32-rc3 #144) 
> > EIP: 0060:[<00000000>] EFLAGS: 00010293 CPU: 0
> > EIP is at 0x0
> > EAX: c73edd7c EBX: c5d2f8e8 ECX: 00000000 EDX: 00000001
> > ESI: c5d2f8d8 EDI: 4aca7522 EBP: c71b1e80 ESP: c71b1e58
> >  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
> > Process nfsv4.1-svc (pid: 3108, ti=c71b0000 task=c4800020 task.ti=c71b0000)
> > Stack:
> >  c176f01a c71b1e80 c176f695 c71b1e90 c73edd7c c1aca8a0 fffffff5 c73ed000
> > <0> c5d2f8d8 00000000 c71b1eb8 c1768dcf c71b1f30 00000fc4 c1aca7bc 00000246
> > <0> c17689e2 00000001 c1aca7bc 00000000 c17c0158 c1aca944 c73ed0c8 00000000
> > Call Trace:
> >  [<c176f01a>] ? cache_check+0xea/0x350
> >  [<c176f695>] ? sunrpc_cache_lookup+0x125/0x140
> >  [<c1768dcf>] ? svcauth_unix_accept+0x15f/0x2e0
> >  [<c17689e2>] ? svc_authenticate+0x142/0x1a0
> >  [<c17c0158>] ? sub_preempt_count+0x8/0x90
> >  [<c17689f7>] ? svc_authenticate+0x157/0x1a0
> >  [<c17bd877>] ? _spin_unlock_irq+0x27/0x50
> >  [<c1764cd3>] ? svc_process_common+0x3f3/0x630
> >  [<c1764fd2>] ? bc_svc_process+0xc2/0x100
> >  [<c1059d0b>] ? trace_hardirqs_on+0xb/0x10
> >  [<c1213487>] ? nfs41_callback_svc+0x87/0x120
> >  [<c1049c50>] ? autoremove_wake_function+0x0/0x50
> >  [<c1213400>] ? nfs41_callback_svc+0x0/0x120
> >  [<c10499a4>] ? kthread+0x74/0x80
> >  [<c1049930>] ? kthread+0x0/0x80
> >  [<c100363b>] ? kernel_thread_helper+0x7/0x10
> > Code:  Bad EIP value.
> > EIP: [<00000000>] 0x0 SS:ESP 0068:c71b1e58
> > CR2: 0000000000000000
> > ---[ end trace 39933fa1a06d9d4b ]---
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 4.1 NULL dereference in 2.6.32-rc3

[J. Bruce Fields]

On Fri, Oct 09, 2009 at 01:02:19PM -0400, J. Bruce Fields wrote:
> On Thu, Oct 08, 2009 at 08:20:20PM -0400, J. Bruce Fields wrote:
> > On Mon, Oct 05, 2009 at 07:07:36PM -0400, J. Bruce Fields wrote:
> > > After mounting and unmounting a 4.1 partition with client and server
> > > both 2.6.32-rc3, I see the following NULL dereference on the client.
> > > 
> > > I think the only cache lookup there is in unix_gid_find().  Hm.
> > > Maybe it's trying to defer a request without a defer method set?
> > 
> > Confirmed.  And I don't see where the client sets any defer method.  (It
> > shouldn't really have to.)
> > 
> > Anyway, I'll think of some way to bypass this upcall.
> 
> Actually, it seems sort of wrong to have what's really nfs
> server-specific credential mapping in generic rpc cred parsing code.
> Maybe we should move that unix_gid_find() into pg_authenticate()
> (so svcauth_unix_set_client() in this case).

Something like this?

It's a little annoying that we then allocate a struct group_info * to
hold the on-the-wire auth_unix groups, even when we'll later discard
them.

But I doubt that's a big deal, and I'd prefer the cleaner nfsd/rpc
separation.

--b.

commit db5566449a9991110e51486cd680dc07772cb728
Author: J. Bruce Fields <bfields@citi.umich.edu>
Date:   Tue Oct 20 18:51:34 2009 -0400

    nfsd4: don't try to map gid's in generic rpc code
    
    For nfsd we provide users the option of mapping uid's to server-side
    supplementary group lists.  That makes sense for nfsd, but not
    necessarily for other rpc users (such as the callback client).
    
    So move that lookup to svcauth_unix_set_client, which is a
    program-specific method.
    
    Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 117f68a..97cc3de 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -655,23 +655,25 @@ static struct unix_gid *unix_gid_lookup(uid_t uid)
 		return NULL;
 }
 
-static int unix_gid_find(uid_t uid, struct group_info **gip,
-			 struct svc_rqst *rqstp)
+static struct group_info *unix_gid_find(uid_t uid, struct svc_rqst *rqstp)
 {
-	struct unix_gid *ug = unix_gid_lookup(uid);
+	struct unix_gid *ug;
+	struct group_info *gi;
+	int ret;
+
+	ug = unix_gid_lookup(uid);
 	if (!ug)
-		return -EAGAIN;
-	switch (cache_check(&unix_gid_cache, &ug->h, &rqstp->rq_chandle)) {
+		return ERR_PTR(-EAGAIN);
+	ret = cache_check(&unix_gid_cache, &ug->h, &rqstp->rq_chandle);
+	switch (ret) {
 	case -ENOENT:
-		*gip = NULL;
-		return 0;
+		return ERR_PTR(-ENOENT);
 	case 0:
-		*gip = ug->gi;
-		get_group_info(*gip);
+		gi = get_group_info(ug->gi);
 		cache_put(&ug->h, &unix_gid_cache);
-		return 0;
+		return gi;
 	default:
-		return -EAGAIN;
+		return ERR_PTR(-EAGAIN);
 	}
 }
 
@@ -681,6 +683,8 @@ svcauth_unix_set_client(struct svc_rqst *rqstp)
 	struct sockaddr_in *sin;
 	struct sockaddr_in6 *sin6, sin6_storage;
 	struct ip_map *ipm;
+	struct group_info *gi;
+	struct svc_cred *cred = &rqstp->rq_cred;
 
 	switch (rqstp->rq_addr.ss_family) {
 	case AF_INET:
@@ -722,6 +726,17 @@ svcauth_unix_set_client(struct svc_rqst *rqstp)
 			ip_map_cached_put(rqstp, ipm);
 			break;
 	}
+
+	gi = unix_gid_find(cred->cr_uid, rqstp);
+	switch (PTR_ERR(gi)) {
+	case -EAGAIN:
+		return SVC_DROP;
+	case -ENOENT:
+		break;
+	default:
+		put_group_info(cred->cr_group_info);
+		cred->cr_group_info = gi;
+	}
 	return SVC_OK;
 }
 
@@ -818,19 +833,11 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *authp)
 	slen = svc_getnl(argv);			/* gids length */
 	if (slen > 16 || (len -= (slen + 2)*4) < 0)
 		goto badcred;
-	if (unix_gid_find(cred->cr_uid, &cred->cr_group_info, rqstp)
-	    == -EAGAIN)
+	cred->cr_group_info = groups_alloc(slen);
+	if (cred->cr_group_info == NULL)
 		return SVC_DROP;
-	if (cred->cr_group_info == NULL) {
-		cred->cr_group_info = groups_alloc(slen);
-		if (cred->cr_group_info == NULL)
-			return SVC_DROP;
-		for (i = 0; i < slen; i++)
-			GROUP_AT(cred->cr_group_info, i) = svc_getnl(argv);
-	} else {
-		for (i = 0; i < slen ; i++)
-			svc_getnl(argv);
-	}
+	for (i = 0; i < slen; i++)
+		GROUP_AT(cred->cr_group_info, i) = svc_getnl(argv);
 	if (svc_getu32(argv) != htonl(RPC_AUTH_NULL) || svc_getu32(argv) != 0) {
 		*authp = rpc_autherr_badverf;
 		return SVC_DENIED;