All of lore.kernel.org
 help / color / mirror / Atom feed
* Early-boot kernel panics from udev-165/extras/ata_id/ata_id.c
@ 2011-01-06 22:02 John Stanley
  2011-01-06 22:29 ` Greg KH
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: John Stanley @ 2011-01-06 22:02 UTC (permalink / raw)
  To: linux-hotplug

[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]

Hello,
There is a problem in udev-165/extras/ata_id/ata_id.c resulting in 
random early boot kernel panics.  As it stands, udev-165 is not usable 
because the boot panics occur to frequently.  The systems are GNU/Linux 
i686 with linux-2.6.36.2 and linux-2.6.37, gcc-4.5.1, and glibc-2.12.1.

By "random," I mean that the panics don't occur on every boot, or on 
every pc I've tried. The panics do not occur for udev-164.  The problem 
code in udev-165 is in the "disk_identify_packet_device_command" 
function (see line 270) where an sg3 'ATA Pass-Through' command is 
issued to a cd/dvd device:

253         ret = ioctl(fd, SG_IO, &io_v4);
254         if (ret != 0) {
255                 /* could be that the driver doesn't do version 4, 
try version 3 */
256                 if (errno == EINVAL) {
257                         struct sg_io_hdr io_hdr;
258
259                         memset(&io_hdr, 0, sizeof(struct sg_io_hdr));
260                         io_hdr.interface_id = 'S';
261                         io_hdr.cmdp = (unsigned char*) cdb;
262                         io_hdr.cmd_len = sizeof (cdb);
263                         io_hdr.dxferp = buf;
264                         io_hdr.dxfer_len = buf_len;
265                         io_hdr.sbp = sense;
266                         io_hdr.mx_sb_len = sizeof (sense);
267                         io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
268                         io_hdr.timeout = COMMAND_TIMEOUT_MSEC;
269
270                         ret = ioctl(fd, SG_IO, &io_hdr); <-- random 
kernel panics result from this call ***
271                         if (ret != 0)
272                                 goto out;
273                 } else {
274                         goto out;
275                 }
276         }
277
278         if (!(sense[0] == 0x72 && desc[0] == 0x9 && desc[1] == 0x0c)) {
279                 errno = EIO;
280                 ret = -1;
281                 goto out;
282         }
283
284  out:
285         return ret;
286 }

While by no means a fix of course, simply commenting out line 270 above 
appears to side-step the issue. Also, suspecting a buffer alignment 
issue, I built udev-165 with the attached patch, and thus far, no kernel 
panics have occurred.

I realize that allocating the sense and response buffers in this manner 
shouldn't be necessary or even a good idea; my hope is that it might 
shed some light on the cause of the panics. I suspect that this issue is 
likely a kernel problem (not a udev problem),  but do you have any 
thoughts/ideas on how to track down the problem?

John

[-- Attachment #2: 02-udev-165-ata-pass-through-memalign.patch --]
[-- Type: text/plain, Size: 1378 bytes --]

--- udev-165.old/extras/ata_id/ata_id.c	2010-11-09 19:30:53.000000000 -0500
+++ udev-165.new/extras/ata_id/ata_id.c	2011-01-05 03:11:06.629999372 -0500
@@ -208,7 +208,9 @@
 {
 	struct sg_io_v4 io_v4;
 	uint8_t cdb[16];
-	uint8_t sense[32];
+	// Allocate page-aligned memory, rather than using an array (uint8_t sense[32]); jps
+	uint8_t* sense;
+	posix_memalign((void**)&sense, sysconf(_SC_PAGESIZE), 32);
 	uint8_t *desc = sense+8;
 	int ret;
 
@@ -251,7 +253,7 @@
 	io_v4.timeout = COMMAND_TIMEOUT_MSEC;
 
 	ret = ioctl(fd, SG_IO, &io_v4);
-	if (ret != 0) {
+	if (ret < 0) {
 		/* could be that the driver doesn't do version 4, try version 3 */
 		if (errno == EINVAL) {
 			struct sg_io_hdr io_hdr;
@@ -268,7 +270,7 @@
 			io_hdr.timeout = COMMAND_TIMEOUT_MSEC;
 
 			ret = ioctl(fd, SG_IO, &io_hdr);
-			if (ret != 0)
+			if (ret < 0)
 				goto out;
 		} else {
 			goto out;
@@ -282,6 +284,7 @@
 	}
 
  out:
+	free(sense);
 	return ret;
 }
 
@@ -447,7 +450,9 @@
 {
 	struct udev *udev;
 	struct hd_driveid id;
-	uint8_t identify[512];
+	// Allocate page-aligned memory, rather than using an array (uint8_t identify[512]); jps
+	uint8_t* identify;
+	posix_memalign((void**)&identify, sysconf(_SC_PAGESIZE), 512);
 	char model[41];
 	char model_enc[256];
 	char serial[21];
@@ -709,5 +714,6 @@
 exit:
 	udev_unref(udev);
 	udev_log_close();
+	free(identify);
 	return rc;
 }

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-01-20 12:59 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-06 22:02 Early-boot kernel panics from udev-165/extras/ata_id/ata_id.c John Stanley
2011-01-06 22:29 ` Greg KH
2011-01-07  2:13 ` Robby Workman
2011-01-07  8:06 ` Ozan Çağlayan
2011-01-10  8:10 ` Hannes Reinecke
2011-01-10 11:35 ` Ozan Çağlayan
2011-01-11 13:25 ` Tejun Heo
2011-01-17  3:53 ` John Stanley
2011-01-17  4:03 ` John Stanley
2011-01-17  5:07 ` John Stanley
2011-01-17 15:27 ` Tejun Heo
2011-01-17 15:28 ` Tejun Heo
2011-01-18  3:38 ` John Stanley
2011-01-18 15:09 ` Tejun Heo
2011-01-18 21:48 ` John Stanley
2011-01-19  2:07 ` Brad Price
2011-01-19 20:20 ` John Stanley
2011-01-20 12:59   ` [PATCH #upstream-fixes] libata: set queue DMA alignment to sector Tejun Heo
2011-01-20 12:59     ` [PATCH #upstream-fixes] libata: set queue DMA alignment to sector size for ATAPI too Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.