Date: Sat, 12 Jan 2002 03:28:44 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Jake Burkholder <jake@locore.ca> Cc: <arch@freebsd.org>, <marcel@freebsd.org> Subject: stackgap lossage (was: cvs commit: src/sys/i386/i386 trap.c) Message-ID: <20020112031708.D2622-100000@gamplex.bde.org> In-Reply-To: <20020101121559.D9752@locore.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
[Redirected from cvs-committers]
On Tue, 1 Jan 2002, Jake Burkholder wrote:
> Apparently, On Tue, Jan 01, 2002 at 04:49:17AM +1100,
> Bruce Evans said words to the effect of;
>
> > On Sun, 30 Dec 2001, Jake Burkholder wrote:
> >
> > > Apparently, On Sun, Dec 30, 2001 at 11:43:59AM -0800,
> > > Poul-Henning Kamp said words to the effect of;
> > >
> > > > phk 2001/12/30 11:43:59 PST
> > > >
> > > > Modified files:
> > > > sys/i386/i386 trap.c
> > > > Log:
> > > > GC an alternate trap_pfault() which has rotted away behind an "#ifdef notyet"
> > > > since 21-Mar-95 .
> > > >
> > > > Revision Changes Path
> > > > 1.210 +0 -118 src/sys/i386/i386/trap.c
> > >
> > > This is the one that should have been used. The existing one is broken because
> > > it allows faults on user memory in kernel mode without using copy{in,out} or
> > > {f,s}uword. Apparently there is still broken kernel code that expects this to
> > > work.
> >
> > I agree. Some of the bitrot was fixed in my local version, but I never
> > got around to testing it.
> >
> > What do the other arches do? They all seem to be based on the ancient
> > version that did trap_pfault() inline in trap(). The i386 version stopped
> > doing this in rev.1.25 (1994/06/06).
>
> The ia64 and alpha ones seem to be, yes. But they do correctly disallow these
> faults, except for in the stackgap used by the linux emulator. The sparc64
> version is based on the non-broken i386 one. Special instructions must be
> used to access user space, so copy{in,out} are mandatory.
The sparc64 version is much cleaner.
I have run on i386's with faults disallowed for a while and finally
hit a problem when I tried Linux netscape. The stackgap stuff is evil.
Pointers into the stackgap are passed to utility functions that expect
ordinary pointers. I fixed just enough to get Linux netscape to work.
The comment has some more details.
%%%
Index: linux_signal.c
===================================================================
RCS file: /home/ncvs/src/sys/compat/linux/linux_signal.c,v
retrieving revision 1.32
diff -u -2 -r1.32 linux_signal.c
--- linux_signal.c 12 Sep 2001 08:36:57 -0000 1.32
+++ linux_signal.c 11 Jan 2002 01:04:59 -0000
@@ -85,6 +85,39 @@
static void
-linux_to_bsd_sigaction(l_sigaction_t *lsa, struct sigaction *bsa)
+linux_to_bsd_sigaction(l_sigaction_t *lsa_xxx, struct sigaction *bsa_xxx)
{
+ struct sigaction bsa_np;
+ l_sigaction_t lsa_np;
+ struct sigaction *bsa;
+ l_sigaction_t *lsa;
+
+ /*
+ * XXX sigactions tend to be in the stack gap, in which case they
+ * must be copied in or out.
+ *
+ * Direct accesses caused fatal pagefaults when the pagefault handler
+ * was fixed to make pagefaults for direct accesses to user memory
+ * fatal. This check doesn't detect invalid accessed to pages that
+ * have already been mapped, and copying probably weakens the check
+ * by mapping the whole stack gap. It's surprising that the whole
+ * stack gap isn't always mapped by a previous copying. Who knows
+ * how many invalid access we don't detect.
+ *
+ * XXX bsa and lsa are bogusly named to avoid changing everything.
+ *
+ * XXX l_sigaction_t is a style violation.
+ *
+ * The VM_MAXUSER_ADDRESS check is a cheap (probably wrong) version
+ * of useracc().
+ */
+ bsa = &bsa_np;
+ bzero(bsa, sizeof(*bsa));
+ if ((uintptr_t)lsa_xxx >= VM_MAXUSER_ADDRESS)
+ lsa = lsa_xxx;
+ else {
+ lsa = &lsa_np;
+ if (copyin(lsa_xxx, lsa, sizeof(*lsa)) != 0)
+ panic("aieeee 1");
+ }
linux_to_bsd_sigset(&lsa->lsa_mask, &bsa->sa_mask);
@@ -105,9 +138,30 @@
if (lsa->lsa_flags & LINUX_SA_NOMASK)
bsa->sa_flags |= SA_NODEFER;
+
+ if ((uintptr_t)bsa_xxx >= VM_MAXUSER_ADDRESS)
+ *bsa_xxx = *bsa;
+ else {
+ if (copyout(bsa, bsa_xxx, sizeof(*bsa_xxx)) != 0)
+ panic("aieeee 2");
+ }
}
static void
-bsd_to_linux_sigaction(struct sigaction *bsa, l_sigaction_t *lsa)
+bsd_to_linux_sigaction(struct sigaction *bsa_xxx, l_sigaction_t *lsa_xxx)
{
+ struct sigaction bsa_np;
+ l_sigaction_t lsa_np;
+ struct sigaction *bsa;
+ l_sigaction_t *lsa;
+
+ if ((uintptr_t)bsa_xxx >= VM_MAXUSER_ADDRESS)
+ bsa = bsa_xxx;
+ else {
+ bsa = &bsa_np;
+ if (copyin(bsa_xxx, bsa, sizeof(*bsa)) != 0)
+ panic("aieeee 3");
+ }
+ lsa = &lsa_np;
+ bzero(lsa, sizeof(*lsa));
bsd_to_linux_sigset(&bsa->sa_mask, &lsa->lsa_mask);
@@ -129,4 +183,11 @@
if (bsa->sa_flags & SA_NODEFER)
lsa->lsa_flags |= LINUX_SA_NOMASK;
+
+ if ((uintptr_t)lsa_xxx >= VM_MAXUSER_ADDRESS)
+ *lsa_xxx = *lsa;
+ else {
+ if (copyout(lsa, lsa_xxx, sizeof(*lsa_xxx)) != 0)
+ panic("aieeee 3");
+ }
}
%%%
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020112031708.D2622-100000>