* [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors
2017-02-15 21:45 [PATCHv4 0/4] OPAL patches Jon Derrick
@ 2017-02-15 21:45 ` Jon Derrick
2017-02-16 14:33 ` Christoph Hellwig
2017-02-15 21:45 ` [PATCHv4 2/4] block/sed: Add helper to qualify response tokens Jon Derrick
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Jon Derrick @ 2017-02-15 21:45 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-kernel, linuxppc-dev,
Scott Bauer, Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
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>
Reviewed-by: Scott Bauer <scott.bauer@intel.com>
---
block/sed-opal.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 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;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors
2017-02-15 21:45 ` [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors Jon Derrick
@ 2017-02-16 14:33 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-02-16 14:33 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-block, linux-kernel, linuxppc-dev, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
On Wed, Feb 15, 2017 at 02:45:54PM -0700, Jon Derrick wrote:
> 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>
> Reviewed-by: Scott Bauer <scott.bauer@intel.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCHv4 2/4] block/sed: Add helper to qualify response tokens
2017-02-15 21:45 [PATCHv4 0/4] OPAL patches Jon Derrick
2017-02-15 21:45 ` [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors Jon Derrick
@ 2017-02-15 21:45 ` Jon Derrick
2017-02-16 14:40 ` Christoph Hellwig
2017-02-15 21:45 ` [PATCHv4 3/4] block/sed: Check received header lengths Jon Derrick
2017-02-15 21:45 ` [PATCHv4 4/4] MAINTAINERS: Remove powerpc's opal match Jon Derrick
3 siblings, 1 reply; 9+ messages in thread
From: Jon Derrick @ 2017-02-15 21:45 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-kernel, linuxppc-dev,
Scott Bauer, Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
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>
Reviewed-by: Scott Bauer <scott.bauer@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..00673cf 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(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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv4 2/4] block/sed: Add helper to qualify response tokens
2017-02-15 21:45 ` [PATCHv4 2/4] block/sed: Add helper to qualify response tokens Jon Derrick
@ 2017-02-16 14:40 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-02-16 14:40 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-block, linux-kernel, linuxppc-dev, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
On Wed, Feb 15, 2017 at 02:45:55PM -0700, Jon Derrick wrote:
> 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>
> Reviewed-by: Scott Bauer <scott.bauer@intel.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCHv4 3/4] block/sed: Check received header lengths
2017-02-15 21:45 [PATCHv4 0/4] OPAL patches Jon Derrick
2017-02-15 21:45 ` [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors Jon Derrick
2017-02-15 21:45 ` [PATCHv4 2/4] block/sed: Add helper to qualify response tokens Jon Derrick
@ 2017-02-15 21:45 ` Jon Derrick
2017-02-16 14:40 ` Christoph Hellwig
2017-02-15 21:45 ` [PATCHv4 4/4] MAINTAINERS: Remove powerpc's opal match Jon Derrick
3 siblings, 1 reply; 9+ messages in thread
From: Jon Derrick @ 2017-02-15 21:45 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-kernel, linuxppc-dev,
Scott Bauer, Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
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>
Reviewed-by: Scott Bauer <scott.bauer@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 00673cf..383d5d6 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 ||
+ slen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+ 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv4 3/4] block/sed: Check received header lengths
2017-02-15 21:45 ` [PATCHv4 3/4] block/sed: Check received header lengths Jon Derrick
@ 2017-02-16 14:40 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-02-16 14:40 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-block, linux-kernel, linuxppc-dev, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
On Wed, Feb 15, 2017 at 02:45:56PM -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>
> Reviewed-by: Scott Bauer <scott.bauer@intel.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCHv4 4/4] MAINTAINERS: Remove powerpc's opal match
2017-02-15 21:45 [PATCHv4 0/4] OPAL patches Jon Derrick
` (2 preceding siblings ...)
2017-02-15 21:45 ` [PATCHv4 3/4] block/sed: Check received header lengths Jon Derrick
@ 2017-02-15 21:45 ` Jon Derrick
2017-02-15 23:55 ` Andrew Donnellan
3 siblings, 1 reply; 9+ messages in thread
From: Jon Derrick @ 2017-02-15 21:45 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-kernel, linuxppc-dev,
Scott Bauer, Rafael Antognolli, Jens Axboe, Christoph Hellwig,
Greg Kroah-Hartman
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv4 4/4] MAINTAINERS: Remove powerpc's opal match
2017-02-15 21:45 ` [PATCHv4 4/4] MAINTAINERS: Remove powerpc's opal match Jon Derrick
@ 2017-02-15 23:55 ` Andrew Donnellan
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Donnellan @ 2017-02-15 23:55 UTC (permalink / raw)
To: Jon Derrick
Cc: Jens Axboe, Rafael Antognolli, Greg Kroah-Hartman, linux-kernel,
linux-block, linuxppc-dev, Christoph Hellwig, Scott Bauer
On 16/02/17 08:45, Jon Derrick wrote:
> 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>
It looks like this change will exclude the following PPC OPAL related files:
drivers/tty/hvc/hvc_opal.c - HYPERVISOR VIRTUAL CONSOLE DRIVER (which
lists linuxppc-dev as the mailing list, doesn't name a maintainer though)
drivers/i2c/busses/i2c-opal.c - I2C SUBSYSTEM
drivers/rtc/rtc-opal.c - REAL TIME CLOCK (RTC) SUBSYSTEM
Documentation/devicetree/bindings/i2c/i2c-opal.txt - I2C SUBSYSTEM, OPEN
FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Documentation/devicetree/bindings/powerpc/opal - OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS
Documentation/devicetree/bindings/powerpc/opal/oppanel-opal.txt - OPEN
FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Documentation/devicetree/bindings/rtc/rtc-opal.txt - OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS, REAL TIME CLOCK (RTC) SUBSYSTEM
Documentation/ABI/stable/sysfs-firmware-opal-elog - no other subsystem
Documentation/ABI/stable/sysfs-firmware-opal-dump - no other subsystem
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 9+ messages in thread