kern/85365: [ PATCH ] geom_label can cause PANIC during load

[Dan Lukes]

>Number:         85365
>Category:       kern
>Synopsis:       [ PATCH ] geom_label can cause PANIC during load
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Aug 27 23:50:19 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dan Lukes
>Release:        FreeBSD 6.0-BETA3 i386
>Organization:
Obludarium
>Environment:
System: FreeBSD 6.0-BETA3 Sat Aug 27 17:43:26 CEST 2005 i386
sys/geom/label/g_label_ext2fs.c,v 1.1.2.1 2005/08/16 08:20:47

>Description:
	geom_load can cause kernel panic:
panic: wrong offset 1024 for sectorsize 2048


>How-To-Repeat:
	load geom_label on computer with empty CD ROM device
>Fix:

	The geom_label taste all devices on loading. When tasting a CD ROM
with no media, the tasting failed for ISO9660. The code then taste for
EXT2FS. It's code issue invalid g_read_data() request catched by KASSERT
within g_io_request()

	The taste routines for other FS may issue similar invalid IO request
on consumers with 'bad' sector size also.

	BTW, the g_label_*fs.c should be rewritten to use apropriate include
files (isofs/cd9660/iso.h or gnu/fs/ext2fs/fs.h for example) whenever possible. 
It's not covered by patches supplied bellow.

--- patch begins here ---
--- sys/geom/label/g_label_ext2fs.c.ORIG	Tue Aug 16 10:20:47 2005
+++ sys/geom/label/g_label_ext2fs.c	Sun Aug 28 00:00:24 2005
@@ -58,6 +58,14 @@
 	pp = cp->provider;
 	label[0] = '\0';
 
+	/*
+	 * Take care not to issue an invalid I/O request.  The
+	 * offset of the superblock candidate must be
+	 * multiples of the provider's sector size, otherwise an
+	 * EXT2FS can't exist on the provider anyway.
+	*/
+	if (EXT2FS_SB_OFFSET % pp->sectorsize != 0)
+		return;
 	fs = (e2sb_t *)g_read_data(cp, EXT2FS_SB_OFFSET, pp->sectorsize, NULL);
 	if (fs == NULL)
 		return;
--- sys/geom/label/g_label_iso9660.c.ORIG	Fri Jul  2 21:40:34 2004
+++ sys/geom/label/g_label_iso9660.c	Sun Aug 28 01:12:34 2005
@@ -52,7 +52,20 @@
 	pp = cp->provider;
 	label[0] = '\0';
 
-	sector = (char *)g_read_data(cp, 0x8000, pp->sectorsize, &error);
+	/*
+	 * Take care not to issue an invalid I/O request.  The
+	 * provider's sector size must be 2048, otherwise an
+	 * CD9660FS can't exist on the provider anyway.
+	 * Well, the standard says this should be 2048 or the 
+	 * physical sector size on the device, whichever is greater.
+	 * For now, we'll just use a constant. It follow the logic used 
+	 * within cd9660_vfsops.c
+	 * BTW, are we sure we want the label from first found descriptor 
+	 * despite of it's type ? And how about SIERRA CD format ?
+	*/
+	if (pp->sectorsize != 2048)
+		return;
+	sector = (char *)g_read_data(cp, 16*2048, pp->sectorsize, &error);
 	if (sector == NULL || error != 0)
 		return;
 	if (bcmp(sector, ISO9660_MAGIC, sizeof(ISO9660_MAGIC) - 1) != 0) {
--- sys/geom/label/g_label_reiserfs.c.ORIG	Tue Aug 16 10:20:47 2005
+++ sys/geom/label/g_label_reiserfs.c	Sun Aug 28 01:19:05 2005
@@ -53,6 +53,14 @@
 {
 	reiserfs_sb_t *fs;
 
+	/*
+	 * Take care not to issue an invalid I/O request.  The
+	 * offset and len of the superblock candidate must be
+	 * multiples of the provider's sector size, otherwise an
+	 * REISERFS can't exist on the provider anyway.
+	*/
+	if (offset % pp->sectorsize != 0 || len % pp->sectorsize != 0)
+		return (NULL);
 	fs = (reiserfs_sb_t *)g_read_data(cp, offset, len, NULL);
 	if (fs == NULL)
 		return (NULL);
--- patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted: