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;
Subject: Re: [PATCH] scsi: ses: fix some risks of out of bound access
To:     jejb@linux.ibm.com, martin.petersen@oracle.com
Cc:     linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
References: <1553499602-27810-1-git-send-email-jianchao.w.wang@oracle.com>
From:   "jianchao.wang" <jianchao.w.wang@oracle.com>
Message-ID: <4ce7e311-e1c8-9914-fb01-9ac3778f793c@oracle.com>
Date:   Tue, 26 Mar 2019 09:19:20 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <1553499602-27810-1-git-send-email-jianchao.w.wang@oracle.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Would anyone please take a look at this.
Our customer encounter terrible memory corruption and panic due to this.

Thanks
Jianchao

On 3/25/19 3:40 PM, Jianchao Wang wrote:
> We have some places with risk of accessing out of bound of the
> buffer allocated from slab, even it could corrupt the memory.
> 
> Signed-off-by: Jianchao Wang <jianchao.w.wang@oracle.com>
> ---
>  drivers/scsi/ses.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index 0fc3922..42e6a1f 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -520,6 +520,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
>  	struct ses_device *ses_dev = edev->scratch;
>  	int types = ses_dev->page1_num_types;
>  	unsigned char *hdr_buf = kzalloc(INIT_ALLOC_SIZE, GFP_KERNEL);
> +	unsigned char *page1_end = ses_dev->page1 + ses_dev->page1_len;
>  
>  	if (!hdr_buf)
>  		goto simple_populate;
> @@ -556,6 +557,11 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
>  	type_ptr = ses_dev->page1_types;
>  	components = 0;
>  	for (i = 0; i < types; i++, type_ptr += 4) {
> +		if (type_ptr > page1_end - 2) {
> +			sdev_printk(KERN_ERR, sdev, "Access out of bound of page1"
> +					"%p page1_end %p\n", page1_end, type_ptr);
> +			break;
> +		}
>  		for (j = 0; j < type_ptr[1]; j++) {
>  			char *name = NULL;
>  			struct enclosure_component *ecomp;
> @@ -566,10 +572,15 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
>  				} else {
>  					len = (desc_ptr[2] << 8) + desc_ptr[3];
>  					desc_ptr += 4;
> -					/* Add trailing zero - pushes into
> -					 * reserved space */
> -					desc_ptr[len] = '\0';
> -					name = desc_ptr;
> +					if (desc_ptr + len >= buf + page7_len) {
> +						desc_ptr = NULL;
> +					} else {
> +
> +						/* Add trailing zero - pushes into
> +						 * reserved space */
> +						desc_ptr[len] = '\0';
> +						name = desc_ptr;
> +					}
>  				}
>  			}
>  			if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
> @@ -693,7 +704,13 @@ static int ses_intf_add(struct device *cdev,
>  	/* begin at the enclosure descriptor */
>  	type_ptr = buf + 8;
>  	/* skip all the enclosure descriptors */
> -	for (i = 0; i < num_enclosures && type_ptr < buf + len; i++) {
> +	for (i = 0; i < num_enclosures; i++) {
> +		if (type_ptr >= buf + len) {
> +			sdev_printk(KERN_ERR, sdev, "Overflow the buf len = %d\n", len);
> +			err = -EINVAL;
> +			goto err_free;
> +		}
> +
>  		types += type_ptr[2];
>  		type_ptr += type_ptr[3] + 4;
>  	}
>