Re: Slab corruption in 2.6.16-rc5-mm2

From: Linus Torvalds
Date: Mon Mar 06 2006 - 15:04:30 EST




On Mon, 6 Mar 2006, Linus Torvalds wrote:
>
> So it's either an aic7xxx bug, or it's generic SCSI.
>
> Considering that we've had other slab corruption issues (the reason I was
> looking closely at yours), generic SCSI isn't out of the question.
>
> If you were a git user, doing a bisection run would be useful since you
> seem to be able to recreate it at will. Oh, well. Testign that one patch
> would still help.

Hmm.. This appended patch may or may not help.

It overwrites the SCSI command "req" pointer when the request has been
done. The request cannot be used afterwards, so anybody accessing it would
be a bug. I think.

HOWEVER. I noticed something else strange. Your slab corruption report
says

Slab corruption: start=f72948a0, len=64
Redzone: 0x5a2cf071/0x5a2cf071.
Last user: [<c02934eb>](sr_do_ioctl+0x11b/0x270)
...

and the scary thing is that "len=64".

The thing is, SCSI uses "SCSI_SENSE_BUFFERSIZE" to determine the maximum
sense size to copy, and what do we have, if not

include/scsi/scsi_cmnd.h:#define SCSI_SENSE_BUFFERSIZE 96

ie a 64-byte buffer is simply TOO DAMN SMALL!

Now, the thing is, the 64 comes from "sizeof(struct request_sense)", which
is what "struct packet_command *" uses. We can change that sizeof() to
just use SCSI_SENSE_BUFFERSIZE, but that still makes me worry about
somebody else having allocated a "packed_command->sense" using just the
same 64-byte "struct request_sense".

Can a SCSI sense-buffer really be 96? If so, why is "struct request_sense"
the wrong size? This looks likea really nasty bug.

Linus

----
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 701a328..296ac8e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -788,6 +788,9 @@ static struct scsi_cmnd *scsi_end_reques
}
}

+ /* Poison the request pointer: it is done and no longer exists after this */
+ cmd->request = (void *) 0x5a5a5a5a;
+
add_disk_randomness(req->rq_disk);

spin_lock_irqsave(q->queue_lock, flags);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/