Skip to content

tpm2: Fix return value on full buffer in ASN1PushLength - #660

Merged
stefanberger merged 1 commit into
masterfrom
stefanberger/4master
Nov 4, 2026
Merged

stefanberger merged 1 commit into
masterfrom
stefanberger/4master

Conversation

@stefanberger

@stefanberger stefanberger commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

In ASN1PushLength(), the return condition was '(ctx->offset > 0) ? start - ctx->offset : 0'. When pushed length octets exactly fill the remaining buffer space, ctx->offset reaches 0 (the valid start of the buffer, whereas negative offset indicates buffer overflow). The function erroneously returned 0 (indicating failure) instead of the number of marshaled bytes.

Check 'ctx->offset >= 0' to correctly handle a full buffer, matching the error check convention in the rest of TpmASN1.c.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected length reporting for encoded certificate data in cases where no prior output has been written. This improves consistency when preparing certificate data for subsequent processing. Negative offsets continue to be treated as failures.

In ASN1PushLength(), the return condition was '(ctx->offset > 0) ?
start - ctx->offset : 0'. When pushed length octets exactly fill the
remaining buffer space, ctx->offset reaches 0 (the valid start of the
buffer, whereas negative offset indicates buffer overflow). The function
erroneously returned 0 (indicating failure) instead of the number of
marshaled bytes.

Check 'ctx->offset >= 0' to correctly handle a full buffer, matching the
error check convention in the rest of TpmASN1.c.

Co-authored-by: IBM Bob <noreply@ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

ASN1PushLength now returns the byte count when the marshaling offset is zero. Negative offsets still return failure.

Changes: ASN.1 Length Return Value

Layer / File(s) Summary
Handle zero marshaling offset
src/tpm2/TPMCmd/tpm/src/X509/TpmASN1.c
ASN1PushLength returns the byte count when the offset is zero. Negative offsets still return failure.

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Merge Risk: Minimal · up to b6952

The targeted return-value fix is consistent with the marshaling contract and introduces no identified merge risk.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the fix and its location, "tpm2: Fix return value on full buffer in ASN1PushLength". The raw summary says ASN1PushLength now reports bytes added when the marshaling offset is zero, and…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the fix and its location, "tpm2: Fix return value on full buffer in ASN1PushLength". The raw summary says ASN1PushLength now reports bytes added when the marshaling offset is zero, and it still fails only when the offset is negative. The objectives match this: the bug is the ctx->offset > 0 check returning 0 when the buffer fills exactly, and the fix accepts ctx->offset >= 0. The title is specific and describes the primary change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 69312a6d-f559-47af-b726-6a853c1f99c3
Commits

Reviewing files that changed from the base of the PR and between 7add127 and b69529b.

Files selected for processing: 1
  • src/tpm2/TPMCmd/tpm/src/X509/TpmASN1.c

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

@stefanberger
stefanberger merged commit 7cd8519 into master Nov 4, 2026
5 checks passed
@stefanberger
stefanberger deleted the stefanberger/4master branch November 4, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant