Re: smbfs kernel panic

[Rumko <rumcic@xxxxxxxxx>]

I'll do that probably around the 1st July ... need to update to latest HEAD
anyway, but dont have the time to mess with this time right now (exams).

On 2005-06-04, Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx> wrote:
>     Please try this patch set.  I'm not sure whether SMBFS can
>     safely use the root creds for file closes, which its
>     inactive code has to do, so I cache the cred used in the
>     first open and use that for the close.
>
>     When I redid the VFS I wanted it to work from pure threads.
>     Most VFSs don't need creds for inactive and choosing which
>     cred to use (because multiple user id's might have had the
>     file open) is rather non-deterministic... so I simply don't
>     make the cred information available.  This means that the
>     three filesystems that need it (ntfs, smbfs, nfs) need to do
>     their own heuristic which typically involves caching the open
>     creds in the auxillary node structure to supply to the close.
>
>     This is mainly due to mistakes made in the protocol 
>     specification.  For example, NFS servers should always accept
>     root creds to mean 'the greatest access the server allows
>     that client to have', but they often don't and instead for user mounts
>     only allow that particular user cred.  It's stupid but it's the way
>     the cookie crumbles.
>
> 						-Matt
>
> Index: smbfs_node.c
>===================================================================
> RCS file: /cvs/src/sys/vfs/smbfs/smbfs_node.c,v
> retrieving revision 1.16
> diff -u -r1.16 smbfs_node.c
> --- smbfs_node.c	17 Dec 2004 00:18:35 -0000	1.16
> +++ smbfs_node.c	4 Jun 2005 17:42:37 -0000
> @@ -346,16 +346,16 @@
>  	struct smb_cred scred;
>  	int error;
>  
> -	KKASSERT(td->td_proc);
> -	cred = td->td_proc->p_ucred;
> -
>  	SMBVDEBUG("%s: %d\n", VTOSMB(vp)->n_name, vp->v_usecount);
>  	if (np->n_opencount) {
>  		error = smbfs_vinvalbuf(vp, V_SAVE, td, 1);
> +		cred = np->n_cached_cred;
> +		np->n_cached_cred = NULL;
>  		smb_makescred(&scred, td, cred);
>  		error = smbfs_smb_close(np->n_mount->sm_share, np->n_fid, 
>  		   &np->n_mtime, &scred);
>  		np->n_opencount = 0;
> +		crfree(cred);
>  	}
>  	return (0);
>  }
> Index: smbfs_node.h
>===================================================================
> RCS file: /cvs/src/sys/vfs/smbfs/smbfs_node.h,v
> retrieving revision 1.4
> diff -u -r1.4 smbfs_node.h
> --- smbfs_node.h	28 Aug 2004 19:02:28 -0000	1.4
> +++ smbfs_node.h	4 Jun 2005 17:35:40 -0000
> @@ -61,6 +61,7 @@
>  	long			n_ino;
>  	int			n_dosattr;
>  	int 			n_opencount;
> +	struct ucred		*n_cached_cred;	/* cred used to open file */
>  	u_int16_t		n_fid;		/* file handle */
>  	int			n_rwstate;	/* granted access mode */
>  	u_char			n_nmlen;
> Index: smbfs_vfsops.c
>===================================================================
> RCS file: /cvs/src/sys/vfs/smbfs/smbfs_vfsops.c,v
> retrieving revision 1.20
> diff -u -r1.20 smbfs_vfsops.c
> --- smbfs_vfsops.c	20 Apr 2005 17:01:56 -0000	1.20
> +++ smbfs_vfsops.c	4 Jun 2005 17:45:16 -0000
> @@ -144,8 +144,10 @@
>  	int error;
>  	char *pc, *pe;
>  
> -	KKASSERT(td->td_proc);
> -	cred = td->td_proc->p_ucred;
> +	if (td->td_proc)
> +	    cred = td->td_proc->p_ucred;
> +	else
> +	    cred = proc0.p_ucred;
>  
>  	if (data == NULL) {
>  		printf("missing data argument\n");
> @@ -254,8 +256,10 @@
>  	struct ucred *cred;
>  	int error, flags;
>  
> -	KKASSERT(td->td_proc);
> -	cred = td->td_proc->p_ucred;
> +	if (td->td_proc)
> +	    cred = td->td_proc->p_ucred;
> +	else
> +	    cred = proc0.p_ucred;
>  
>  	SMBVDEBUG("smbfs_unmount: flags=%04x\n", mntflags);
>  	flags = 0;
> @@ -307,9 +311,6 @@
>  	struct smb_cred scred;
>  	int error;
>  
> -	KKASSERT(td->td_proc);
> -	cred = td->td_proc->p_ucred;
> -
>  	if (smp == NULL) {
>  		SMBERROR("smp == NULL (bug in umount)\n");
>  		return EINVAL;
> @@ -318,6 +319,11 @@
>  		*vpp = SMBTOV(smp->sm_root);
>  		return vget(*vpp, LK_EXCLUSIVE | LK_RETRY, td);
>  	}
> +	if (td->td_proc)
> +		cred = td->td_proc->p_ucred;
> +	else
> +		cred = proc0.p_ucred;
> +
>  	smb_makescred(&scred, td, cred);
>  	error = smbfs_smb_lookup(NULL, NULL, 0, &fattr, &scred);
>  	if (error)
> @@ -388,8 +394,10 @@
>  	struct ucred *cred;
>  	int error = 0;
>  
> -	KKASSERT(td->td_proc);
> -	cred = td->td_proc->p_ucred;
> +	if (td->td_proc)
> +	    cred = td->td_proc->p_ucred;
> +	else
> +	    cred = proc0.p_ucred;
>  
>  	if (np == NULL)
>  		return EINVAL;
> Index: smbfs_vnops.c
>===================================================================
> RCS file: /cvs/src/sys/vfs/smbfs/smbfs_vnops.c,v
> retrieving revision 1.22
> diff -u -r1.22 smbfs_vnops.c
> --- smbfs_vnops.c	15 Feb 2005 08:32:18 -0000	1.22
> +++ smbfs_vnops.c	4 Jun 2005 17:42:21 -0000
> @@ -178,6 +178,8 @@
>  		return EACCES;
>  	}
>  	if (vp->v_type == VDIR) {
> +		if (np->n_opencount == 0)
> +			np->n_cached_cred = crhold(ap->a_cred);
>  		np->n_opencount++;
>  		return 0;
>  	}
> @@ -216,6 +218,7 @@
>  		error = smbfs_smb_open(np, accmode, &scred);
>  	}
>  	if (!error) {
> +		np->n_cached_cred = crhold(ap->a_cred);
>  		np->n_opencount++;
>  	}
>  	smbfs_attr_cacheremove(vp);
> @@ -258,6 +261,8 @@
>  		error = smbfs_smb_close(np->n_mount->sm_share, np->n_fid, 
>  			   &np->n_mtime, &scred);
>  	}
> +	crfree(np->n_cached_cred);
> +	np->n_cached_cred = NULL;
>  	smbfs_attr_cacheremove(vp);
>  	return error;
>  }