tpm: ibmvtpm: check the response size against the buffer size
Subsystems (4)
PATCH 1/1 tpm: ibmvtpm: check the response size against the buffer size
The send path validates the transfer size against the size of the RTCE buffer:
if (count > ibmvtpm->rtce_size) {
dev_err(ibmvtpm->dev,
"Invalid size in send: count=%zd, rtce_size=%d\n",
count, ibmvtpm->rtce_size);
return -EIO;
}The receive path has no such check: the response length is whatever the hypervisor reported in the completion CRQ, and it is only compared against the size the caller asked for before being used to copy and clear the RTCE buffer:
len = ibmvtpm->res_len;
if (count < len) {
...
return -EIO;
}
spin_lock(&ibmvtpm->rtce_lock);
memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, len);
memset(ibmvtpm->rtce_buf, 0, len);A response length larger than the RTCE buffer makes the memcpy read past the buffer and the memset clear past it.
Reject a response that does not fit in the RTCE buffer, mirroring the check the send path already performs.
--- drivers/char/tpm/tpm_ibmvtpm.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_ibmvtpm.c +7 -0
@@ -121,6 +121,13 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)return -EIO ; } +if (len > ibmvtpm->rtce_size) { +dev_err (ibmvtpm->dev, +"Invalid size in recv: res_len=%d, rtce_size=%d\n" , + len, ibmvtpm->rtce_size); +return -EIO ; + } +spin_lock (&ibmvtpm->rtce_lock);memcpy ((void *)buf, (void *)ibmvtpm->rtce_buf, len);memset (ibmvtpm->rtce_buf,0 , len);
Signature · 1 line
2.50.1
Jarkko Sakkinen
maintainerQuoted 29 lines · Guo Zihao
The send path validates the transfer size against the size of the RTCE buffer:
if (count > ibmvtpm->rtce_size) { dev_err(ibmvtpm->dev, "Invalid size in send: count=%zd, rtce_size=%d\n", count, ibmvtpm->rtce_size); return -EIO; }The receive path has no such check: the response length is whatever the hypervisor reported in the completion CRQ, and it is only compared against the size the caller asked for before being used to copy and clear the RTCE buffer:
len = ibmvtpm->res_len; if (count < len) { ... return -EIO; } spin_lock(&ibmvtpm->rtce_lock); memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, len); memset(ibmvtpm->rtce_buf, 0, len);A response length larger than the RTCE buffer makes the memcpy read past the buffer and the memset clear past it.
Quoted 7 lines
Reject a response that does not fit in the RTCE buffer, mirroring the check the send path already performs.
Fixes 132f76294744 ("drivers/char/tpm: Add new device driver to support IBM vTPM")Signed-off-by Guo Zihao <[email protected]>---
Ditto. I.e, I'm not sure if this is a bug fix or just better validation for failing components.
Br, Jarkko
PATCH v2 1/1 tpm: ibmvtpm: check the response size against the buffer size
The send path validates the transfer size against the size of the RTCE buffer:
if (count > ibmvtpm->rtce_size) {
dev_err(ibmvtpm->dev,
"Invalid size in send: count=%zd, rtce_size=%d\n",
count, ibmvtpm->rtce_size);
return -EIO;
}The receive path has no such check: the response length is whatever the hypervisor reported in the completion CRQ, and it is only compared against the size the caller asked for before being used to copy and clear the RTCE buffer:
len = ibmvtpm->res_len;
if (count < len) {
...
return -EIO;
}
spin_lock(&ibmvtpm->rtce_lock);
memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, len);
memset(ibmvtpm->rtce_buf, 0, len);A response length larger than the RTCE buffer makes the memcpy read past the buffer and the memset clear past it.
Reject a response that does not fit in the RTCE buffer, mirroring the check the send path already performs.
---
v2: Drop the Fixes and Cc: stable tags, as suggested by Jarkko
Sakkinen: a hypervisor reporting an oversized response is not known to violate the architecture, so this is hardening rather than a bug fix. No code changes.
drivers/char/tpm/tpm_ibmvtpm.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_ibmvtpm.c +7 -0
@@ -121,6 +121,13 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)return -EIO ; } +if (len > ibmvtpm->rtce_size) { +dev_err (ibmvtpm->dev, +"Invalid size in recv: res_len=%d, rtce_size=%d\n" , + len, ibmvtpm->rtce_size); +return -EIO ; + } +spin_lock (&ibmvtpm->rtce_lock);memcpy ((void *)buf, (void *)ibmvtpm->rtce_buf, len);memset (ibmvtpm->rtce_buf,0 , len);
Signature · 1 line
2.50.1
Quoted 64 lines · Guo Zihao
The send path validates the transfer size against the size of the RTCE buffer:
if (count > ibmvtpm->rtce_size) { dev_err(ibmvtpm->dev, "Invalid size in send: count=%zd, rtce_size=%d\n", count, ibmvtpm->rtce_size); return -EIO; }The receive path has no such check: the response length is whatever the hypervisor reported in the completion CRQ, and it is only compared against the size the caller asked for before being used to copy and clear the RTCE buffer:
len = ibmvtpm->res_len; if (count < len) { ... return -EIO; } spin_lock(&ibmvtpm->rtce_lock); memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, len); memset(ibmvtpm->rtce_buf, 0, len);A response length larger than the RTCE buffer makes the memcpy read past the buffer and the memset clear past it.
Reject a response that does not fit in the RTCE buffer, mirroring the check the send path already performs.
Signed-off-by Guo Zihao <[email protected]>---
v2: Drop the Fixes and Cc: stable tags, as suggested by Jarkko
Sakkinen: a hypervisor reporting an oversized response is not known to violate the architecture, so this is hardening rather than a bug fix. No code changes.drivers/char/tpm/tpm_ibmvtpm.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/char/tpm/tpm_ibmvtpm.c +7
-0@@ -121,6 +121,13 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)return -EIO ; } +if (len > ibmvtpm->rtce_size) { +dev_err (ibmvtpm->dev, +"Invalid size in recv: res_len=%d, rtce_size=%d\n" , + len, ibmvtpm->rtce_size); +return -EIO ; + } +spin_lock (&ibmvtpm->rtce_lock);memcpy ((void *)buf, (void *)ibmvtpm->rtce_buf, len);memset (ibmvtpm->rtce_buf,0 , len);Signature · 1 line
2.50.1
Thank you.
Br, Jarkko