Re: fix for use-after-free problem in 10.x [review please].

2016-10-05 Thread Julian Elischer 

Please review..
https://reviews.freebsd.org/D8160
Direct fix for stable/10 as bug is not present in 11+ in this form.

Julian


On 4/10/2016 8:06 PM, Julian Elischer wrote:
In 11 and 12 the taskqueue code has been rewritten in this area but 
under 10 this bug still occurs.


On our appliances this bug stops the system from mounting the ZFS 
root, so it is quite severe.
Basically while the thread is sleeping during the ZFS mount of root 
(in the while loop), another thread can free the 'task' item it is 
checking in that while loop and it can be reused or filled with 
'deadcode' etc., with the waiting code unaware of the change.. The 
fix is to refetch the item at the end of the queue each time around 
the loop.
I don't really want to do the bigger change of MFCing the change in 
11, as it is more extensive, though if someone else does, that's ok 
by me. (If it's ABI compatible)


Any comments or suggestions?

here's the fix in diff form:


A slightly better fix is at
https://reviews.freebsd.org/D8160




[robot@porridge /usr/src]$ p4 diff -du ...
--- 
//depot/pbranches/jelischer/FreeBSD-PZ/10.3/sys/kern/subr_taskqueue.c 
2016-09-27 09:14:59.0 -0700
+++ /usr/src/sys/kern/subr_taskqueue.c  2016-09-27 
09:14:59.0 -0700

@@ -441,9 +441,10 @@

TQ_LOCK(queue);
task = STAILQ_LAST(>tq_queue, task, ta_link);
-   if (task != NULL)
-   while (task->ta_pending != 0)
-   TQ_SLEEP(queue, task, >tq_mutex, 
PWAIT, "-", 0);

+   while (task != NULL && task->ta_pending != 0) {
+   TQ_SLEEP(queue, task, >tq_mutex, PWAIT, "-", 0);
+   task = STAILQ_LAST(>tq_queue, task, ta_link);
+   }
taskqueue_drain_running(queue);
KASSERT(STAILQ_EMPTY(>tq_queue),
("taskqueue queue is not empty after draining"));



___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: aibs(4) / atk0110 support for newer systems

2016-10-05 Thread Andriy Gapon 
On 05/10/2016 23:28, Torfinn Ingolfsen wrote:
> #6  0x80cd0081 in calltrap ()
> at /usr/src/sys/amd64/amd64/exception.S:238
> #7  0x81bcb078 in aibs_add_sensor () from /boot/kernel/aibs.ko
> #8  0x81bcb4b4 in aibs_attach_sif () from /boot/kernel/aibs.ko

Argh, I've just spotted a very silly typo.
Could you please replace '0' with 'o' in
err = aibs_add_sensor(sc, 0, [i], );
?
I wish compilers were more noisy about passing a scalar as a pointer argument.

Thanks!
-- 
Andriy Gapon
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: aibs(4) / atk0110 support for newer systems

2016-10-05 Thread Andriy Gapon 
On 05/10/2016 23:28, Torfinn Ingolfsen wrote:
> On Wed, 5 Oct 2016 16:09:40 +0300
> Andriy Gapon  wrote:
> 
>>
>> Seems like this is a clang vs gcc issue as I didn't get warnings here.
>> Could you please simply add initialization to those variables in 
>> aibs_attach_ggrp()?
>> E.g.:
>> s_idx = NULL;
>> so = NULL;
> 
> I ended up adding
> s_idx = NULL;
> so = NULL;
> name = NULL;
> 
> to aibs_attach_ggrp(), that made the module compile.

Okay, thank for letting me know.

> Unfortunately, my machines panics as soon as this module is loaded:

Not good...

> root@kg-core1# cat /var/crash/info.0
> Dump header from device /dev/ada0p3
>   Architecture: amd64
>   Architecture Version: 2
>   Dump Length: 1466040320B (1398 MB)
>   Blocksize: 512
>   Dumptime: Wed Oct  5 21:48:42 2016
>   Hostname: kg-core1.kg4.no
>   Magic: FreeBSD Kernel Dump
>   Version String: FreeBSD 9.3-STABLE #3 r304838: Fri Aug 26 12:11:25 CEST 2016
> r...@kg-core1.kg4.no:/usr/obj/usr/src/sys/GENERIC
>   Panic String: page fault
>   Dump Parity: 1914870814
>   Bounds: 0
>   Dump Status: good
> 
> core.txt.0 attached
> HTH

Seems like the module got built without debug symbols.
Please try
make KERNBUILDDIR=/usr/obj/...


-- 
Andriy Gapon
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: aibs(4) / atk0110 support for newer systems

2016-10-05 Thread Andriy Gapon 
On 05/10/2016 23:28, Torfinn Ingolfsen wrote:
> Unfortunately, my machines panics as soon as this module is loaded:

Not good...

> (kgdb) #0  doadump (textdump=) at pcpu.h:235
> #1  0x80900b06 in kern_reboot (howto=260)
> at /usr/src/sys/kern/kern_shutdown.c:454
> #2  0x80901007 in panic (fmt=0x1 )
> at /usr/src/sys/kern/kern_shutdown.c:642
> #3  0x80ce6887 in trap_fatal (frame=0xc, eva=)
> at /usr/src/sys/amd64/amd64/trap.c:876
> #4  0x80ce6bd1 in trap_pfault (frame=0xff88b8554380, usermode=0)
> at /usr/src/sys/amd64/amd64/trap.c:798
> #5  0x80ce7199 in trap (frame=0xff88b8554380)
> at /usr/src/sys/amd64/amd64/trap.c:462
> #6  0x80cd0081 in calltrap ()
> at /usr/src/sys/amd64/amd64/exception.S:238
> #7  0x81bcb078 in aibs_add_sensor () from /boot/kernel/aibs.ko
> #8  0x81bcb4b4 in aibs_attach_sif () from /boot/kernel/aibs.ko
> #9  0x81bcb901 in aibs_attach () from /boot/kernel/aibs.ko

I see that line numbers are not reported for the module, so it probably lacks
debug symbols.  But it seems that you compile your kernel with them.
Please try to compile the module using
make KERNBUILDDIR=/usr/obj/...


> #10 0x8093424c in device_attach (dev=0x0) at device_if.h:180
> #11 0x8033bd43 in acpi_driver_added (dev=, 
> driver=) at /usr/src/sys/dev/acpica/acpi.c:841
> #12 0x809323d5 in devclass_driver_added (dc=0xfe00093c5900, 
> driver=0x81bcbe9c) at bus_if.h:204
> #13 0x80932f13 in devclass_add_driver (dc=0xfe00093c5900, 
> driver=0x81bcc140, pass=2147483647, dcp=0x81bcc1e0)
> at /usr/src/sys/kern/subr_bus.c:1086
> #14 0x808ebef8 in module_register_init (arg=)
> at /usr/src/sys/kern/kern_module.c:123
> #15 0x808e38ae in linker_load_module (kldname=, 
> modname=0xfe03ec774000 "aibs", parent=0x0, verinfo=0x0, 
> lfpp=0xff88b8554aa0) at /usr/src/sys/kern/kern_linker.c:233
> #16 0x808e3fc4 in kern_kldload (td=, 
> file=0xfe03ec774000 "aibs", fileid=0xff88b8554af4)
> at /usr/src/sys/kern/kern_linker.c:1038
> #17 0x808e40d4 in sys_kldload (td=0xfe020f434490, 
> uap=) at /usr/src/sys/kern/kern_linker.c:1064
> #18 0x80ce5ffa in amd64_syscall (td=0xfe020f434490, traced=0)
> at subr_syscall.c:142
> #19 0x80cd0367 in Xfast_syscall ()
> at /usr/src/sys/amd64/amd64/exception.S:398
> #20 0x00080085d10c in ?? ()
> Previous frame inner to this frame (corrupt stack?)
> (kgdb) 

