Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756192AbbDIVM0 (ORCPT ); Thu, 9 Apr 2015 17:12:26 -0400 Received: from mail-ie0-f178.google.com ([209.85.223.178]:35311 "EHLO mail-ie0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754056AbbDIVMX (ORCPT ); Thu, 9 Apr 2015 17:12:23 -0400 MIME-Version: 1.0 In-Reply-To: References:

Date: Thu, 9 Apr 2015 14:12:22 -0700 X-Google-Sender-Auth: 6i65r4mOawHJw8PDqKghLpR-CW4 Message-ID: Subject: =?UTF-8?Q?Re=3A_NULL_deref_around_blkmq_in_v4=2E0=2Drc1=E2=80=93rc7?= From: Linus Torvalds To: Jan Engelhardt Cc: "Rafael J. Wysocki" , Jens Axboe , Linux Kernel Mailing List Content-Type: multipart/mixed; boundary=001a1140bb968e2dbe0513511a79 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3610 Lines: 86 --001a1140bb968e2dbe0513511a79 Content-Type: text/plain; charset=UTF-8 On Thu, Apr 9, 2015 at 11:24 AM, Jan Engelhardt wrote: > > It's fairly consistent (reproducible?). Only 1 in 15 or so (have not kept track > really) attempts does it not die. > > With frame pointers: > [] scsi_queue_rq+0x2e8/0x3d2 > [] __blk_mq_run_hw_queue+0x19b/0x2a2 > [] ? blk_mq_merge_queue_io+0x75/0x147 > [] ? __xfs_get_blocks+0x2f9/0x2f9 [xfs] > [] blk_mq_run_hw_queue+0x4f/0x99 > [] blk_sq_make_request+0x163/0x170 Ok, good. So the cmd comes from struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req); which in turn is just return (void *) rq + sizeof(*rq); which in turn is written by some crazy monkey on crack. That's some shit code. Why the hell you'd write it that way, when the natural thing to do would be just return rq + 1; without the sizeof, and without the cast. The particular crazy monkey on crack is Jens Axboe, in commit 320ae51feed5c. Jens, really. This code is shit. That ->sense_buffer thing is supposed to be initialized by the blk_mq_ops.init_request() function, which is called - if it exists = when the array of requests ('->rqs[]') is initialized. And that code too looks like crap. It seems to be very clever, trying to allocaet big contiguous chunks of RAM for the requests, but then the initialization sequence is questionable as hell. It takes that nonzeroed allocation, and zeroes a few fields randomly. The rest will contain whatever garbage data they used to. Does this entirely untested patch make any difference? And Jens, this all really looks very fishy. When I look at these kinds of core functions, and find just *stupid* code like this, it makes me unhappy. Anyway, I assume that the actual "disk" in question is mpt fusion? Linus --001a1140bb968e2dbe0513511a79 Content-Type: text/plain; charset=US-ASCII; name="patch.diff" Content-Disposition: attachment; filename="patch.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i8ania4f0 IGJsb2NrL2Jsay1tcS5jIHwgNCArLS0tCiAxIGZpbGUgY2hhbmdlZCwgMSBpbnNlcnRpb24oKyks IDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvYmxvY2svYmxrLW1xLmMgYi9ibG9jay9ibGst bXEuYwppbmRleCBiN2I4OTMzZWMyNDEuLjMzYzQyODUzMDE5MyAxMDA2NDQKLS0tIGEvYmxvY2sv YmxrLW1xLmMKKysrIGIvYmxvY2svYmxrLW1xLmMKQEAgLTE0NTcsNyArMTQ1Nyw3IEBAIHN0YXRp YyBzdHJ1Y3QgYmxrX21xX3RhZ3MgKmJsa19tcV9pbml0X3JxX21hcChzdHJ1Y3QgYmxrX21xX3Rh Z19zZXQgKnNldCwKIAogCQlkbyB7CiAJCQlwYWdlID0gYWxsb2NfcGFnZXNfbm9kZShzZXQtPm51 bWFfbm9kZSwKLQkJCQlHRlBfS0VSTkVMIHwgX19HRlBfTk9XQVJOIHwgX19HRlBfTk9SRVRSWSwK KwkJCQlHRlBfS0VSTkVMIHwgX19HRlBfTk9XQVJOIHwgX19HRlBfTk9SRVRSWSB8IF9fR0ZQX1pF Uk8sCiAJCQkJdGhpc19vcmRlcik7CiAJCQlpZiAocGFnZSkKIAkJCQlicmVhazsKQEAgLTE0Nzks OCArMTQ3OSw2IEBAIHN0YXRpYyBzdHJ1Y3QgYmxrX21xX3RhZ3MgKmJsa19tcV9pbml0X3JxX21h cChzdHJ1Y3QgYmxrX21xX3RhZ19zZXQgKnNldCwKIAkJbGVmdCAtPSB0b19kbyAqIHJxX3NpemU7 CiAJCWZvciAoaiA9IDA7IGogPCB0b19kbzsgaisrKSB7CiAJCQl0YWdzLT5ycXNbaV0gPSBwOwot CQkJdGFncy0+cnFzW2ldLT5hdG9taWNfZmxhZ3MgPSAwOwotCQkJdGFncy0+cnFzW2ldLT5jbWRf ZmxhZ3MgPSAwOwogCQkJaWYgKHNldC0+b3BzLT5pbml0X3JlcXVlc3QpIHsKIAkJCQlpZiAoc2V0 LT5vcHMtPmluaXRfcmVxdWVzdChzZXQtPmRyaXZlcl9kYXRhLAogCQkJCQkJdGFncy0+cnFzW2ld LCBoY3R4X2lkeCwgaSwK --001a1140bb968e2dbe0513511a79-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/