From owner-svn-src-head@freebsd.org Fri Jan 5 07:09:41 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8D286EB86A8; Fri, 5 Jan 2018 07:09:41 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 5DC657A11E; Fri, 5 Jan 2018 07:09:41 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w0579e4h031324; Fri, 5 Jan 2018 07:09:40 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w0579eGa031321; Fri, 5 Jan 2018 07:09:40 GMT (envelope-from imp@FreeBSD.org) Message-Id: <201801050709.w0579eGa031321@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Fri, 5 Jan 2018 07:09:40 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r327576 - head/usr.sbin/dumpcis X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/usr.sbin/dumpcis X-SVN-Commit-Revision: 327576 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Jan 2018 07:09:41 -0000 Author: imp Date: Fri Jan 5 07:09:40 2018 New Revision: 327576 URL: https://svnweb.freebsd.org/changeset/base/327576 Log: Add a number of sanity checks to the data that we're handling from the CIS. Coverity has tagged it as tainted. While this data is more trusted than your average data, we still need to do some basic validation on it. Check ioctl return value to ensure we switch memory targets between common and attribute as well as the lseek. CID: 1210464, 1006640, 1006868, 1007292, 1009091, 1009822, 1009824 Modified: head/usr.sbin/dumpcis/printcis.c head/usr.sbin/dumpcis/readcis.c head/usr.sbin/dumpcis/readcis.h Modified: head/usr.sbin/dumpcis/printcis.c ============================================================================== --- head/usr.sbin/dumpcis/printcis.c Fri Jan 5 07:09:29 2018 (r327575) +++ head/usr.sbin/dumpcis/printcis.c Fri Jan 5 07:09:40 2018 (r327576) @@ -26,10 +26,8 @@ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#ifndef lint -static const char rcsid[] = - "$FreeBSD$"; -#endif /* not lint */ +#include +__FBSDID("$FreeBSD$"); /* * Code cleanup, bug-fix and extension @@ -183,8 +181,7 @@ static void dump_config_map(struct tuple *tp) { u_char *p = tp->data, x; - int rlen, mlen = 0; - int i; + unsigned int rlen, mlen = 0, i; rlen = (p[0] & 3) + 1; if (tp->code == CIS_CONF_MAP) Modified: head/usr.sbin/dumpcis/readcis.c ============================================================================== --- head/usr.sbin/dumpcis/readcis.c Fri Jan 5 07:09:29 2018 (r327575) +++ head/usr.sbin/dumpcis/readcis.c Fri Jan 5 07:09:40 2018 (r327576) @@ -26,16 +26,15 @@ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#ifndef lint -static const char rcsid[] = - "$FreeBSD$"; -#endif /* not lint */ +#include +__FBSDID("$FreeBSD$"); /* * Code cleanup, bug-fix and extension * by Tatsumi Hosokawa */ +#include #include #include #include @@ -52,42 +51,44 @@ static struct tuple_list *read_tuples(int); static struct tuple *find_tuple_in_list(struct tuple_list *, unsigned char); static struct tuple_info *get_tuple_info(unsigned char); +#define LENGTH_ANY 255 + static struct tuple_info tuple_info[] = { {"Null tuple", CIS_NULL, 0}, - {"Common memory descriptor", CIS_MEM_COMMON, 255}, - {"Long link to next chain for CardBus", CIS_LONGLINK_CB, 255}, - {"Indirect access", CIS_INDIRECT, 255}, - {"Configuration map for CardBus", CIS_CONF_MAP_CB, 255}, - {"Configuration entry for CardBus", CIS_CONFIG_CB, 255}, - {"Long link to next chain for MFC", CIS_LONGLINK_MFC, 255}, + {"Common memory descriptor", CIS_MEM_COMMON, LENGTH_ANY}, + {"Long link to next chain for CardBus", CIS_LONGLINK_CB, LENGTH_ANY}, + {"Indirect access", CIS_INDIRECT, LENGTH_ANY}, + {"Configuration map for CardBus", CIS_CONF_MAP_CB, LENGTH_ANY}, + {"Configuration entry for CardBus", CIS_CONFIG_CB, LENGTH_ANY}, + {"Long link to next chain for MFC", CIS_LONGLINK_MFC, LENGTH_ANY}, {"Base address register for CardBus", CIS_BAR, 6}, {"Checksum", CIS_CHECKSUM, 5}, {"Long link to attribute memory", CIS_LONGLINK_A, 4}, {"Long link to common memory", CIS_LONGLINK_C, 4}, {"Link target", CIS_LINKTARGET, 3}, {"No link", CIS_NOLINK, 0}, - {"Version 1 info", CIS_INFO_V1, 255}, - {"Alternate language string", CIS_ALTSTR, 255}, - {"Attribute memory descriptor", CIS_MEM_ATTR, 255}, - {"JEDEC descr for common memory", CIS_JEDEC_C, 255}, - {"JEDEC descr for attribute memory", CIS_JEDEC_A, 255}, - {"Configuration map", CIS_CONF_MAP, 255}, - {"Configuration entry", CIS_CONFIG, 255}, - {"Other conditions for common memory", CIS_DEVICE_OC, 255}, - {"Other conditions for attribute memory", CIS_DEVICE_OA, 255}, - {"Geometry info for common memory", CIS_DEVICEGEO, 255}, - {"Geometry info for attribute memory", CIS_DEVICEGEO_A, 255}, + {"Version 1 info", CIS_INFO_V1, LENGTH_ANY}, + {"Alternate language string", CIS_ALTSTR, LENGTH_ANY}, + {"Attribute memory descriptor", CIS_MEM_ATTR, LENGTH_ANY}, + {"JEDEC descr for common memory", CIS_JEDEC_C, LENGTH_ANY}, + {"JEDEC descr for attribute memory", CIS_JEDEC_A, LENGTH_ANY}, + {"Configuration map", CIS_CONF_MAP, LENGTH_ANY}, + {"Configuration entry", CIS_CONFIG, LENGTH_ANY}, + {"Other conditions for common memory", CIS_DEVICE_OC, LENGTH_ANY}, + {"Other conditions for attribute memory", CIS_DEVICE_OA, LENGTH_ANY}, + {"Geometry info for common memory", CIS_DEVICEGEO, LENGTH_ANY}, + {"Geometry info for attribute memory", CIS_DEVICEGEO_A, LENGTH_ANY}, {"Manufacturer ID", CIS_MANUF_ID, 4}, {"Functional ID", CIS_FUNC_ID, 2}, - {"Functional EXT", CIS_FUNC_EXT, 255}, + {"Functional EXT", CIS_FUNC_EXT, LENGTH_ANY}, {"Software interleave", CIS_SW_INTERLV, 2}, - {"Version 2 Info", CIS_VERS_2, 255}, - {"Data format", CIS_FORMAT, 255}, + {"Version 2 Info", CIS_VERS_2, LENGTH_ANY}, + {"Data format", CIS_FORMAT, LENGTH_ANY}, {"Geometry", CIS_GEOMETRY, 4}, {"Byte order", CIS_BYTEORDER, 2}, {"Card init date", CIS_DATE, 4}, {"Battery replacement", CIS_BATTERY, 4}, - {"Organization", CIS_ORG, 255}, + {"Organization", CIS_ORG, LENGTH_ANY}, {"Terminator", CIS_END, 0}, {0, 0, 0} }; @@ -99,10 +100,9 @@ xmalloc(int sz) sz = (sz + 7) & ~7; p = malloc(sz); - if (p) - bzero(p, sz); - else + if (p == NULL) errx(1, "malloc"); + bzero(p, sz); return (p); } @@ -205,39 +205,40 @@ read_tuples(int fd) do { flag = MDF_ATTR; tp = find_tuple_in_list(last_tl, CIS_LONGLINK_A); - if (tp == 0) { + if (tp == NULL) { flag = 0; tp = find_tuple_in_list(last_tl, CIS_LONGLINK_C); } - if (tp && tp->length == 4) { - offs = tpl32(tp->data); + + if (tp == NULL || tp->length != 4) + break; + + offs = (uint32_t)tpl32(tp->data); #ifdef DEBUG - printf("Checking long link at %zd (%s memory)\n", - offs, flag ? "Attribute" : "Common"); + printf("Checking long link at %zd (%s memory)\n", + offs, flag ? "Attribute" : "Common"); #endif - /* If a link was found, read the tuple list from it. */ - if (ck_linktarget(fd, offs, flag)) { - tl = read_one_tuplelist(fd, flag, offs); - last_tl->next = tl; - last_tl = tl; - } - } else - tl = 0; + /* + * If a link was found, it looks sane read the tuple list from it. + */ + if (offs > 0 && offs < 32 * 1024 && ck_linktarget(fd, offs, flag)) { + tl = read_one_tuplelist(fd, flag, offs); + last_tl->next = tl; + last_tl = tl; + } } while (tl); /* - * If the primary list had no NOLINK tuple, and no LINKTARGET, - * then try to read a tuple list at common memory (offset 0). + * If the primary list had no NOLINK tuple, and no LINKTARGET, then try + * to read a tuple list at common memory (offset 0). */ if (find_tuple_in_list(tlist, CIS_NOLINK) == 0 && find_tuple_in_list(tlist, CIS_LINKTARGET) == 0 && ck_linktarget(fd, (off_t) 0, 0)) { - offs = 0; #ifdef DEBUG - printf("Reading long link at %zd (%s memory)\n", - offs, flag ? "Attribute" : "Common"); + printf("Reading long link at 0 (Common memory)\n"); #endif - tlist->next = read_one_tuplelist(fd, 0, offs); + tlist->next = read_one_tuplelist(fd, 0, 0); } return (tlist); } @@ -261,13 +262,15 @@ read_one_tuplelist(int fd, int flags, off_t offs) tl = xmalloc(sizeof(*tl)); tl->offs = offs; tl->flags = flags & MDF_ATTR; - ioctl(fd, PIOCRWFLAG, &flags); - lseek(fd, offs, SEEK_SET); + if (ioctl(fd, PIOCRWFLAG, &flags) < 0) + err(1, "Setting flag to rad %s memory failed", + flags ? "attribute" : "common"); + if (lseek(fd, offs, SEEK_SET) < 0) + err(1, "Unable to seek to memory offset %ju", + (uintmax_t)offs); do { - if (read(fd, &code, 1) != 1) { - warn("CIS code read"); - break; - } + if (read(fd, &code, 1) != 1) + errx(1, "CIS code read"); total++; if (code == CIS_NULL) continue; @@ -276,42 +279,50 @@ read_one_tuplelist(int fd, int flags, off_t offs) if (code == CIS_END) length = 0; else { - if (read(fd, &length, 1) != 1) { - warn("CIS len read"); - break; - } + if (read(fd, &length, 1) != 1) + errx(1, "CIS len read"); total++; } - tp->length = length; #ifdef DEBUG printf("Tuple code = 0x%x, len = %d\n", code, length); #endif + + /* + * A length of 255 is invalid, all others are valid. Treat a + * length of 255 as the end of the list. Some cards don't have a + * CIS_END at the end. These work on other systems because the + * end of the CIS eventually sees an area that's not decoded and + * read back as 0xff. + */ if (length == 0xFF) { - length = tp->length = 0; + length = 0; code = CIS_END; } - if (length != 0) { - total += length; - tp->data = xmalloc(length); - if (read(fd, tp->data, length) != length) { - warn("CIS read"); - break; - } - } + assert(length < 0xff); /* * Check the tuple, and ignore it if it isn't in the table * or the length is illegal. */ tinfo = get_tuple_info(code); - if (tinfo != NULL && (tinfo->length != 255 && tinfo->length > length)) { + if (tinfo == NULL || (tinfo->length != LENGTH_ANY && tinfo->length > length)) { printf("code %s (%d) ignored\n", tuple_name(code), code); - tp->code = CIS_NULL; + continue; } - if (tl->tuples == NULL) - tl->tuples = tp; - else + tp->length = length; + if (length != 0) { + total += length; + tp->data = xmalloc(length); + if (read(fd, tp->data, length) != length) + errx(1, "Can't read CIS data"); + } + + if (last_tp != NULL) last_tp->next = tp; + if (tl->tuples == NULL) { + tl->tuples = tp; + tp->next = NULL; + } last_tp = tp; } while (code != CIS_END && total < 1024); return (tl); @@ -325,8 +336,12 @@ ck_linktarget(int fd, off_t offs, int flag) { char blk[5]; - ioctl(fd, PIOCRWFLAG, &flag); - lseek(fd, offs, SEEK_SET); + if (ioctl(fd, PIOCRWFLAG, &flags) < 0) + err(1, "Setting flag to rad %s memory failed", + flags ? "attribute" : "common"); + if (lseek(fd, offs, SEEK_SET) < 0) + err(1, "Unable to seek to memory offset %ju", + (uintmax_t)offs); if (read(fd, blk, 5) != 5) return (0); if (blk[0] == CIS_LINKTARGET && Modified: head/usr.sbin/dumpcis/readcis.h ============================================================================== --- head/usr.sbin/dumpcis/readcis.h Fri Jan 5 07:09:29 2018 (r327575) +++ head/usr.sbin/dumpcis/readcis.h Fri Jan 5 07:09:40 2018 (r327576) @@ -31,7 +31,7 @@ struct tuple { struct tuple *next; unsigned char code; - int length; + unsigned char length; unsigned char *data; };