___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: Reproducible panic - Going nowhere without my init!

2016-10-05 Thread Andy Farkas 
On 05/10/2016 23:36, Konstantin Belousov wrote:

> Please try this variation, I want to see if the error code changed.

Afraid not. Still signal 0, exit 0.

Screenshot:  http://imgur.com/AU6weU0

-andyf
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

iicsmb

2016-10-05 Thread Andriy Gapon 

Does anyone use iicsmb driver for any practical purposes?
Or more broadly, does anyone have a system with an I2C controller behind which
SMBus-compatible slaves are known to exist?

-- 
Andriy Gapon
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: Reproducible panic - Going nowhere without my init!

2016-10-05 Thread Konstantin Belousov 
On Wed, Oct 05, 2016 at 07:32:33PM +1000, Andy Farkas wrote:
> On 05/10/2016 18:43, Konstantin Belousov wrote:
> 
> > Apply the following patch.  I am interested if anything additional appear
> > on the console.  Screenshot is good enough.
> 
> Patch applied. Panic (easlily!) reproduced. No additional output.
> 
> Screenshot:  http://imgur.com/KOOBysH
> 
> I guess init is dying before it gets there.

No, init does not die in your case, since error code is zero, and the
termination signal is absent.  It must occur because init explicitely
called _exit(0).

