From patchwork Tue Aug 18 14:54:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bugzilla-daemon@bugzilla.kernel.org X-Patchwork-Id: 7031741 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 60D5EC05AC for ; Tue, 18 Aug 2015 14:54:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 546282054C for ; Tue, 18 Aug 2015 14:54:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5CFB320595 for ; Tue, 18 Aug 2015 14:54:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752144AbbHROyd (ORCPT ); Tue, 18 Aug 2015 10:54:33 -0400 Received: from mail.kernel.org ([198.145.29.136]:34632 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750893AbbHROyd convert rfc822-to-8bit (ORCPT ); Tue, 18 Aug 2015 10:54:33 -0400 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CE0692054C for ; Tue, 18 Aug 2015 14:54:31 +0000 (UTC) Received: from bugzilla1.web.kernel.org (bugzilla1.web.kernel.org [172.20.200.51]) by mail.kernel.org (Postfix) with ESMTP id A2CF1205DF for ; Tue, 18 Aug 2015 14:54:28 +0000 (UTC) Received: by bugzilla1.web.kernel.org (Postfix, from userid 48) id 94D744066C; Tue, 18 Aug 2015 14:54:28 +0000 (UTC) From: bugzilla-daemon@bugzilla.kernel.org To: linux-scsi@vger.kernel.org Subject: [Bug 101891] mvsas prep failed, NULL pointer dereference in mvs_slot_task_free+0x5/0x1f0 [mvsas] Date: Tue, 18 Aug 2015 14:54:28 +0000 X-Bugzilla-Reason: None X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: AssignedTo scsi_drivers-other@kernel-bugs.osdl.org X-Bugzilla-Product: SCSI Drivers X-Bugzilla-Component: Other X-Bugzilla-Version: 2.5 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: davispuh@gmail.com X-Bugzilla-Status: NEW X-Bugzilla-Priority: P1 X-Bugzilla-Assigned-To: scsi_drivers-other@kernel-bugs.osdl.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: X-Bugzilla-URL: https://bugzilla.kernel.org/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP https://bugzilla.kernel.org/show_bug.cgi?id=101891 --- Comment #4 from D?vis --- (In reply to D?vis from comment #3) > I narrowed it down to this section of mvs_abort_task function > (drivers/scsi/mvsas/mv_sas.c) > > } else if (task->task_proto & SAS_PROTOCOL_SATA || > task->task_proto & SAS_PROTOCOL_STP) { > if (SAS_SATA_DEV == dev->dev_type) { > struct mvs_slot_info *slot = task->lldd_task; > u32 slot_idx = (u32)(slot - mvi->slot_info); > mv_dprintk("mvs_abort_task() mvi=%p task=%p " > "slot=%p slot_idx=x%x\n", > mvi, task, slot, slot_idx); > task->task_state_flags |= SAS_TASK_STATE_ABORTED; > mvs_slot_task_free(mvi, task, slot, slot_idx); > rc = TMF_RESP_FUNC_COMPLETE; > goto out; > } > > } > > > Basically this line "u32 slot_idx = (u32)(slot - mvi->slot_info)". > I think (slot - mvi->slot_info) returns 0x10 and that's why > (there's no "mvs_abort_task()" in journal so it crashes before that. > Sorry for being idiot, that line doesn't cause any pointer dereference and neither does previous line. It's just so obvious, compiler reordered instructions so that mvs_slot_task_free is executed before mv_dprintk is called and that's why it's not in journal. Even as title I wrote NULL pointer dereference in mvs_slot_task_free and that's exactly where had to look. So anyway when in mvs_task_prep and if pci_pool_alloc fails then task->lldd_task is NULL as can see task->lldd_task = NULL; slot->n_elem = n_elem; slot->slot_tag = tag; slot->buf = pci_pool_alloc(mvi->dma_pool, GFP_ATOMIC, &slot->buf_dma); if (!slot->buf) goto err_out_tag; then later it's aborted with mvs_abort_task and there mvs_slot_task_free is called with (slot = task->lldd_task) which is NULL and in mvs_slot_task_free { if (!slot->task) return; happens this NULL pointer dereference because slot is NULL. There's 2 ways to fix this, either check if slot is NULL before calling mvs_slot_task_free or just inside it check it. I went for second option as it seems easier and won't have to always check before calling. Here's a patch, haven't tested it yet but I think it will fix this and it's compiling right now so I'll let know once I'll have tested it. if (!sas_protocol_ata(task->task_proto)) diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c index 454536c..9c78074 100644 --- a/drivers/scsi/mvsas/mv_sas.c +++ b/drivers/scsi/mvsas/mv_sas.c @@ -887,6 +887,8 @@ static void mvs_slot_free(struct mvs_info *mvi, u32 rx_desc) static void mvs_slot_task_free(struct mvs_info *mvi, struct sas_task *task, struct mvs_slot_info *slot, u32 slot_idx) { + if (!slot) + return; if (!slot->task) return;