Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <3b2542225e519a8cf8e329745b0ee2e2464787c7.camel@redhat.com>
Subject: Re: [BUG] scsi: ses: out of bound accessing in
 ses_enclosure_data_process
From:   "Ewan D. Milne" <emilne@redhat.com>
To:     "Martin K. Petersen" <martin.petersen@oracle.com>,
        "jianchao.wang" <jianchao.w.wang@oracle.com>
Cc:     jejb@linux.ibm.com,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Junxiao Bi <junxiao.bi@oracle.com>, diego.gonzalez@oracle.com
Date:   Mon, 18 Mar 2019 11:22:35 -0400
In-Reply-To: <yq1k1gwkc6x.fsf@oracle.com>
References: <78dd3eca-7e8a-72f9-07f9-e2c7cc4569b0@oracle.com>
         <yq1k1gwkc6x.fsf@oracle.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 2019-03-18 at 01:01 -0400, Martin K. Petersen wrote:
> Jianchao,
> 
> > When our customer probe the lpfc devices, they encountered odd memory
> > corruption issues, and we get 'out of bound' access warning at
> > following position after open KASAN
> 
> Please provide the output of:
> 
> # sg_ses -p 1 /dev/sgN
> # sg_ses -p 7 /dev/sgN
> 
> for the enclosure device in question.
> 

The ses driver is allocating kernel buffers based upon the size
reported by RECEIVE DIAGNOSTIC commands, and is iterating through
them based on sizes in the individual descriptors.  It appears to
be vulnerable to incorrect data from the device causing out-of-bounds
memory access, because the for() test does not prevent the use of
the pointer in subsequent code, e.g.:

        for (i = 0; i < num_enclosures && type_ptr < buf + len; i++) {
                types += type_ptr[2];
                type_ptr += type_ptr[3] + 4;
        }

        ses_dev->page1_types = type_ptr;
        ses_dev->page1_num_types = types;

Whether or not this is the current problem, it's wrong.

-Ewan