From root@pc1762.alcatel.com.au Sun Dec 13 15:03:09 1998 Received: from alcanet.com.au (border.alcanet.com.au [203.62.196.10]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id PAA20505 for ; Sun, 13 Dec 1998 15:03:06 -0800 (PST) (envelope-from root@pc1762.alcatel.com.au) Received: by border.alcanet.com.au id <40351>; Mon, 14 Dec 1998 10:02:14 +1100 Message-Id: <98Dec14.100214est.40351@border.alcanet.com.au> Date: Mon, 14 Dec 1998 10:02:49 +1100 From: Charlie Root Reply-To: peter.jeremy@alcatel.com.au To: FreeBSD-gnats-submit@FreeBSD.ORG Cc: peter.jeremy@auss2.alcatel.com.au Subject: Changes to SysV Semaphore defaults not handled in sysv_sem.c X-Send-Pr-Version: 3.2 >Number: 9068 >Category: kern >Synopsis: Changing SysV semaphore defaults not correctly handled >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sun Dec 13 15:10:00 PST 1998 >Closed-Date: Mon Dec 14 00:35:10 PST 1998 >Last-Modified: Mon Dec 14 00:36:20 PST 1998 >Originator: Peter Jeremy >Release: FreeBSD 3.0-RELEASE i386 >Organization: Alcatel Australia Limited >Environment: Dell OptiPlex GXi P5-133 with 96MB ram and the following config file: machine "i386" cpu "I586_CPU" ident "pc1762" maxusers 40 options "MAXDSIZ=(256*1024*1024)" options "DFLDSIZ=(256*1024*1024)" options "MAXMEM=(96*1024)" options FAILSAFE #Be conservative config kernel root on wd0 options "COMPAT_43" #Compatible with BSD 4.3 [KEEP THIS!] options SYSVSHM options SHMMAX="(16*1024*1024)" options SYSVSEM options SEMUME=1 options SEMMNU=1000 options SYSVMSG options "MD5" options KTRACE #kernel tracing options PERFMON options USERCONFIG #boot -c editor options VISUAL_USERCONFIG #visual boot -c editor options INET #InterNETworking pseudo-device ether pseudo-device loop options FFS #Berkeley Fast Filesystem options FFS_ROOT # Allow FFS root options SOFTUPDATES options NFS #Network Filesystem options FDESC #File descriptor filesystem options MFS #Memory filesystem options PROCFS #Process filesystem pseudo-device pty 256 pseudo-device speaker pseudo-device gzip # Exec gzipped a.out's pseudo-device vn pseudo-device ccd 4 pseudo-device bpfilter 4 controller scbus0 device da0 # SCSI disks device st0 # SCSI tapes device pass0 # SCSI passthru for CAM controller isa0 options "AUTO_EOI_1" device ed0 at isa? port 0x300 net irq 10 iomem 0xcc000 vector edintr controller pci0 controller pnp0 device sc0 at isa? port "IO_KBD" tty irq 1 vector scintr options SC_DISABLE_REBOOT device npx0 at isa? port "IO_NPX" flags 0x0 irq 13 vector npxintr controller wdc0 at isa? port "IO_WD1" flags 0xb0ffb0ff bio irq 14 vector wdintr disk wd0 at wdc0 drive 0 disk wd1 at wdc0 disable drive 1 controller wdc1 at isa? disable port "IO_WD2" flags 0xb0ffb0ff bio irq 15 vector wdintr disk wd2 at wdc1 drive 0 disk wd3 at wdc1 disable drive 1 options "CMD640" controller fdc0 at isa? port "IO_FD1" bio irq 6 drq 2 vector fdintr disk fd0 at fdc0 drive 0 disk fd1 at fdc0 disable drive 1 device lpt0 at isa? port? tty irq 7 vector lptintr device psm0 at isa? port "IO_KBD" conflicts tty irq 12 vector psmintr device apm0 at isa? # Advanced Power Management device sio0 at isa? port "IO_COM1" tty irq 4 vector siointr device sio1 at isa? disable port "IO_COM2" tty irq 3 vector siointr controller ahc0 options AHC_ALLOW_MEMIO # Memory-mapped I/O device xl0 options NO_LKM options "CLK_USE_TSC_CALIBRATION" options PPS_SYNC >Description: When running a kernel using the above config file, the system panic's `getnewbuf: inconsistent EMPTY queue, qindex=0' immediately after the message `changing root device to wd0s1a'. Tracing system initialisation showed that buf[0].b_qindex was being changed from 5 (QUEUE_EMPTY) to 0 inside seminit(). Studying the code, the default semaphore parameters are defined in sys/sem.h. These parameters can be over-ridden via config file options which are stored in opt_param.h. The parameters are mainly used to initialise the seminfo structure which is used by most of the rest of the code to define the actual number of semaphores present. The `mainly' and `most of' imply exceptions, and these are the problem: the macros SEMUME and SEMUSZ are referenced from sysv_sem.c (directly and via SEMU()). However opt_param.h is not included in sysv_sem.c so it references the default, rather than actual parameter values. The net effect is that (for the configuration above), seminit() believes there are seminfo.semmnu (1000) sem_undo structures, each of which is SEMUSZ (100), rather than seminfo.semusz (28) bytes long - incorrectly initialising assorted innocent kernel memory. There is also a problem in the SEMUSZ macro. This macro over-estimates the size of a sem_undo structure by sizeof(struct undo) [8] bytes because the first undo is counted in both the basic struct sem_undo and explicitly allocated as a struct undo. This just wastes kernel memory (rather than directly causing a problem) because this macro is always used to calculate the structure size. SysV shm is not affected (the defaults are all calculated in param.c) and SysV msg does not appear to be affected - although the defaults are calculated in sys/msg.h, they do not appear to be referenced except in param.c. >How-To-Repeat: Build 3.0-RELEASE kernel using above config file and boot it. >Fix: A number of options are possible and the choice between them is primarily stylistic and speed-driven. Note that the patches in the following have been cut-and-pasted, hence the whitespace will be wrong. There may also be discrepancies in the line numbers due to debugging code not relevant to these patches. 1) Include opt_param.h in sys/sem.h (protected by #ifdef KERNEL): --- sys/sem.h.orig Wed Jul 15 12:32:32 1998 +++ sys/sem.h Mon Dec 14 09:43:52 1998 @@ -116,6 +116,7 @@ /* * Configuration parameters */ +#include "opt_param.h" /* possibly override following macros */ #ifndef SEMMNI #define SEMMNI 10 /* # of semaphore identifiers */ #endif [It is safe to include opt_param.h multiple times because it is re-defining macros to be exactly the same expansion - which is explicitly allowed by ANSI C]. This is the simplest change and retains the efficiency of having macro references which turn into constant expressions (allowing the compiler to optimize them). 2) Include opt_param.h in kern/sysv_sem.h: --- kern/sysv_sem.c.orig Mon Dec 14 07:23:02 1998 +++ kern/sysv_sem.c Mon Dec 14 09:49:01 1998 @@ -8,6 +8,8 @@ * This software is provided ``AS IS'' without any warranties of any kind. */ +#include "opt_param.h" + #include #include #include This is basically equivalent to the above change but only includes opt_param.h into the code which (may) need it. 3) Change sys/sem.h and kern/sysv_sem.c to refer to values via seminfo rather than macros: --- sys/sem.h.orig Wed Jul 15 12:32:32 1998 +++ sys/sem.h Mon Dec 14 10:00:54 1998 @@ -150,7 +151,7 @@ /* * Macro to find a particular sem_undo vector */ -#define SEMU(ix) ((struct sem_undo *)(((intptr_t)semu)+ix * SEMUSZ)) +#define SEMU(ix) ((struct sem_undo *)(((intptr_t)semu)+ix * seminfo.semusz)) /* * Process sem_undo vectors at proc exit. --- kern/sysv_sem.c.orig Mon Dec 14 07:23:02 1998 +++ kern/sysv_sem.c Mon Dec 14 09:59:19 1998 @@ -283,7 +285,7 @@ /* Didn't find the right entry - create it */ if (adjval == 0) return(0); - if (suptr->un_cnt != SEMUME) { + if (suptr->un_cnt != seminfo.semume) { sunptr = &suptr->un_ent[suptr->un_cnt]; suptr->un_cnt++; sunptr->un_adjval = adjval; This is probably the `cleaner' patch, but may slow down semaphore undo related operations due to additional memory references and less optimisable code. In addition to the above, the definition of SEMUSZ should be changed to: --- sys/sem.h.orig Wed Jul 15 12:32:32 1998 +++ sys/sem.h Mon Dec 14 09:43:52 1998 @@ -141,7 +142,7 @@ #endif /* actual size of an undo structure */ -#define SEMUSZ (sizeof(struct sem_undo)+sizeof(struct undo)*SEMUME) +#define SEMUSZ (sizeof(struct sem_undo)+sizeof(struct undo)*(SEMUME - 1)) extern struct semid_ds *sema; /* semaphore id pool */ extern struct sem *sem; /* semaphore pool */ -- Peter Jeremy (VK2PJ) peter.jeremy@alcatel.com.au Alcatel Australia Limited 41 Mandible St Phone: +61 2 9690 5019 ALEXANDRIA NSW 2015 Fax: +61 2 9690 5982 >Release-Note: >Audit-Trail: State-Changed-From-To: open->closed State-Changed-By: dillon State-Changed-When: Mon Dec 14 00:35:10 PST 1998 State-Changed-Why: committed fix to -current. Basically option 3 was used except defined SEMUSZ using offsetof(). >Unformatted: