Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 9 Sep 2013 10:11:12 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Oliver Pinter <oliver.pntr@gmail.com>
Cc:        stable@freebsd.org
Subject:   Re: 9.2-STABLE: supervisor read data, page not present
Message-ID:  <20130909071112.GI41229@kib.kiev.ua>
In-Reply-To: <CAPjTQNHe8=Q4OH5vfqTHWdUQJXkBfM4Xw3a=Z85=m8NYvxQP6Q@mail.gmail.com>
References:  <CAPjTQNFxUFAUx9U3heVgDH9D8_TJ9NuOwcwUfSEi_RegQ5-wpw@mail.gmail.com> <CAPjTQNGusVYJpUxb-uuUwBD%2B8tJijLAB6u=pbFWvbE0%2B19-RXQ@mail.gmail.com> <20130909051444.GG41229@kib.kiev.ua> <CAPjTQNHe8=Q4OH5vfqTHWdUQJXkBfM4Xw3a=Z85=m8NYvxQP6Q@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
On Mon, Sep 09, 2013 at 08:45:21AM +0200, Oliver Pinter wrote:
> On 9/9/13, Konstantin Belousov <kostikbel@gmail.com> wrote:
> > On Sun, Sep 08, 2013 at 11:40:01PM +0200, Oliver Pinter wrote:
> >> #6  0xffffffff806a2ab3 in cpuctl_ioctl (dev=<value optimized out>,
> >>     cmd=<value optimized out>, data=<value optimized out>, flags=0,
> >>     td=<value optimized out>) at /usr/src/sys/dev/cpuctl/cpuctl.c:478
> >> 	cpu = <value optimized out>
> >> 	ret = <value optimized out>
> >
> > Do you indeed posses VIA CPU ?  Was it due to some violence act ?
> 
> Nope, this is an Intel Q9300.
> 
> FreeBSD clang version 3.3 (tags/RELEASE_33/final 183502) 20130610
> CPU: Intel(R) Core(TM)2 Quad  CPU   Q9300  @ 2.50GHz (2499.76-MHz K8-class CPU)
>   Origin = "GenuineIntel"  Id = 0x10677  Family = 0x6  Model = 0x17
> Stepping = 7
>   Features=0xbfebfbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CLFLUSH,DTS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE>
>   Features2=0x8e3fd<SSE3,DTES64,MON,DS_CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1>
>   AMD Features=0x20100800<SYSCALL,NX,LM>
>   AMD Features2=0x1<LAHF>
>   TSC: P-state invariant, performance statistics
> real memory  = 4294967296 (4096 MB)
> avail memory = 4103024640 (3912 MB)
> Event timer "LAPIC" quality 400
> ACPI APIC Table: <A_M_I_ OEMAPIC >
> FreeBSD/SMP: Multiprocessor System Detected: 4 CPUs
> FreeBSD/SMP: 1 package(s) x 4 core(s)
>  cpu0 (BSP): APIC ID:  0
>  cpu1 (AP): APIC ID:  1
>  cpu2 (AP): APIC ID:  2
>  cpu3 (AP): APIC ID:  3
Intel update code has the same issue.  Still, it is weird that the debugger
reported the line from the update_via().

> 
> 
> >
> > I am not sure about the first panic, lets fix the malloc/free corruption
> > and see. The proc_reap() issue might be a consequence of the memory
> > corruption from the wrong free().
> >
> > There is no public documentation for VIA CPUs, at least I was not
> > able to find anything when I looked. According to the comment in the
> > update_via(), all what is needed is that update buffer was 4-bytes
> > aligned, which is always guaranteed by our malloc(9), at least for the
> > allocation of size >=4.
> >
> > Try this.

Updated patch.

