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

[C++] ConcatenateBuffers triggers a UBSan error when called with an empty buffer #36913

Closed
sfc-gh-ebrossard opened this issue Jul 27, 2023 · 0 comments · Fixed by #36914
Closed
Assignees
Milestone

Comments

@sfc-gh-ebrossard
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

If buffer->data() is nullptr here, ConcatenateBuffers calls std::memcpy with nullptr for the src argument, which is undefined behavior:

If either dest or src is an invalid or null pointer, the behavior is undefined, even if count is zero.

The simplest fix is probably to skip the std::memcpy call if the buffer has zero length.

Component(s)

C++

@ianmcook ianmcook changed the title ConcatenateBuffers triggers a UBSan error when called with an empty buffer [C++] ConcatenateBuffers triggers a UBSan error when called with an empty buffer Jul 27, 2023
@pitrou pitrou added this to the 14.0.0 milestone Jul 27, 2023
pitrou added a commit that referenced this issue Jul 27, 2023
…36914)

### Rationale for this change

This is a trivial fix for a UBSan error in calls to `ConcatenateBuffers` with an empty buffer that has a null data pointer.

### What changes are included in this PR?

Conditional call to `std::memcpy` based on whether the buffer's length is 0.

### Are these changes tested?

Test added in buffer_test.cc.

### Are there any user-facing changes?

No
* Closes: #36913

Lead-authored-by: Elliott Brossard <elliott.brossard@snowflake.com>
Co-authored-by: Elliott Brossard <64754120+sfc-gh-ebrossard@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@raulcd raulcd modified the milestones: 14.0.0, 13.0.0 Jul 28, 2023
raulcd pushed a commit that referenced this issue Jul 28, 2023
…36914)

### Rationale for this change

This is a trivial fix for a UBSan error in calls to `ConcatenateBuffers` with an empty buffer that has a null data pointer.

### What changes are included in this PR?

Conditional call to `std::memcpy` based on whether the buffer's length is 0.

### Are these changes tested?

Test added in buffer_test.cc.

### Are there any user-facing changes?

No
* Closes: #36913

Lead-authored-by: Elliott Brossard <elliott.brossard@snowflake.com>
Co-authored-by: Elliott Brossard <64754120+sfc-gh-ebrossard@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Antoine Pitrou <antoine@python.org>
R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
…ror (apache#36914)

### Rationale for this change

This is a trivial fix for a UBSan error in calls to `ConcatenateBuffers` with an empty buffer that has a null data pointer.

### What changes are included in this PR?

Conditional call to `std::memcpy` based on whether the buffer's length is 0.

### Are these changes tested?

Test added in buffer_test.cc.

### Are there any user-facing changes?

No
* Closes: apache#36913

Lead-authored-by: Elliott Brossard <elliott.brossard@snowflake.com>
Co-authored-by: Elliott Brossard <64754120+sfc-gh-ebrossard@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Antoine Pitrou <antoine@python.org>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…ror (apache#36914)

### Rationale for this change

This is a trivial fix for a UBSan error in calls to `ConcatenateBuffers` with an empty buffer that has a null data pointer.

### What changes are included in this PR?

Conditional call to `std::memcpy` based on whether the buffer's length is 0.

### Are these changes tested?

Test added in buffer_test.cc.

### Are there any user-facing changes?

No
* Closes: apache#36913

Lead-authored-by: Elliott Brossard <elliott.brossard@snowflake.com>
Co-authored-by: Elliott Brossard <64754120+sfc-gh-ebrossard@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants