From nobody@FreeBSD.org Mon Jun 14 16:17:58 2010 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 395621065678 for ; Mon, 14 Jun 2010 16:17:58 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 1B63F8FC1F for ; Mon, 14 Jun 2010 16:17:58 +0000 (UTC) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id o5EGHv3W044399 for ; Mon, 14 Jun 2010 16:17:57 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id o5EGHvea044397; Mon, 14 Jun 2010 16:17:57 GMT (envelope-from nobody) Message-Id: <201006141617.o5EGHvea044397@www.freebsd.org> Date: Mon, 14 Jun 2010 16:17:57 GMT From: Ed Harbin To: freebsd-gnats-submit@FreeBSD.org Subject: kernel panic when IPMI enabled on some machines X-Send-Pr-Version: www-3.1 X-GNATS-Notify: >Number: 147855 >Category: kern >Synopsis: [ipmi] [patch] kernel panic when IPMI enabled on some machines >Confidential: no >Severity: serious >Priority: high >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Jun 14 16:20:00 UTC 2010 >Closed-Date: >Last-Modified: Tue Jun 15 01:25:12 UTC 2010 >Originator: Ed Harbin >Release: Production Release 8.0 >Organization: Netsocket >Environment: >Description: Panic on some machines (eg Compaq Presario SR5350F) when booting GENERIC + device ipmi device smbus. In sys/dev/ipmi/ipmi_smbios.c: smbios_run_table() is copying smbios entries into a fixed size automatic, table[20], which it overruns on some types of smbios, and hence corrupts the stack. The following patch just tests for the overrun and prevents the panic. However, this would truncate the table, and perhaps there is a way of learning the actual table size. Also, perhaps every entry in the smbios table is not a string. >How-To-Repeat: On every boot of problem machines. >Fix: Patch attached with submission follows: *** ipmi_smbios.c Mon Jun 14 15:43:01 2010 --- ipmi_smbios.c.new Mon Jun 14 15:43:52 2010 *************** *** 116,122 **** static struct mtx ipmi_info_mtx; MTX_SYSINIT(ipmi_info, &ipmi_info_mtx, "ipmi info", MTX_DEF); ! static char *get_strings(char *, char **); static void ipmi_smbios_probe(struct ipmi_get_info *); static int smbios_cksum (struct smbios_table_entry *); static void smbios_run_table(uint8_t *, int, dispatchproc_t *, --- 116,122 ---- static struct mtx ipmi_info_mtx; MTX_SYSINIT(ipmi_info, &ipmi_info_mtx, "ipmi info", MTX_DEF); ! static char *get_strings(char *, char **, int); static void ipmi_smbios_probe(struct ipmi_get_info *); static int smbios_cksum (struct smbios_table_entry *); static void smbios_run_table(uint8_t *, int, dispatchproc_t *, *************** *** 173,182 **** } static char * ! get_strings(char *p, char **table) { /* Scan for strings, stoping at a single null byte */ ! while (*p != 0) { *table++ = p; p += strlen(p) + 1; } --- 173,183 ---- } static char * ! get_strings(char *p, char **table, int tablesize) { + int i = 0; /* Scan for strings, stoping at a single null byte */ ! while (i++ < tablesize && *p != 0) { *table++ = p; p += strlen(p) + 1; } *************** *** 186,203 **** return (p + 1); } ! static void smbios_run_table(uint8_t *p, int entries, dispatchproc_t *dispatchstatus, struct ipmi_get_info *info) { struct structure_header *s; ! char *table[20]; uint8_t *nextp; while(entries--) { s = (struct structure_header *) p; ! nextp = get_strings(p + s->length, table); /* * No strings still has a double-null at the end, --- 187,204 ---- return (p + 1); } ! #define SMB_TSIZE 20 static void smbios_run_table(uint8_t *p, int entries, dispatchproc_t *dispatchstatus, struct ipmi_get_info *info) { struct structure_header *s; ! char *table[SMB_TSIZE]; uint8_t *nextp; while(entries--) { s = (struct structure_header *) p; ! nextp = get_strings(p + s->length, table, SMB_TSIZE); /* * No strings still has a double-null at the end, >Release-Note: >Audit-Trail: >Unformatted: