Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <174a1911-6a12-a184-5a08-d18b1b7ab296@mcst.ru>
Date:   Mon, 3 Apr 2023 19:47:16 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH] scsi: megaraid: Fix null dereference
Content-Language: en-US
To:     jejb@linux.ibm.com, Kashyap Desai <kashyap.desai@broadcom.com>
Cc:     Sumit Saxena <sumit.saxena@broadcom.com>,
        Shivasharan S <shivasharan.srikanteshwara@broadcom.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        megaraidlinux.pdl@broadcom.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org
References: <20230403143440.1923323-1-Igor.A.Artemiev@mcst.ru>
 <84d55c1032a98de8b2118715d3ec435c409ca0a2.camel@linux.ibm.com>
From:   "Igor A. Artemiev" <Igor.A.Artemiev@mcst.ru>
In-Reply-To: <84d55c1032a98de8b2118715d3ec435c409ca0a2.camel@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 4/3/23 18:19, James Bottomley wrote:
> On Mon, 2023-04-03 at 17:34 +0300, Igor Artemiev wrote:
>> When cmdid == CMDID_INT_CMDS, the 'mbox' pointer is NULL but is
>> dereferenced below.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> Fixes: 0f2bb84d2a68 ("[SCSI] megaraid: simplify internal command
>> handling")
>> Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru>
>> ---
>>   drivers/scsi/megaraid.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
>> index bf491af9f0d6..4fbf92dc717e 100644
>> --- a/drivers/scsi/megaraid.c
>> +++ b/drivers/scsi/megaraid.c
>> @@ -1441,6 +1441,7 @@ mega_cmd_done(adapter_t *adapter, u8
>> completed[], int nstatus, int status)
>>                   */
>>                  if (cmdid == CMDID_INT_CMDS) {
>>                          scb = &adapter->int_scb;
>> +                       mbox = (mbox_t *)scb->raw_mbox;
> Have you actually seen this and if so which firmware?  I thought
> megaraid internal commands only ever returned success or fail (0 or 1)
> meaning they can never get into the sense processing case that is the
> only consumer of the mbox.
>
> James
>
No, I haven't seen this. A null dereference can be if the 'status'  is 
0x02. But if 'status' cannot be equal to 0x02, assignment isn't required.

Thanks,
Igor