Re: [PATCH 3/4] block/sed: Check received header lengths

Re: [PATCH 3/4] block/sed: Check received header lengths

[Scott Bauer]

On Wed, Feb 15, 2017 at 12:38:53PM -0700, Jon Derrick wrote:
> Add a buffer size check against discovery and response header lengths
> before we loop over their buffers.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  block/sed-opal.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index d6dd604..addf89e 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -340,10 +340,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
>  	const struct d0_header *hdr = (struct d0_header *)dev->resp;
>  	const u8 *epos = dev->resp, *cpos = dev->resp;
>  	u16 comid = 0;
> +	u32 hlen = be32_to_cpu(hdr->length);
>  
> -	print_buffer(dev->resp, be32_to_cpu(hdr->length));
> +	print_buffer(dev->resp, hlen);
>  
> -	epos += be32_to_cpu(hdr->length); /* end of buffer */
> +	if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
> +		pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
> +			sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
> +		return -EFAULT;
> +	}
> +
> +	epos += hlen; /* end of buffer */
>  	cpos += sizeof(*hdr); /* current position on buffer */
>  
>  	while (cpos < epos && supported) {
> @@ -713,6 +720,7 @@ static int response_parse(const u8 *buf, size_t length,
>  	int total;
>  	ssize_t token_length;
>  	const u8 *pos;
> +	u32 clen, plen, slen;
>  
>  	if (!buf)
>  		return -EFAULT;
> @@ -724,17 +732,16 @@ static int response_parse(const u8 *buf, size_t length,
>  	pos = buf;
>  	pos += sizeof(*hdr);
>  
> -	pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
> -		 be32_to_cpu(hdr->cp.length),
> -		 be32_to_cpu(hdr->pkt.length),
> -		 be32_to_cpu(hdr->subpkt.length));
> -
> -	if (hdr->cp.length == 0 || hdr->pkt.length == 0 ||
> -	    hdr->subpkt.length == 0) {
> -		pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
> -		       be32_to_cpu(hdr->cp.length),
> -		       be32_to_cpu(hdr->pkt.length),
> -		       be32_to_cpu(hdr->subpkt.length));
> +	clen = be32_to_cpu(hdr->cp.length);
> +	plen = be32_to_cpu(hdr->pkt.length);
> +	slen = be32_to_cpu(hdr->subpkt.length);
> +	pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
> +		 clen, plen, slen);
> +
> +	if (clen == 0 || plen == 0 || slen == 0 ||
> +	    (u64)clen + plen + slen > IO_BUFFER_LENGTH) {

I think we're over calculating here. From my understanding of the spec:
TCG_Storage_Architecture_Core_Spec_v2.01_r1.00.pdf
3.2.3
  ComPackets, Packets & Subpackets

The Comp packet size should the size of the packet, and the subpackets.
The "packet" should be the size of all the subpackets.
And lastly the subpacket should be the size its payload.

if ((u64) slen + sizeof(*hdr) > IO_BUFFER_LENGTH)

Since we never need the com packet length or pkt length (we never use them
anywhere) I think the above check is okay. Let me know if i'm wrong though,
or you have a different understanding of the lengths.

Re: [PATCH 3/4] block/sed: Check received header lengths

[Jon Derrick]

On 02/15/2017 11:15 AM, Scott Bauer wrote:
> On Wed, Feb 15, 2017 at 12:38:53PM -0700, Jon Derrick wrote:
>> Add a buffer size check against discovery and response header lengths
>> before we loop over their buffers.
>>
>> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
>> ---
>>  block/sed-opal.c | 35 +++++++++++++++++++++--------------
>>  1 file changed, 21 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/sed-opal.c b/block/sed-opal.c
>> index d6dd604..addf89e 100644
>> --- a/block/sed-opal.c
>> +++ b/block/sed-opal.c
>> @@ -340,10 +340,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
>>  	const struct d0_header *hdr = (struct d0_header *)dev->resp;
>>  	const u8 *epos = dev->resp, *cpos = dev->resp;
>>  	u16 comid = 0;
>> +	u32 hlen = be32_to_cpu(hdr->length);
>>  
>> -	print_buffer(dev->resp, be32_to_cpu(hdr->length));
>> +	print_buffer(dev->resp, hlen);
>>  
>> -	epos += be32_to_cpu(hdr->length); /* end of buffer */
>> +	if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
>> +		pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
>> +			sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
>> +		return -EFAULT;
>> +	}
>> +
>> +	epos += hlen; /* end of buffer */
>>  	cpos += sizeof(*hdr); /* current position on buffer */
>>  
>>  	while (cpos < epos && supported) {
>> @@ -713,6 +720,7 @@ static int response_parse(const u8 *buf, size_t length,
>>  	int total;
>>  	ssize_t token_length;
>>  	const u8 *pos;
>> +	u32 clen, plen, slen;
>>  
>>  	if (!buf)
>>  		return -EFAULT;
>> @@ -724,17 +732,16 @@ static int response_parse(const u8 *buf, size_t length,
>>  	pos = buf;
>>  	pos += sizeof(*hdr);
>>  
>> -	pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
>> -		 be32_to_cpu(hdr->cp.length),
>> -		 be32_to_cpu(hdr->pkt.length),
>> -		 be32_to_cpu(hdr->subpkt.length));
>> -
>> -	if (hdr->cp.length == 0 || hdr->pkt.length == 0 ||
>> -	    hdr->subpkt.length == 0) {
>> -		pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
>> -		       be32_to_cpu(hdr->cp.length),
>> -		       be32_to_cpu(hdr->pkt.length),
>> -		       be32_to_cpu(hdr->subpkt.length));
>> +	clen = be32_to_cpu(hdr->cp.length);
>> +	plen = be32_to_cpu(hdr->pkt.length);
>> +	slen = be32_to_cpu(hdr->subpkt.length);
>> +	pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
>> +		 clen, plen, slen);
>> +
>> +	if (clen == 0 || plen == 0 || slen == 0 ||
>> +	    (u64)clen + plen + slen > IO_BUFFER_LENGTH) {
> 
> I think we're over calculating here. From my understanding of the spec:
> TCG_Storage_Architecture_Core_Spec_v2.01_r1.00.pdf
> 3.2.3
>   ComPackets, Packets & Subpackets
> 
> The Comp packet size should the size of the packet, and the subpackets.
> The "packet" should be the size of all the subpackets.
> And lastly the subpacket should be the size its payload.
Yep I forgot that. Good catch!


> 
> if ((u64) slen + sizeof(*hdr) > IO_BUFFER_LENGTH)
> 
> Since we never need the com packet length or pkt length (we never use them
> anywhere) I think the above check is okay. Let me know if i'm wrong though,
> or you have a different understanding of the lengths.
Agreed

> 
> 
> 
> 
> 
> 
> 
> 

[PATCH 1/4] block/sed: Use ssize_t on atom parsers to return errors

[Jon Derrick]

The short atom parser can return an errno from decoding but does not
currently return the error as a signed value. Convert all of the parsers
to ssize_t.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c         | 28 ++++++++++++++--------------
 include/linux/sed-opal.h |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index e95b8a5..77623ad 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -635,8 +635,8 @@ static enum opal_token response_get_token(const struct parsed_resp *resp,
 	return tok->pos[0];
 }
 
-static size_t response_parse_tiny(struct opal_resp_tok *tok,
-				  const u8 *pos)
+static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
+				   const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = 1;
@@ -652,8 +652,8 @@ static size_t response_parse_tiny(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_short(struct opal_resp_tok *tok,
-				   const u8 *pos)
+static ssize_t response_parse_short(struct opal_resp_tok *tok,
+				    const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = (pos[0] & SHORT_ATOM_LEN_MASK) + 1;
@@ -665,7 +665,7 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
 		tok->type = OPAL_DTA_TOKENID_SINT;
 	} else {
 		u64 u_integer = 0;
-		int i, b = 0;
+		ssize_t i, b = 0;
 
 		tok->type = OPAL_DTA_TOKENID_UINT;
 		if (tok->len > 9) {
@@ -682,8 +682,8 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_medium(struct opal_resp_tok *tok,
-				    const u8 *pos)
+static ssize_t response_parse_medium(struct opal_resp_tok *tok,
+				     const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = (((pos[0] & MEDIUM_ATOM_LEN_MASK) << 8) | pos[1]) + 2;
@@ -699,8 +699,8 @@ static size_t response_parse_medium(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_long(struct opal_resp_tok *tok,
-				  const u8 *pos)
+static ssize_t response_parse_long(struct opal_resp_tok *tok,
+				   const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = ((pos[1] << 16) | (pos[2] << 8) | pos[3]) + 4;
@@ -716,8 +716,8 @@ static size_t response_parse_long(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_token(struct opal_resp_tok *tok,
-				   const u8 *pos)
+static ssize_t response_parse_token(struct opal_resp_tok *tok,
+				    const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = 1;
@@ -734,7 +734,7 @@ static int response_parse(const u8 *buf, size_t length,
 	struct opal_resp_tok *iter;
 	int num_entries = 0;
 	int total;
-	size_t token_length;
+	ssize_t token_length;
 	const u8 *pos;
 
 	if (!buf)
@@ -780,8 +780,8 @@ static int response_parse(const u8 *buf, size_t length,
 		else /* TOKEN */
 			token_length = response_parse_token(iter, pos);
 
-		if (token_length == -EINVAL)
-			return -EINVAL;
+		if (token_length < 0)
+			return token_length;
 
 		pos += token_length;
 		total -= token_length;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 205d520..4fc72d4 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -69,7 +69,7 @@ enum opal_response_token {
  */
 struct opal_resp_tok {
 	const u8 *pos;
-	size_t len;
+	ssize_t len;
 	enum opal_response_token type;
 	enum opal_atom_width width;
 	union {
-- 
1.8.3.1

[PATCH 1/4] block/sed: Use ssize_t on atom parsers to return errors

[Jon Derrick]

The short atom parser can return an errno from decoding but does not
currently return the error as a signed value. Convert all of the parsers
to ssize_t.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c         | 28 ++++++++++++++--------------
 include/linux/sed-opal.h |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index e95b8a5..77623ad 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -635,8 +635,8 @@ static enum opal_token response_get_token(const struct parsed_resp *resp,
 	return tok->pos[0];
 }
 
-static size_t response_parse_tiny(struct opal_resp_tok *tok,
-				  const u8 *pos)
+static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
+				   const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = 1;
@@ -652,8 +652,8 @@ static size_t response_parse_tiny(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_short(struct opal_resp_tok *tok,
-				   const u8 *pos)
+static ssize_t response_parse_short(struct opal_resp_tok *tok,
+				    const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = (pos[0] & SHORT_ATOM_LEN_MASK) + 1;
@@ -665,7 +665,7 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
 		tok->type = OPAL_DTA_TOKENID_SINT;
 	} else {
 		u64 u_integer = 0;
-		int i, b = 0;
+		ssize_t i, b = 0;
 
 		tok->type = OPAL_DTA_TOKENID_UINT;
 		if (tok->len > 9) {
@@ -682,8 +682,8 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_medium(struct opal_resp_tok *tok,
-				    const u8 *pos)
+static ssize_t response_parse_medium(struct opal_resp_tok *tok,
+				     const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = (((pos[0] & MEDIUM_ATOM_LEN_MASK) << 8) | pos[1]) + 2;
@@ -699,8 +699,8 @@ static size_t response_parse_medium(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_long(struct opal_resp_tok *tok,
-				  const u8 *pos)
+static ssize_t response_parse_long(struct opal_resp_tok *tok,
+				   const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = ((pos[1] << 16) | (pos[2] << 8) | pos[3]) + 4;
@@ -716,8 +716,8 @@ static size_t response_parse_long(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_token(struct opal_resp_tok *tok,
-				   const u8 *pos)
+static ssize_t response_parse_token(struct opal_resp_tok *tok,
+				    const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = 1;
@@ -734,7 +734,7 @@ static int response_parse(const u8 *buf, size_t length,
 	struct opal_resp_tok *iter;
 	int num_entries = 0;
 	int total;
-	size_t token_length;
+	ssize_t token_length;
 	const u8 *pos;
 
 	if (!buf)
@@ -780,8 +780,8 @@ static int response_parse(const u8 *buf, size_t length,
 		else /* TOKEN */
 			token_length = response_parse_token(iter, pos);
 
-		if (token_length == -EINVAL)
-			return -EINVAL;
+		if (token_length < 0)
+			return token_length;
 
 		pos += token_length;
 		total -= token_length;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 205d520..4fc72d4 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -69,7 +69,7 @@ enum opal_response_token {
  */
 struct opal_resp_tok {
 	const u8 *pos;
-	size_t len;
+	ssize_t len;
 	enum opal_response_token type;
 	enum opal_atom_width width;
 	union {
-- 
1.8.3.1

[PATCH 2/4] block/sed: Add helper to qualify response tokens

[Jon Derrick]

Add helper which verifies the response token is valid and matches the
expected value. Merges token_type and response_get_token.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 77623ad..d6dd604 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -591,48 +591,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
 	return 0;
 }
 
-static enum opal_response_token token_type(const struct parsed_resp *resp,
-					   int n)
+static const struct opal_resp_tok *response_get_token(
+				const struct parsed_resp *resp,
+				int n)
 {
 	const struct opal_resp_tok *tok;
 
 	if (n >= resp->num) {
 		pr_err("Token number doesn't exist: %d, resp: %d\n",
 		       n, resp->num);
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
 	tok = &resp->toks[n];
 	if (tok->len == 0) {
 		pr_err("Token length must be non-zero\n");
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
-	return tok->type;
-}
-
-/*
- * This function returns 0 in case of invalid token. One should call
- * token_type() first to find out if the token is valid or not.
- */
-static enum opal_token response_get_token(const struct parsed_resp *resp,
-					  int n)
-{
-	const struct opal_resp_tok *tok;
-
-	if (n >= resp->num) {
-		pr_err("Token number doesn't exist: %d, resp: %d\n",
-		       n, resp->num);
-		return 0;
-	}
-
-	tok = &resp->toks[n];
-	if (tok->len == 0) {
-		pr_err("Token length must be non-zero\n");
-		return 0;
-	}
-
-	return tok->pos[0];
+	return tok;
 }
 
 static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
@@ -851,20 +828,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
 	return resp->toks[n].stored.u;
 }
 
+static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
+{
+	if (IS_ERR_OR_NULL(token) ||
+	    token->type != OPAL_DTA_TOKENID_TOKEN ||
+	    token->pos[0] != match)
+		return false;
+	return true;
+}
+
 static u8 response_status(const struct parsed_resp *resp)
 {
-	if (token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN &&
-	    response_get_token(resp, 0) == OPAL_ENDOFSESSION) {
+	const struct opal_resp_tok *tok;
+
+	tok = response_get_token(resp, 0);
+	if (response_token_matches(tok, OPAL_ENDOFSESSION))
 		return 0;
-	}
 
 	if (resp->num < 5)
 		return DTAERROR_NO_METHOD_STATUS;
 
-	if (token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN ||
-	    token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN ||
-	    response_get_token(resp, resp->num - 1) != OPAL_ENDLIST ||
-	    response_get_token(resp, resp->num - 5) != OPAL_STARTLIST)
+	tok = response_get_token(resp, resp->num - 5);
+	if (!response_token_matches(tok, OPAL_STARTLIST))
+		return DTAERROR_NO_METHOD_STATUS;
+
+	tok = response_get_token(resp, resp->num - 1);
+	if (!response_token_matches(tok, OPAL_ENDLIST))
 		return DTAERROR_NO_METHOD_STATUS;
 
 	return response_get_u64(resp, resp->num - 4);
-- 
1.8.3.1

[PATCH 2/4] block/sed: Add helper to qualify response tokens

[Jon Derrick]

Add helper which verifies the response token is valid and matches the
expected value. Merges token_type and response_get_token.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 77623ad..d6dd604 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -591,48 +591,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
 	return 0;
 }
 
-static enum opal_response_token token_type(const struct parsed_resp *resp,
-					   int n)
+static const struct opal_resp_tok *response_get_token(
+				const struct parsed_resp *resp,
+				int n)
 {
 	const struct opal_resp_tok *tok;
 
 	if (n >= resp->num) {
 		pr_err("Token number doesn't exist: %d, resp: %d\n",
 		       n, resp->num);
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
 	tok = &resp->toks[n];
 	if (tok->len == 0) {
 		pr_err("Token length must be non-zero\n");
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
-	return tok->type;
-}
-
-/*
- * This function returns 0 in case of invalid token. One should call
- * token_type() first to find out if the token is valid or not.
- */
-static enum opal_token response_get_token(const struct parsed_resp *resp,
-					  int n)
-{
-	const struct opal_resp_tok *tok;
-
-	if (n >= resp->num) {
-		pr_err("Token number doesn't exist: %d, resp: %d\n",
-		       n, resp->num);
-		return 0;
-	}
-
-	tok = &resp->toks[n];
-	if (tok->len == 0) {
-		pr_err("Token length must be non-zero\n");
-		return 0;
-	}
-
-	return tok->pos[0];
+	return tok;
 }
 
 static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
@@ -851,20 +828,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
 	return resp->toks[n].stored.u;
 }
 
+static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
+{
+	if (IS_ERR_OR_NULL(token) ||
+	    token->type != OPAL_DTA_TOKENID_TOKEN ||
+	    token->pos[0] != match)
+		return false;
+	return true;
+}
+
 static u8 response_status(const struct parsed_resp *resp)
 {
-	if (token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN &&
-	    response_get_token(resp, 0) == OPAL_ENDOFSESSION) {
+	const struct opal_resp_tok *tok;
+
+	tok = response_get_token(resp, 0);
+	if (response_token_matches(tok, OPAL_ENDOFSESSION))
 		return 0;
-	}
 
 	if (resp->num < 5)
 		return DTAERROR_NO_METHOD_STATUS;
 
-	if (token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN ||
-	    token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN ||
-	    response_get_token(resp, resp->num - 1) != OPAL_ENDLIST ||
-	    response_get_token(resp, resp->num - 5) != OPAL_STARTLIST)
+	tok = response_get_token(resp, resp->num - 5);
+	if (!response_token_matches(tok, OPAL_STARTLIST))
+		return DTAERROR_NO_METHOD_STATUS;
+
+	tok = response_get_token(resp, resp->num - 1);
+	if (!response_token_matches(tok, OPAL_ENDLIST))
 		return DTAERROR_NO_METHOD_STATUS;
 
 	return response_get_u64(resp, resp->num - 4);
-- 
1.8.3.1

[PATCH 3/4] block/sed: Check received header lengths

[Jon Derrick]

Add a buffer size check against discovery and response header lengths
before we loop over their buffers.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index d6dd604..addf89e 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -340,10 +340,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
 	const struct d0_header *hdr = (struct d0_header *)dev->resp;
 	const u8 *epos = dev->resp, *cpos = dev->resp;
 	u16 comid = 0;
+	u32 hlen = be32_to_cpu(hdr->length);
 
-	print_buffer(dev->resp, be32_to_cpu(hdr->length));
+	print_buffer(dev->resp, hlen);
 
-	epos += be32_to_cpu(hdr->length); /* end of buffer */
+	if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+		pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
+			sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
+		return -EFAULT;
+	}
+
+	epos += hlen; /* end of buffer */
 	cpos += sizeof(*hdr); /* current position on buffer */
 
 	while (cpos < epos && supported) {
@@ -713,6 +720,7 @@ static int response_parse(const u8 *buf, size_t length,
 	int total;
 	ssize_t token_length;
 	const u8 *pos;
+	u32 clen, plen, slen;
 
 	if (!buf)
 		return -EFAULT;
@@ -724,17 +732,16 @@ static int response_parse(const u8 *buf, size_t length,
 	pos = buf;
 	pos += sizeof(*hdr);
 
-	pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
-		 be32_to_cpu(hdr->cp.length),
-		 be32_to_cpu(hdr->pkt.length),
-		 be32_to_cpu(hdr->subpkt.length));
-
-	if (hdr->cp.length == 0 || hdr->pkt.length == 0 ||
-	    hdr->subpkt.length == 0) {
-		pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
-		       be32_to_cpu(hdr->cp.length),
-		       be32_to_cpu(hdr->pkt.length),
-		       be32_to_cpu(hdr->subpkt.length));
+	clen = be32_to_cpu(hdr->cp.length);
+	plen = be32_to_cpu(hdr->pkt.length);
+	slen = be32_to_cpu(hdr->subpkt.length);
+	pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
+		 clen, plen, slen);
+
+	if (clen == 0 || plen == 0 || slen == 0 ||
+	    (u64)clen + plen + slen > IO_BUFFER_LENGTH) {
+		pr_err("Bad header length. cp: %u, pkt: %u, subpkt: %u\n",
+		       clen, plen, slen);
 		print_buffer(pos, sizeof(*hdr));
 		return -EINVAL;
 	}
@@ -743,7 +750,7 @@ static int response_parse(const u8 *buf, size_t length,
 		return -EFAULT;
 
 	iter = resp->toks;
-	total = be32_to_cpu(hdr->subpkt.length);
+	total = slen;
 	print_buffer(pos, total);
 	while (total > 0) {
 		if (pos[0] <= TINY_ATOM_BYTE) /* tiny atom */
-- 
1.8.3.1

[PATCH 3/4] block/sed: Check received header lengths

[Jon Derrick]

Add a buffer size check against discovery and response header lengths
before we loop over their buffers.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index d6dd604..addf89e 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -340,10 +340,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
 	const struct d0_header *hdr = (struct d0_header *)dev->resp;
 	const u8 *epos = dev->resp, *cpos = dev->resp;
 	u16 comid = 0;
+	u32 hlen = be32_to_cpu(hdr->length);
 
-	print_buffer(dev->resp, be32_to_cpu(hdr->length));
+	print_buffer(dev->resp, hlen);
 
-	epos += be32_to_cpu(hdr->length); /* end of buffer */
+	if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+		pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
+			sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
+		return -EFAULT;
+	}
+
+	epos += hlen; /* end of buffer */
 	cpos += sizeof(*hdr); /* current position on buffer */
 
 	while (cpos < epos && supported) {
@@ -713,6 +720,7 @@ static int response_parse(const u8 *buf, size_t length,
 	int total;
 	ssize_t token_length;
 	const u8 *pos;
+	u32 clen, plen, slen;
 
 	if (!buf)
 		return -EFAULT;
@@ -724,17 +732,16 @@ static int response_parse(const u8 *buf, size_t length,
 	pos = buf;
 	pos += sizeof(*hdr);
 
-	pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
-		 be32_to_cpu(hdr->cp.length),
-		 be32_to_cpu(hdr->pkt.length),
-		 be32_to_cpu(hdr->subpkt.length));
-
-	if (hdr->cp.length == 0 || hdr->pkt.length == 0 ||
-	    hdr->subpkt.length == 0) {
-		pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
-		       be32_to_cpu(hdr->cp.length),
-		       be32_to_cpu(hdr->pkt.length),
-		       be32_to_cpu(hdr->subpkt.length));
+	clen = be32_to_cpu(hdr->cp.length);
+	plen = be32_to_cpu(hdr->pkt.length);
+	slen = be32_to_cpu(hdr->subpkt.length);
+	pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
+		 clen, plen, slen);
+
+	if (clen == 0 || plen == 0 || slen == 0 ||
+	    (u64)clen + plen + slen > IO_BUFFER_LENGTH) {
+		pr_err("Bad header length. cp: %u, pkt: %u, subpkt: %u\n",
+		       clen, plen, slen);
 		print_buffer(pos, sizeof(*hdr));
 		return -EINVAL;
 	}
@@ -743,7 +750,7 @@ static int response_parse(const u8 *buf, size_t length,
 		return -EFAULT;
 
 	iter = resp->toks;
-	total = be32_to_cpu(hdr->subpkt.length);
+	total = slen;
 	print_buffer(pos, total);
 	while (total > 0) {
 		if (pos[0] <= TINY_ATOM_BYTE) /* tiny atom */
-- 
1.8.3.1

[PATCH 4/4] MAINTAINERS: Remove powerpc's opal match

[Jon Derrick]

PPC's 'opal' match pattern also matches block/sed-opal.c, where it looks
like the 'arch/powerpc' file pattern should be enough to match powerpc
opal code by itself. Remove the opal regex pattern from powerpc.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b983b25..430dd02 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7404,7 +7404,6 @@ F:	drivers/pci/hotplug/pnv_php.c
 F:	drivers/pci/hotplug/rpa*
 F:	drivers/scsi/ibmvscsi/
 F:	tools/testing/selftests/powerpc
-N:	opal
 N:	/pmac
 N:	powermac
 N:	powernv
-- 
1.8.3.1