cras_input: Fix for a potential security issue [chromium/src : main]
0 views
Skip to first unread message
Yu-Hsuan Hsu (Gerrit)
Apr 21, 2026, 4:38:29 AM (2 days ago) Apr 21
to chromium...@chromium.org, Daniel Angulo, feature-me...@chromium.org
Yu-Hsuan Hsu has uploaded the change for review![]( "Open in Gerrit")
Commit message
cras_input: Fix for a potential security issue
1. Add DCHECK to make sure audio_bus_->frames() is equal to CRAS actually delivered.
2. Added span for audio cras buffer.
Bug: 499870279
Change-Id: I2c50608d9c4ff2538ae8e4b662639303f520344c
Change diff
diff --git a/media/audio/cras/cras_input.cc b/media/audio/cras/cras_input.cc
index edb8f56..9a22b11 100644
--- a/media/audio/cras/cras_input.cc
+++ b/media/audio/cras/cras_input.cc
@@ -502,9 +502,13 @@
// The delay says how long ago the capture was, so we subtract the delay from
// Now() to find the capture time.
const base::TimeTicks capture_time = base::TimeTicks::Now() - delay;
-
- audio_bus_->FromInterleaved<SignedInt16SampleTypeTraits>(
- reinterpret_cast<int16_t*>(buffer), audio_bus_->frames());
+ // The frames should be equal to CRAS actually delivered.
+ DCHECK_EQ(static_cast<size_t>(audio_bus_->frames()), frames);
+ // SAFETY: buffer is guaranteed to be at least as large as the number of
+ // frames to read.
+ audio_bus_->FromInterleaved<SignedInt16SampleTypeTraits>(base::span(
+ reinterpret_cast<int16_t*>(buffer),
+ static_cast<size_t>(audio_bus_->channels() * audio_bus_->frames())));
peak_detector_.FindPeak(audio_bus_.get());
Change information
Files:
- M media/audio/cras/cras_input.cc
Change size: S
Delta: 1 file changed, 7 insertions(+), 3 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yu-Hsuan Hsu (Gerrit)
Apr 21, 2026, 4:43:19 AM (2 days ago) Apr 21
to Daniel Angulo, Hsinyu Chao, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Daniel Angulo and Hsinyu Chao
Yu-Hsuan Hsu added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Yu-Hsuan Hsu . resolved
Hi all, I have modified CL:7756967 and made sure this CL work. PTAL
Related details
Attention is currently required from:
- Daniel Angulo
- Hsinyu Chao
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Angulo (Gerrit)
11:39 AM (3 hours ago) 11:39 AM
to Yu-Hsuan Hsu, Code Review Nudger, Hsinyu Chao, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Hsinyu Chao and Yu-Hsuan Hsu
Daniel Angulo added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Attention is currently required from:
- Hsinyu Chao
- Yu-Hsuan Hsu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Angulo (Gerrit)
11:47 AM (3 hours ago) 11:47 AM
to Yu-Hsuan Hsu, Code Review Nudger, Hsinyu Chao, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Hsinyu Chao and Yu-Hsuan Hsu
Daniel Angulo added 1 comment![]( "Open in Gerrit")
File media/audio/cras/cras_input.cc
Line 509, Patchset 2 (Latest):
audio_bus_->FromInterleaved<SignedInt16SampleTypeTraits>(base::span(Daniel Angulo . unresolved
need UNSAFE_BUFFERS?
Related details
Attention is currently required from:
- Hsinyu Chao
- Yu-Hsuan Hsu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arthur Sonzogni (Gerrit)
12:04 PM (3 hours ago) 12:04 PM
to Yu-Hsuan Hsu, Code Review Nudger, Daniel Angulo, Hsinyu Chao, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Hsinyu Chao and Yu-Hsuan Hsu
Arthur Sonzogni added 1 comment![]( "Open in Gerrit")
File media/audio/cras/cras_input.cc
Line 462, Patchset 2 (Latest):
libcras_stream_cb_data_get_frames(data, &frames);
libcras_stream_cb_data_get_buf(data, &buf);
libcras_stream_cb_data_get_latency(data, &latency);
libcras_stream_cb_data_get_usr_arg(data, &usr_arg);
CrasInputStream* me = static_cast<CrasInputStream*>(usr_arg);
me->ReadAudio(frames, buf, &latency);Arthur Sonzogni . unresolved
drive-by: Would it be possible to convert (buf, frames) to a span<int16_t> as close as possible of the C library? Then we understand why we can't spanify further.
Then the "SAFETY:/UNSAFE_BUFFERS" comment would be easier to double check.
Related details
Attention is currently required from:
- Hsinyu Chao
- Yu-Hsuan Hsu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yu-Hsuan Hsu (Gerrit)
12:17 PM (3 hours ago) 12:17 PM
to Arthur Sonzogni, Code Review Nudger, Daniel Angulo, Hsinyu Chao, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Arthur Sonzogni, Daniel Angulo and Hsinyu Chao
Yu-Hsuan Hsu added 2 comments![]( "Open in Gerrit")
File media/audio/cras/cras_input.cc
Line 462, Patchset 2:
libcras_stream_cb_data_get_frames(data, &frames);
libcras_stream_cb_data_get_buf(data, &buf);
libcras_stream_cb_data_get_latency(data, &latency);
libcras_stream_cb_data_get_usr_arg(data, &usr_arg);
CrasInputStream* me = static_cast<CrasInputStream*>(usr_arg);
me->ReadAudio(frames, buf, &latency);Arthur Sonzogni . unresolved
drive-by: Would it be possible to convert (buf, frames) to a span<int16_t> as close as possible of the C library? Then we understand why we can't spanify further.
Then the "SAFETY:/UNSAFE_BUFFERS" comment would be easier to double check.
Yu-Hsuan Hsu
I'm not sure how to do that. Do you have any example?
Line 509, Patchset 2:
audio_bus_->FromInterleaved<SignedInt16SampleTypeTraits>(base::span(Daniel Angulo . resolved
Yu-Hsuan Hsuneed UNSAFE_BUFFERS?
Done
Related details
Attention is currently required from:
- Arthur Sonzogni
- Daniel Angulo
- Hsinyu Chao
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arthur Sonzogni (Gerrit)
12:19 PM (3 hours ago) 12:19 PM
to Yu-Hsuan Hsu, Code Review Nudger, Daniel Angulo, Hsinyu Chao, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Daniel Angulo, Hsinyu Chao and Yu-Hsuan Hsu
Arthur Sonzogni added 2 comments![]( "Open in Gerrit")
File media/audio/cras/cras_input.cc
Line 462, Patchset 2:
libcras_stream_cb_data_get_frames(data, &frames);
libcras_stream_cb_data_get_buf(data, &buf);
libcras_stream_cb_data_get_latency(data, &latency);
libcras_stream_cb_data_get_usr_arg(data, &usr_arg);
CrasInputStream* me = static_cast<CrasInputStream*>(usr_arg);
me->ReadAudio(frames, buf, &latency);Arthur Sonzogni . unresolved
Yu-Hsuan Hsudrive-by: Would it be possible to convert (buf, frames) to a span<int16_t> as close as possible of the C library? Then we understand why we can't spanify further.
Then the "SAFETY:/UNSAFE_BUFFERS" comment would be easier to double check.
I'm not sure how to do that. Do you have any example?
Arthur Sonzogni
Update `CrasInputStream::ReadAudio` signature to use a span.
Create the span here instead of in L508
Line 506, Patchset 4 (Latest):
DCHECK_EQ(static_cast<size_t>(audio_bus_->frames()), frames);Arthur Sonzogni . unresolved
Can this be a CHECK ? Otherwise, we still have a potential OOB, because users don't run with CHECK enabled in general.
Related details
Attention is currently required from:
- Daniel Angulo
- Hsinyu Chao
- Yu-Hsuan Hsu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages