On Wed, 30 May 2007, Juergen Keil wrote:
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).
Not an unreasonable idea, but if we're talking changing the function
interfacing then I'd propose we'll purge syncpcp() completely and inline
the VOP_PUTPAGE() call into the callers. pcfs_putpage() does the
vn_has_cached_data() check as well, so there's no real need for syncpcp()
at all.
For the time being (to get a quick fix), I'm more inclined for the
single-line change. There's more that's buggy in the sync/rele codepaths
of PCFS, it'll all break loose once the global lock is removed. We won't
run out of bugs there any time soon ;-)
FrankH.
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]
