Serious compatibility breakage in -current.

Kostik Belousov kostikbel at gmail.com
Fri Nov 30 07:26:41 PST 2007


On Thu, Nov 29, 2007 at 10:35:52PM -0800, Carl Shapiro wrote:
> On Nov 29, 2007 3:28 AM, Kostik Belousov <kostikbel at gmail.com> wrote:
> > Please, try the patch below and report whether it is enough to fix cmucl
> > and any other regressions.
> 
> With this patch applied old and new binaries are delivered a SIGBUS
> instead of SIGSEGV and the si_code is always BUS_ADRERR.  This is
> halfway between the behavior old binaries expect and the new behavior.
>  When an access violation occurs, old binaries expect a SIGBUS and an
> si_code of BUS_PAGE_FAULT.  Presumably, we want new binaries to
> receive a SIGSEGV when an access violation occurs.  This patch does
> not consider whether we are running under an old or new binary.  Is
> that really okay?

The patch I posted was enough for CMUCL to survive the gc.

The introduced problem seems to be quite complex to solve. The worst issue
is that we are very late in the 7.0 release cycle. I discussed the situation
with the portmgr at . Proposition from Kris Kennaway and me below:
1. Repair the ELF OSABI note tag from lib/csu/common/csubrand.c.
   Currently, the tag is compiled, but optimized out.
2. For the i386 and amd64, create the tunable, machdep.prot_fault_translation,
   with the following behaviour:
	0 = autodetect the signal to be delivered on KERN_PROTECTION_FAILURE
	    from vm_fault based on the ELF OSABI note:
		no note or __FreeBSD_version < 700004 - SIGBUS/BUS_PAGE_FAULT
		note, and __FreeBSD_version >= 700004 - SIGSEGV/SEGV_ACCERR
	1 = always SIGBUS
	2 = always SIGSEGV

This would do mostly automatic correction of ABI breakage, with the exception
of the untaged binaries for 7-CURRENT/RELENG_7 before the note is fixed. For
them, sysctl would allow to run the binary with manual settings.
    
Patch below shall implement it, and be MFCed to RELENG_7. The change to
crtbrand.c shall be MFCed to RELENG_6 (and, possibly, RELENG_5). Patch
was lightly tested on i386, and compile tested on amd64.
Please, give it a run.

Could someone with RELENG_5 world check whether the OSABI tag is inserted into
the binaries (I doubt it). The easiest way is to call file on some binary.
Sample output on binary without tag:
/bin/ls: ELF 32-bit LSB executable, Intel 80386, version 1 (FreeBSD), dynamically linked (uses shared libs), stripped
With tag:
atrun: ELF 32-bit LSB executable, Intel 80386, version 1 (FreeBSD), for FreeBSD 8.0 (800004), dynamically linked (uses shared libs), FreeBSD-style, not stripped

diff --git a/lib/csu/common/crtbrand.c b/lib/csu/common/crtbrand.c
index 7e90144..e3309f8 100644
--- a/lib/csu/common/crtbrand.c
+++ b/lib/csu/common/crtbrand.c
@@ -43,7 +43,7 @@ static const struct {
     int32_t	type;
     char	name[sizeof ABI_VENDOR];
     int32_t	desc;
-} abitag __attribute__ ((section (ABI_SECTION), aligned(4))) __unused = {
+} abitag __attribute__ ((section (ABI_SECTION), aligned(4), used)) __unused = {
     sizeof ABI_VENDOR,
     sizeof(int32_t),
     ABI_NOTETYPE,
diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index 62d81f6..577851f 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -144,6 +144,9 @@ SYSCTL_INT(_machdep, OID_AUTO, kdb_on_nmi, CTLFLAG_RW,
 static int panic_on_nmi = 1;
 SYSCTL_INT(_machdep, OID_AUTO, panic_on_nmi, CTLFLAG_RW,
 	&panic_on_nmi, 0, "Panic on NMI");
+static int prot_fault_translation = 0;
+SYSCTL_INT(_machdep, OID_AUTO, prot_fault_translation, CTLFLAG_RW,
+	&prot_fault_translation, 0, "Select signal to deliver on protection fault");
 
 extern char *syscallnames[];
 
@@ -312,8 +315,32 @@ trap(struct trapframe *frame)
 			if (i == SIGSEGV)
 				ucode = SEGV_MAPERR;
 			else {
-				i = SIGSEGV; /* XXX hack */
-				ucode = SEGV_ACCERR;
+				if (prot_fault_translation == 0) {
+					/*
+					 * Autodetect.
+					 * This check also covers the images
+					 * without the ABI-tag ELF note.
+					 */
+					if (p->p_osrel >= 700004) {
+						i = SIGSEGV;
+						ucode = SEGV_ACCERR;
+					} else {
+						i = SIGBUS;
+						ucode = BUS_PAGE_FAULT;
+					}
+				} else if (prot_fault_translation == 1) {
+					/*
+					 * Always compat mode.
+					 */
+					i = SIGBUS;
+					ucode = BUS_PAGE_FAULT;
+				} else {
+					/*
+					 * Always SIGSEGV mode.
+					 */
+					i = SIGSEGV;
+					ucode = SEGV_ACCERR;
+				}
 			}
 			break;
 
diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
index e66fa1e..cddffc0 100644
--- a/sys/i386/i386/trap.c
+++ b/sys/i386/i386/trap.c
@@ -158,6 +158,9 @@ SYSCTL_INT(_machdep, OID_AUTO, kdb_on_nmi, CTLFLAG_RW,
 static int panic_on_nmi = 1;
 SYSCTL_INT(_machdep, OID_AUTO, panic_on_nmi, CTLFLAG_RW,
 	&panic_on_nmi, 0, "Panic on NMI");
+static int prot_fault_translation = 0;
+SYSCTL_INT(_machdep, OID_AUTO, prot_fault_translation, CTLFLAG_RW,
+	&prot_fault_translation, 0, "Select signal to deliver on protection fault");
 
 extern char *syscallnames[];
 
@@ -375,8 +378,32 @@ trap(struct trapframe *frame)
 			if (i == SIGSEGV)
 				ucode = SEGV_MAPERR;
 			else {
-				i = SIGSEGV; /* XXX hack */
-				ucode = SEGV_ACCERR;
+				if (prot_fault_translation == 0) {
+					/*
+					 * Autodetect.
+					 * This check also covers the images
+					 * without the ABI-tag ELF note.
+					 */
+					if (p->p_osrel >= 700004) {
+						i = SIGSEGV;
+						ucode = SEGV_ACCERR;
+					} else {
+						i = SIGBUS;
+						ucode = BUS_PAGE_FAULT;
+					}
+				} else if (prot_fault_translation == 1) {
+					/*
+					 * Always compat mode.
+					 */
+					i = SIGBUS;
+					ucode = BUS_PAGE_FAULT;
+				} else {
+					/*
+					 * Always SIGSEGV mode.
+					 */
+					i = SIGSEGV;
+					ucode = SEGV_ACCERR;
+				}
 			}
 			addr = eva;
 			break;
diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
index 6c77244..acbc9a0 100644
--- a/sys/kern/imgact_elf.c
+++ b/sys/kern/imgact_elf.c
@@ -592,11 +592,13 @@ fail:
 	return (error);
 }
 
+static const char FREEBSD_ABI_VENDOR[] = "FreeBSD";
+
 static int
 __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
 {
 	const Elf_Ehdr *hdr = (const Elf_Ehdr *)imgp->image_header;
-	const Elf_Phdr *phdr;
+	const Elf_Phdr *phdr, *pnote = NULL;
 	Elf_Auxargs *elf_auxargs;
 	struct vmspace *vmspace;
 	vm_prot_t prot;
@@ -607,7 +609,9 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
 	int error = 0, i;
 	const char *interp = NULL;
 	Elf_Brandinfo *brand_info;
+	const Elf_Note *note, *note_end;
 	char *path;
+	const char *note_name;
 	struct thread *td = curthread;
 	struct sysentvec *sv;
 
@@ -747,6 +751,9 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
 		case PT_PHDR: 	/* Program header table info */
 			proghdr = phdr[i].p_vaddr;
 			break;
+		case PT_NOTE:
+			pnote = &phdr[i];
+			break;
 		default:
 			break;
 		}
@@ -828,6 +835,36 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
 	imgp->auxargs = elf_auxargs;
 	imgp->interpreted = 0;
 
+	/*
+	 * Try to fetch the osreldate for FreeBSD binary from the ELF
+	 * OSABI-note. Only the first page of the image is searched,
+	 * the same as for headers.
+	 */
+#define	ALIGN4(x) (((u_long)(x) + 4) & ~4)
+	if (pnote != NULL && pnote->p_offset < PAGE_SIZE &&
+	    pnote->p_offset + pnote->p_filesz < PAGE_SIZE ) {
+		note = (const Elf_Note *)(imgp->image_header + pnote->p_offset);
+		note_end = (const Elf_Note *)(imgp->image_header + pnote->p_offset +
+		    pnote->p_filesz);
+		while (note < note_end) {
+			if (note->n_namesz == sizeof(FREEBSD_ABI_VENDOR) &&
+			    note->n_descsz == sizeof(int32_t) &&
+			    note->n_type == 1 /* ABI_NOTETYPE */) {
+				note_name = (const char *)(note + 1);
+				if (strncmp(FREEBSD_ABI_VENDOR, note_name,
+				    sizeof(FREEBSD_ABI_VENDOR)) == 0) {
+					imgp->proc->p_osrel = *(const int32_t *)
+					    (note_name +
+					     ALIGN4(sizeof(FREEBSD_ABI_VENDOR)));
+					break;
+				}
+			}
+			note = (const Elf_Note *)((const char *)(note + 1) +
+			    ALIGN4(note->n_namesz) + ALIGN4(note->n_descsz));
+		}
+	}
+#undef	ALIGN4
+
 	return (error);
 }
 
@@ -894,8 +931,6 @@ static void __elfN(puthdr)(struct thread *, void *, size_t *, int);
 static void __elfN(putnote)(void *, size_t *, const char *, int,
     const void *, size_t);
 
-extern int osreldate;
-
 int
 __elfN(coredump)(td, vp, limit)
 	struct thread *td;
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index d4c04e5..3a85150 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -373,9 +373,10 @@ proc0_init(void *dummy __unused)
 	td = &thread0;
 	
 	/*
-	 * Initialize magic number.
+	 * Initialize magic number and osrel.
 	 */
 	p->p_magic = P_MAGIC;
+	p->p_osrel = osreldate;
 
 	/*
 	 * Initialize thread and process structures.
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 2d32c34..6545614 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -391,6 +391,7 @@ interpret:
 	if (error)
 		goto exec_fail_dealloc;
 
+	imgp->proc->p_osrel = 0;
 	/*
 	 *	If the current process has a special image activator it
 	 *	wants to try first, call it.   For example, emulating shell
diff --git a/sys/kern/kern_mib.c b/sys/kern/kern_mib.c
index 7280c52..78a9445 100644
--- a/sys/kern/kern_mib.c
+++ b/sys/kern/kern_mib.c
@@ -104,7 +104,6 @@ SYSCTL_STRING(_kern, KERN_OSTYPE, ostype, CTLFLAG_RD,
  * NOTICE: The *userland* release date is available in
  * /usr/include/osreldate.h
  */
-extern int osreldate;
 SYSCTL_INT(_kern, KERN_OSRELDATE, osreldate, CTLFLAG_RD,
     &osreldate, 0, "Kernel release date");
 
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 54a8413..d75acb9 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -561,6 +561,8 @@ struct proc {
 /* The following fields are all copied upon creation in fork. */
 #define	p_startcopy	p_endzero
 	u_int		p_magic;	/* (b) Magic number. */
+	int		p_osrel;	/* (x) osreldate for the
+					       binary (from ELF note, if any) */
 	char		p_comm[MAXCOMLEN + 1];	/* (b) Process name. */
 	struct pgrp	*p_pgrp;	/* (c + e) Pointer to process group. */
 	struct sysentvec *p_sysent;	/* (b) Syscall dispatch info. */
diff --git a/sys/sys/systm.h b/sys/sys/systm.h
index 1a9d964..95bc05f 100644
--- a/sys/sys/systm.h
+++ b/sys/sys/systm.h
@@ -101,6 +101,7 @@ extern int maxusers;		/* system tune hint */
  * in two files.
  * XXX most of these variables should be const.
  */
+extern int osreldate;
 extern int envmode;
 extern int hintmode;		/* 0 = off. 1 = config, 2 = fallback */
 extern int dynamic_kenv;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20071130/b56ab8c0/attachment.pgp


More information about the freebsd-current mailing list