Please try this variation, I want to see if the error code changed.

diff --git a/sbin/init/init.c b/sbin/init/init.c
index bda86b5..1e88964 100644
--- a/sbin/init/init.c
+++ b/sbin/init/init.c
@@ -884,8 +884,13 @@ single_user(void)
if (Reboot) {
/* Instead of going single user, let's reboot the machine */
sync();
-   reboot(howto);
-   _exit(0);
+   if (reboot(howto) == -1) {
+   emergency("reboot(%#x) failed, %s", howto,
+   strerror(errno));
+   _exit(1); /* panic and reboot */
+   }
+   warning("reboot(%#x) returned", howto);
+   _exit(97); /* panic as well */
}
 
shell = get_shell();
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: aibs(4) / atk0110 support for newer systems

2016-10-05 Thread Andriy Gapon 
On 05/10/2016 15:37, Torfinn Ingolfsen wrote:
> On Mon, 3 Oct 2016 23:05:48 +0300
> Andriy Gapon  wrote:
> 
>> Yes, it does.  Thank you!
>> It seems like a couple of minor changes are not in the source tree that you 
>> are
>> using.  One is some casts in a diagnostic printf and the other is a different
>> rounding of 0C in Kelvins.
>> I've generated a patch that should apply to your tree:
>> https://people.freebsd.org/~avg/aibs-ggrp-gitm.93.diff
>> Please try.
> 
> The patch applied cleanly (I removed the old one with patch -R first):

Good!

> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c: In 
> function 'aibs_attach':
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: 
> warning: 's_idx' may be used uninitialized in this function
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: note: 
> 's_idx' was declared here
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: 
> warning: 'so' may be used uninitialized in this function
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: note: 
> 'so' was declared here
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: 
> warning: 'name' may be used uninitialized in this function
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: note: 
> 'name' was declared here
> *** [atk0110.o] Error code 1
> 
> Stop in /usr/src/sys/modules/acpi/aibs.
> 
> Do I have to do something more in order to build the new module?
> 

Seems like this is a clang vs gcc issue as I didn't get warnings here.
Could you please simply add initialization to those variables in 
aibs_attach_ggrp()?
E.g.:
s_idx = NULL;
so = NULL;
Thank you!

-- 
Andriy Gapon
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: aibs(4) / atk0110 support for newer systems

2016-10-05 Thread Torfinn Ingolfsen 
On Mon, 3 Oct 2016 23:05:48 +0300
Andriy Gapon  wrote:

> Yes, it does.  Thank you!
> It seems like a couple of minor changes are not in the source tree that you 
> are
> using.  One is some casts in a diagnostic printf and the other is a different
> rounding of 0C in Kelvins.
> I've generated a patch that should apply to your tree:
> https://people.freebsd.org/~avg/aibs-ggrp-gitm.93.diff
> Please try.

The patch applied cleanly (I removed the old one with patch -R first):
root@kg-core1# patch  -p0 < /home/tingo/dl/aibs-ggrp-gitm.93.diff
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--
|Index: sys/dev/acpi_support/atk0110.c
|===
|--- sys/dev/acpi_support/atk0110.c(revision 306109)
|+++ sys/dev/acpi_support/atk0110.c(working copy)
--
Patching file sys/dev/acpi_support/atk0110.c using Plan A...
Hunk #1 succeeded at 28.
Hunk #2 succeeded at 52.
Hunk #3 succeeded at 78.
Hunk #4 succeeded at 91.
Hunk #5 succeeded at 124.
Hunk #6 succeeded at 134.
Hunk #7 succeeded at 362.
Hunk #8 succeeded at 370.
Hunk #9 succeeded at 377.
Hunk #10 succeeded at 391.
Hunk #11 succeeded at 435.
Hunk #12 succeeded at 450.
Hunk #13 succeeded at 457.
Hunk #14 succeeded at 488.
Hunk #15 succeeded at 495.
done

However, build fails:
root@kg-core1# pwd
/sys/modules/acpi/aibs
root@kg-core1# make
Warning: Object directory not changed from original 
/usr/src/sys/modules/acpi/aibs
@ -> /usr/src/sys
machine -> /usr/src/sys/amd64/include
x86 -> /usr/src/sys/x86/include
:> opt_acpi.h
awk -f @/tools/makeobjops.awk @/dev/acpica/acpi_if.m -h
awk -f @/tools/makeobjops.awk @/kern/bus_if.m -h
awk -f @/tools/makeobjops.awk @/kern/device_if.m -h
:> opt_ddb.h
cc -O2 -pipe -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE -nostdinc   
-I. -I@ -I@/contrib/altq -finline-limit=8000 --param inline-unit-growth=100 
--param large-function-growth=1000 -fno-common  -fno-omit-frame-pointer 
-mno-omit-leaf-frame-pointer  -mcmodel=kernel -mno-red-zone -mno-mmx -mno-sse 
-msoft-float  -fno-asynchronous-unwind-tables -ffreestanding -fstack-protector 
-std=iso9899:1999  -fstack-protector -Wall -Wredundant-decls -Wnested-externs 
-Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  
-Wundef -Wno-pointer-sign -fformat-extensions  -Wmissing-include-dirs 
-fdiagnostics-show-option   -c 
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c -o atk0110.o
cc1: warnings being treated as errors
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c: In function 
'aibs_attach':
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: 
warning: 's_idx' may be used uninitialized in this function
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: note: 
's_idx' was declared here
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: 
warning: 'so' may be used uninitialized in this function
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: note: 
'so' was declared here
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: 
warning: 'name' may be used uninitialized in this function
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: note: 
'name' was declared here
*** [atk0110.o] Error code 1

Stop in /usr/src/sys/modules/acpi/aibs.

Do I have to do something more in order to build the new module?
-- 
Torfinn Ingolfsen 
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: Reproducible panic - Going nowhere without my init!

2016-10-05 Thread Andy Farkas 
On 05/10/2016 18:43, Konstantin Belousov wrote:

> Apply the following patch.  I am interested if anything additional appear
> on the console.  Screenshot is good enough.

Patch applied. Panic (easlily!) reproduced. No additional output.

Screenshot:  http://imgur.com/KOOBysH

I guess init is dying before it gets there.

-andyf
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Re: Reproducible panic - Going nowhere without my init!

2016-10-05 Thread Konstantin Belousov 
On Wed, Oct 05, 2016 at 05:32:18AM +1000, Andy Farkas wrote:
> On 04/10/2016 23:11, Andy Farkas wrote:
> > On 04/10/2016 21:24, Konstantin Belousov wrote:
> >> On Tue, Oct 04, 2016 at 11:14:38AM +1000, Andy Farkas wrote:
> >>> Is it just me or
> >>>
> >>> Step 1: boot
> >>> Step 2: login as root
> >>> Step 3: type "w" *
> >>> Step 4: type "shutdown now; logout"
> >>> Step 5: press  at the 'Enter full pathname of shell or RETURN 
> >>> for
> >>> /bin/sh:' prompt
> >>> Step 6: type "reboot"
> >>> Step 7: get a Panic: "Going nowhere without my init!"
> >> This means that init process (pid 1) exited for some reason. Show
> >> exact console log of the events.
> 
> I can also offer a (badly taken) photo of the console screen:
> 
> http://imgur.com/1xixODY
> 
> -andyf
> 
> ps. Thank you for taking an interest. I only really wanted to know
> if anyone else could reproduce the panic because it has happened
> on several of my (home network) boxes since 10.0

Apply the following patch.  I am interested if anything additional appear
on the console.  Screenshot is good enough.

diff --git a/sbin/init/init.c b/sbin/init/init.c
index bda86b5..1e88964 100644
--- a/sbin/init/init.c
+++ b/sbin/init/init.c
@@ -884,8 +884,13 @@ single_user(void)
if (Reboot) {
/* Instead of going single user, let's reboot the machine */
sync();
-   reboot(howto);
-   _exit(0);
+   if (reboot(howto) == -1) {
+   emergency("reboot(%#x) failed, %s", howto,
+   strerror(errno));
+   _exit(1); /* panic and reboot */
+   }
+   warning("reboot(%#x) returned", howto);
+   _exit(0); /* panic as well */
}
 
shell = get_shell();
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"