Instead of using kcred or CRED(): What about changing pc_nodesync(), pc_rele()
and syncpcp() to use an additional cred_t* argument?
syspcp() is either called via:
pcfs_fsync() -> pc_nodesync() -> syncpcp()
or:
pcfs_inactive() -> pc_rele() -> syncpcp()
Both pcfs_fsync() and pcfs_inactive() have the cred_t* arg (and in the
panic I got yesterday, pcfs_inactive was called with a cred_t* != NULL).
> From: Frank Hofmann <[EMAIL PROTECTED]>
> Subject: Re: [ufs-discuss] Re: pcfs NULL pointer dereference panic in
genunix:crgetmapped(), called via pcfs:syncpcp() / 6549510 Need the ability to
store SIDs in the Solaris cred_t
> To: [EMAIL PROTECTED]
> Cc: Juergen Keil <[EMAIL PROTECTED]>, [email protected]
>
>
> Casper's right wrt. to the PCFS change of course, kcred - not CRED() -
> because there's no guarantee syncpcp() will not be called from cred-less
> (interrupt) context.
>
> FrankH.
>
> On Wed, 30 May 2007, [EMAIL PROTECTED] wrote:
>
> >
> >>
> >> With the recent changes for:
> >>
> >> PSARC 2007/064 Unified POSIX and Windows Credentials for Solaris
> >> 4994017 data structure sharing between rpcbind and libnsl leads to
accidents
> >> 6549510 Need the ability to store SIDs in the Solaris cred_t
> >> 6549515 PSARC 2007/064: uid_t and gid_t to become unsigned
> >>
> >>
> >> the opensolaris kernel now panics with a simple file copy from a pcfs
> >> filesystem: cp /path/to/a/file/on/a/pcfs/filesystem /tmp, e.g.
> >> cp /media/MightyDrive/onnv-gate.hg /tmp
> >
> >
> > Yes, I was told about this yesterday evening late and the fix seems
> > fairly simple.
> >
> > In reality it's a bug in pcfs, I think, as it passes a NULL cred_t pointer
to
> > a VOP_*. But since my putback triggered it, I will fix it and I think
> > I may add some defensive code to VOP_XIDMAP.
> >
> >> pc_rele() calls syncpcp(), which calls fop_putpage with a NULL cred_t ?
> >
> > That seems wrong too; it's the only place in the kernel which calls
> > VOP_PUTPAGE with a NULL cred_t.
> >
> > But since these are not well-defined interfaces "what used to work
> > should continue to work".
> >
> > So I am thinking of fixing this as follows:
> >
> > ------- usr/src/uts/common/fs/pcfs/pc_node.c -------
> >
> > 22c22
> > < * Copyright 2006 Sun Microsystems, Inc. All rights reserved.
> > ---
> >> * Copyright 2007 Sun Microsystems, Inc. All rights reserved.
> > 210,211c210
> > < err = VOP_PUTPAGE(PCTOV(pcp), (offset_t)0, (uint_t)0,
> > < flags, (struct cred *)0);
> > ---
> >> err = VOP_PUTPAGE(PCTOV(pcp), 0, 0, flags, kcred);
> >
> > ------- usr/src/uts/common/fs/vnode.c -------
> >
> > 157,159c157,161
> > < vfs_t *vfsp = (vp)->v_vfsp; \
> > < if (vfsp != NULL && (vfsp->vfs_flag & VFS_XID) == 0) \
> > < cr = crgetmapped(cr); \
> > ---
> >> if (cr != NULL) { \
> >> vfs_t *vfsp = (vp)->v_vfsp; \
> >> if (vfsp != NULL && (vfsp->vfs_flag & VFS_XID) == 0) \
> >> cr = crgetmapped(cr); \
> >> } \
> >
> > (the casts in pcfs probably predate the requirement for function prototypes
> > and are no longer needed)
> >
> > I am half of a mind to add an "ASSERT(cr != NULL)" at that point so
> > debug kernels will still trip over this.
> >
> > Casper
> > _______________________________________________
> > ufs-discuss mailing list
> > [email protected]
> >
Juergen Keil [EMAIL PROTECTED]
Tools GmbH +49 (228) 9858011
Vorgebirgsstraße 37-39 http://www.tools.de
53119 BONN
Sitz- und Registergericht HRB Bonn 4026
Geschäftsführung Wolfgang Franke & Wolfgang Solfrank
_______________________________________________
ufs-discuss mailing list
[email protected]
