Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential use-after-free in error handling. #895

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

yjhjstz
Copy link
Member

@yjhjstz yjhjstz commented Jan 26, 2025

Fixes #ISSUE_Number

What does this PR do?

Motivation:
The original code called PQresultStatus() after invoking cdbdisp_clearCdbPgResults(),
which can lead to undefined behavior or potential segmentation faults.
Accessing a pg_result pointer after clearing the CDB results may reference
already freed memory.

Changes:

  • Capture the result of PQresultStatus() in a local variable before clearing results
  • Use the cached status for error reporting and comparison
  • Prevent accessing pg_result after memory deallocation

This change ensures that all result status checks and error reporting occur
before releasing the memory associated with the query results.

Enhances safety and readability in segment result processing

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


- Store PQresultStatus() result in a local variable before calling cdbdisp_clearCdbPgResults()
- Prevent accessing PQresultStatus() after clearing CDB results
- Improve consistency in error handling across different functions
- Avoid redundant function calls by caching result of PQresultStatus()

Enhances safety and readability in segment result processing
@yjhjstz yjhjstz added the type: Bug Something isn't working label Jan 26, 2025
@yjhjstz yjhjstz requested review from my-ship-it and reshke January 26, 2025 09:56
@yjhjstz yjhjstz requested a review from avamingli January 27, 2025 06:54
@yjhjstz yjhjstz merged commit 9150751 into apache:main Jan 27, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants