* [PATCH v1 0/1] 390/zcrypt: Validate length information in reply @ 2026-08-20 14:04 Holger Dengler 2026-08-20 14:04 ` [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler 0 siblings, 1 reply; 6+ messages in thread From: Holger Dengler @ 2026-08-20 14:04 UTC (permalink / raw) To: Harald Freudenberger Cc: dengler, linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Ingo Franzki This series contains adds validation of a length information in a hardware reply, to prevent buffer over-read/over-write in case of a broken/modified replies. This series applies on-top of Linus' master branch. Holger Dengler (1): s390/zcrypt: Validate length in reply before using it drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) base-commit: 59e6295fac26b8e85c1ea859cdd89fa1e47519d7 -- 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it 2026-08-20 14:04 [PATCH v1 0/1] 390/zcrypt: Validate length information in reply Holger Dengler @ 2026-08-20 14:04 ` Holger Dengler 2026-08-20 14:20 ` sashiko-bot 2026-08-20 14:46 ` Harald Freudenberger 0 siblings, 2 replies; 6+ messages in thread From: Holger Dengler @ 2026-08-20 14:04 UTC (permalink / raw) To: Harald Freudenberger Cc: dengler, linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Ingo Franzki The length information in the reply is used to copy the key token to the target buffer. An invalid information in t->len of the reply may cause an over-read of the target buffer and also a over-write of the target buffer. To prevent that, check t->len before using it. As the available space in destination and source buffer is always larger than the valid length value in the parameter block in the reply, compare t->len with this (already validated) length information. As a side effect, this check also prevents buffer over-read and over-write. Signed-off-by: Holger Dengler <dengler@linux.ibm.com> --- drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c index d4ce6352b5b2..54b8edd1dfc7 100644 --- a/drivers/s390/crypto/zcrypt_ccamisc.c +++ b/drivers/s390/crypto/zcrypt_ccamisc.c @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain, /* do not check the key here, it may be incomplete */ - /* copy the vlsc key token back */ + /* + * Copy the vlsc key token back. + * The available space in the destination (key_token) and the source + * (t) buffer is always larger as the valid range of prepparm->kb.len. + * Validate t->len ba comparing it with the length information in the + * param block of the request (prepparm->kb.len) + * The value range of prepparm->kb.len has been checked above. + */ t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token; + if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) { + ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n", + __func__, t->len); + rc = -EIO; + goto out; + } memcpy(key_token, t, t->len); *key_token_size = t->len; -- 2.55.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it 2026-08-20 14:04 ` [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler @ 2026-08-20 14:20 ` sashiko-bot 2026-08-20 15:40 ` Holger Dengler 2026-08-20 14:46 ` Harald Freudenberger 1 sibling, 1 reply; 6+ messages in thread From: sashiko-bot @ 2026-08-20 14:20 UTC (permalink / raw) To: Holger Dengler Cc: Vasily Gorbik, linux-s390, Heiko Carstens, Alexander Gordeev, Christian Borntraeger Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] The patch fixes an out-of-bounds read in `_ip_cprb_helper()` but misses identical vulnerabilities in `cca_gencipherkey()` and `_ep11_genaeskey()`. - [High] Unvalidated length fields in the CPRBX response payload are used to advance a memory pointer, causing severe out-of-bounds reads in `cca_query_crypto_facility()`. -- --- Patch [1]: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it --- commit 3fcaad86f04a2a9419691d5f1a54cc873f326675 Author: Holger Dengler <dengler@linux.ibm.com> s390/zcrypt: Validate length in reply before using it The length information in the reply is used to copy the key token to the target buffer. An invalid information in t->len of the reply may cause an over-read of the target buffer and also a over-write of the target buffer. To prevent that, check t->len before using it. > diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c > index d4ce6352b5b2e..54b8edd1dfc74 100644 > --- a/drivers/s390/crypto/zcrypt_ccamisc.c > +++ b/drivers/s390/crypto/zcrypt_ccamisc.c > @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain, > > /* do not check the key here, it may be incomplete */ > > - /* copy the vlsc key token back */ > + /* > + * Copy the vlsc key token back. > + * The available space in the destination (key_token) and the source > + * (t) buffer is always larger as the valid range of prepparm->kb.len. > + * Validate t->len ba comparing it with the length information in the > + * param block of the request (prepparm->kb.len) > + * The value range of prepparm->kb.len has been checked above. > + */ > t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token; > + if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) { > + ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n", > + __func__, t->len); > + rc = -EIO; > + goto out; > + } > memcpy(key_token, t, t->len); > *key_token_size = t->len; [Severity: High] This isn't a bug introduced by this patch, but since you are fixing the unvalidated length field from the device reply here, should similar validation be added to cca_gencipherkey() and _ep11_genaeskey()? In drivers/s390/crypto/zcrypt_ccamisc.c:cca_gencipherkey(), the code reads the generated token length directly from the untrusted reply and calculates the copy length without checking bounds against the structure size: keybuflen = prepparm->kb.tlv1.len - 2 * sizeof(uint16_t); ... if (keybuf) { if (*keybufsize >= keybuflen) memcpy(keybuf, t, keybuflen); Could a malicious or compromised device return an inflated length, causing memcpy() to read thousands of bytes out-of-bounds from the cprbmem allocation and potentially leak kernel memory to userspace? A similar unvalidated copy appears in drivers/s390/crypto/zcrypt_ep11misc.c:_ep11_genaeskey(), which relies entirely on the untrusted device data length: memcpy(keybuf, rep_pl->data, rep_pl->data_len); [Severity: High] This is a pre-existing issue, but does a similar vulnerability exist in drivers/s390/crypto/zcrypt_ccamisc.c:cca_query_crypto_facility() where the unvalidated length field is used to advance a memory pointer? In that function, the code blindly advances ptr using an unchecked length provided by the device payload: len = *((u16 *)ptr); if (len > sizeof(u16)) { ... ptr += len; } If the hardware returns a corrupt or malicious length, could ptr advance far beyond the cprbmem buffer bounds, leading to an out-of-bounds dereference on the next iteration or exposing arbitrary kernel memory during the subsequent memcpy()? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260820140439.892324-2-dengler@linux.ibm.com?part=1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it 2026-08-20 14:20 ` sashiko-bot @ 2026-08-20 15:40 ` Holger Dengler 0 siblings, 0 replies; 6+ messages in thread From: Holger Dengler @ 2026-08-20 15:40 UTC (permalink / raw) To: sashiko-reviews Cc: Vasily Gorbik, linux-s390, Heiko Carstens, Alexander Gordeev, Christian Borntraeger On 8/20/26 16:20, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > > Pre-existing issues: > - [High] The patch fixes an out-of-bounds read in `_ip_cprb_helper()` but misses identical vulnerabilities in `cca_gencipherkey()` and `_ep11_genaeskey()`. > - [High] Unvalidated length fields in the CPRBX response payload are used to advance a memory pointer, causing severe out-of-bounds reads in `cca_query_crypto_facility()`. > -- > > --- Patch [1]: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it --- > commit 3fcaad86f04a2a9419691d5f1a54cc873f326675 > Author: Holger Dengler <dengler@linux.ibm.com> > > s390/zcrypt: Validate length in reply before using it > > The length information in the reply is used to copy the key token to > the target buffer. An invalid information in t->len of the reply may > cause an over-read of the target buffer and also a over-write of the > target buffer. To prevent that, check t->len before using it. > >> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c >> index d4ce6352b5b2e..54b8edd1dfc74 100644 >> --- a/drivers/s390/crypto/zcrypt_ccamisc.c >> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c >> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain, >> >> /* do not check the key here, it may be incomplete */ >> >> - /* copy the vlsc key token back */ >> + /* >> + * Copy the vlsc key token back. >> + * The available space in the destination (key_token) and the source >> + * (t) buffer is always larger as the valid range of prepparm->kb.len. >> + * Validate t->len ba comparing it with the length information in the >> + * param block of the request (prepparm->kb.len) >> + * The value range of prepparm->kb.len has been checked above. >> + */ >> t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token; >> + if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) { >> + ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n", >> + __func__, t->len); >> + rc = -EIO; >> + goto out; >> + } >> memcpy(key_token, t, t->len); >> *key_token_size = t->len; > > [Severity: High] > This isn't a bug introduced by this patch, but since you are fixing the > unvalidated length field from the device reply here, should similar validation > be added to cca_gencipherkey() and _ep11_genaeskey()? > > In drivers/s390/crypto/zcrypt_ccamisc.c:cca_gencipherkey(), the code reads > the generated token length directly from the untrusted reply and calculates > the copy length without checking bounds against the structure size: > > keybuflen = prepparm->kb.tlv1.len - 2 * sizeof(uint16_t); > ... > if (keybuf) { > if (*keybufsize >= keybuflen) > memcpy(keybuf, t, keybuflen); > > Could a malicious or compromised device return an inflated length, causing > memcpy() to read thousands of bytes out-of-bounds from the cprbmem > allocation and potentially leak kernel memory to userspace? The memcpy is only called, if keylen is less equal *keysize (which is the available space in keybuf). As far as I can see, this prevents any over-writes of the keybuf buffer, even with a malicious response from the hardware. This objection is not relevant. > A similar unvalidated copy appears in > drivers/s390/crypto/zcrypt_ep11misc.c:_ep11_genaeskey(), which relies entirely > on the untrusted device data length: > > memcpy(keybuf, rep_pl->data, rep_pl->data_len); In _ep11_genaeskey(), the rep_pl->data_len must be less equal *keybufsize, otherwise the function returns with an error. This objection is false. > [Severity: High] > This is a pre-existing issue, but does a similar vulnerability exist in > drivers/s390/crypto/zcrypt_ccamisc.c:cca_query_crypto_facility() where the > unvalidated length field is used to advance a memory pointer? > > In that function, the code blindly advances ptr using an unchecked length > provided by the device payload: > > len = *((u16 *)ptr); > if (len > sizeof(u16)) { > ... > ptr += len; > } > > If the hardware returns a corrupt or malicious length, could ptr advance > far beyond the cprbmem buffer bounds, leading to an out-of-bounds dereference > on the next iteration or exposing arbitrary kernel memory during the > subsequent memcpy()? This objection is under investigation and will be fixed (if required) in another series. -- Mit freundlichen Grüßen / Kind regards Holger Dengler ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it 2026-08-20 14:04 ` [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler 2026-08-20 14:20 ` sashiko-bot @ 2026-08-20 14:46 ` Harald Freudenberger 2026-08-20 15:04 ` Holger Dengler 1 sibling, 1 reply; 6+ messages in thread From: Harald Freudenberger @ 2026-08-20 14:46 UTC (permalink / raw) To: Holger Dengler Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Ingo Franzki On 2026-08-20 16:04, Holger Dengler wrote: > The length information in the reply is used to copy the key token to > the target buffer. An invalid information in t->len of the reply may > cause an over-read of the target buffer and also a over-write of the > target buffer. To prevent that, check t->len before using it. > > As the available space in destination and source buffer is always > larger than the valid length value in the parameter block in the > reply, compare t->len with this (already validated) length > information. As a side effect, this check also prevents buffer > over-read and over-write. > > Signed-off-by: Holger Dengler <dengler@linux.ibm.com> > --- > drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c > b/drivers/s390/crypto/zcrypt_ccamisc.c > index d4ce6352b5b2..54b8edd1dfc7 100644 > --- a/drivers/s390/crypto/zcrypt_ccamisc.c > +++ b/drivers/s390/crypto/zcrypt_ccamisc.c > @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 > domain, > > /* do not check the key here, it may be incomplete */ > > - /* copy the vlsc key token back */ > + /* > + * Copy the vlsc key token back. > + * The available space in the destination (key_token) and the source > + * (t) buffer is always larger as the valid range of > prepparm->kb.len. > + * Validate t->len ba comparing it with the length information in the what is this funny "ba" in the previous sentence ? > + * param block of the request (prepparm->kb.len) > + * The value range of prepparm->kb.len has been checked above. > + */ > t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token; > + if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) { > + ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n", > + __func__, t->len); > + rc = -EIO; > + goto out; > + } > memcpy(key_token, t, t->len); > *key_token_size = t->len; Reviewed-by: Harald Freudenberger <freude@linux.ibm.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it 2026-08-20 14:46 ` Harald Freudenberger @ 2026-08-20 15:04 ` Holger Dengler 0 siblings, 0 replies; 6+ messages in thread From: Holger Dengler @ 2026-08-20 15:04 UTC (permalink / raw) To: freude Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Ingo Franzki On 8/20/26 16:46, Harald Freudenberger wrote: > On 2026-08-20 16:04, Holger Dengler wrote: >> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c >> b/drivers/s390/crypto/zcrypt_ccamisc.c >> index d4ce6352b5b2..54b8edd1dfc7 100644 >> --- a/drivers/s390/crypto/zcrypt_ccamisc.c >> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c >> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain, >> >> /* do not check the key here, it may be incomplete */ >> >> - /* copy the vlsc key token back */ >> + /* >> + * Copy the vlsc key token back. >> + * The available space in the destination (key_token) and the source >> + * (t) buffer is always larger as the valid range of prepparm- >> >kb.len. >> + * Validate t->len ba comparing it with the length information in >> the > > what is this funny "ba" in the previous sentence ? A typo. Should be "by". Will be fixed in v2. > >> + * param block of the request (prepparm->kb.len) >> + * The value range of prepparm->kb.len has been checked above. >> + */ >> t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token; >> + if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) { >> + ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n", >> + __func__, t->len); >> + rc = -EIO; >> + goto out; >> + } >> memcpy(key_token, t, t->len); >> *key_token_size = t->len; > > Reviewed-by: Harald Freudenberger <freude@linux.ibm.com> Thx, I'll pick it for the v2. -- Mit freundlichen Grüßen / Kind regards Holger Dengler ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-20 15:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-20 14:04 [PATCH v1 0/1] 390/zcrypt: Validate length information in reply Holger Dengler 2026-08-20 14:04 ` [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler 2026-08-20 14:20 ` sashiko-bot 2026-08-20 15:40 ` Holger Dengler 2026-08-20 14:46 ` Harald Freudenberger 2026-08-20 15:04 ` Holger Dengler
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.