diff --git a/sys/dev/cpuctl/cpuctl.c b/sys/dev/cpuctl/cpuctl.c
index 4e5abb2..317fc08 100644
--- a/sys/dev/cpuctl/cpuctl.c
+++ b/sys/dev/cpuctl/cpuctl.c
@@ -295,10 +295,10 @@ cpuctl_do_update(int cpu, cpuctl_update_args_t *data, struct thread *td)
 static int
 update_intel(int cpu, cpuctl_update_args_t *args, struct thread *td)
 {
-	void *ptr = NULL;
+	void *ptr;
 	uint64_t rev0, rev1;
 	uint32_t tmp[4];
-	int is_bound = 0;
+	int is_bound;
 	int oldcpu;
 	int ret;
 
@@ -312,10 +312,11 @@ update_intel(int cpu, cpuctl_update_args_t *args, struct thread *td)
 	}
 
 	/*
-	 * 16 byte alignment required.
+	 * 16 byte alignment required.  Rely on the fact that
+	 * malloc(9) always returns the pointer aligned at least on
+	 * the size of the allocation.
 	 */
 	ptr = malloc(args->size + 16, M_CPUCTL, M_WAITOK);
-	ptr = (void *)(16 + ((intptr_t)ptr & ~0xf));
 	if (copyin(args->data, ptr, args->size) != 0) {
 		DPRINTF("[cpuctl,%d]: copyin %p->%p of %zd bytes failed",
 		    __LINE__, args->data, ptr, args->size);
@@ -408,10 +409,10 @@ fail:
 static int
 update_via(int cpu, cpuctl_update_args_t *args, struct thread *td)
 {
-	void *ptr = NULL;
+	void *ptr;
 	uint64_t rev0, rev1, res;
 	uint32_t tmp[4];
-	int is_bound = 0;
+	int is_bound;
 	int oldcpu;
 	int ret;
 
@@ -427,8 +428,7 @@ update_via(int cpu, cpuctl_update_args_t *args, struct thread *td)
 	/*
 	 * 4 byte alignment required.
 	 */
-	ptr = malloc(args->size + 16, M_CPUCTL, M_WAITOK);
-	ptr = (void *)(16 + ((intptr_t)ptr & ~0xf));
+	ptr = malloc(args->size, M_CPUCTL, M_WAITOK);
 	if (copyin(args->data, ptr, args->size) != 0) {
 		DPRINTF("[cpuctl,%d]: copyin %p->%p of %zd bytes failed",
 		    __LINE__, args->data, ptr, args->size);

[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.21 (FreeBSD)

iQIcBAEBAgAGBQJSLXSPAAoJEJDCuSvBvK1Bv84P/3cmggIntT24YsCN9/qnt1a5
px4mDPkRyMH+Up+R2FOdewryiKdstDi61RtA/4C4WPBe8cqork6x9V5RHzwTnSvU
3YAgtv/PSK5ABMtwv2IgfejzAowGivwxpFYj+i+jibq4DpiF56upS8iElQMI/HQi
uL7JX5zDWZE7ePMWIE3HZddRahKUyJUrHLdCdaUJyNen77V/sOSPuDalvvNbrrAJ
qh9JWkZvmvgH/ei3iKLGa+GWVYD/w5KvfL1MAoLJyatVque0Sakn3JCb1+8dmVgZ
68lQv4v3kCThEZlM7y1QTxE6iCBx7x1IHQYBhNyd4JtgCzJuqe45fpvtCq9snmyf
j83jMEGya5+LWBnNtxubtCexNt57KEM4C14T4A0wNq7gJ1CjylegZ5ppG/BCVpEj
WB63QZCmtdfdaYAblbnCB1uq2E7YKKxCzbx/hi1xRRLa8cWKJt7fxKvxjqpK8t78
mM6yKktes8pGQxBLJAqv39aCOdKcVLK9SxN0lx1mi2E3QV/vqaaekpXxAm/VZ9RK
qJciPYeSXzuZMT66E/4GO2pWGo9G6d59GFS5VlhuxurVIxHf5aCpusKv0SNNW+ON
kpQjTVUsHFp940tSQIWr6MqjI3MeHQeK+CDU39ERAeJZWdn0hwH/tQIpKZCiS+Df
4kU/s/rIAU67/vBds6AK
=1qPV
-----END PGP SIGNATURE-----

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130909071112.GI41229>