You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/09/03 11:51:01 UTC
[GitHub] [mynewt-core] kasjer opened a new pull request #2371: hw/drivers/i2s: Add buffer overwrite check
kasjer opened a new pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371
It is easy to accidentally write past the sample buffer.
Such writes could lead to difficult to trace memory corruptions.
With this change most common errors will be detected early.
To have checking code working syscfg value I2S_BUFFER_GUARD
should specify how many bytes before and after actual data
is reserved and filled with pattern.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#issuecomment-686473611
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] wes3 commented on a change in pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
wes3 commented on a change in pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#discussion_r487422823
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] wes3 commented on a change in pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
wes3 commented on a change in pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#discussion_r487422823
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#issuecomment-686473611
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#issuecomment-696602522
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#issuecomment-696602522
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] wes3 commented on a change in pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
wes3 commented on a change in pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#discussion_r487422823
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -125,10 +126,16 @@ i2s_buffers_from_pool(struct i2s *i2s, struct i2s_buffer_pool *pool)
sample_data = (uintptr_t)&buffers[pool->buffer_count];
for (i = 0; i < pool->buffer_count; ++i) {
+ for (j = 0; j < I2S_BUFFER_GUARD; ++j, ++sample_data) {
+ *(uint8_t *)sample_data = 0xFD;
Review comment:
Another nit-pick: Maybe define the buffer guard value and use the definition instead of the hard-coded value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#issuecomment-686473611
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] kasjer merged pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
kasjer merged pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] sjanc commented on a change in pull request #2371: hw/drivers/i2s: Add buffer overwrite check
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #2371:
URL: https://github.com/apache/mynewt-core/pull/2371#discussion_r483447399
##########
File path: hw/drivers/i2s/src/i2s.c
##########
@@ -316,7 +323,16 @@ i2s_buffer_put(struct i2s *i2s, struct i2s_sample_buffer *buffer)
{
int sr;
int rc = I2S_OK;
+ int i;
+ if (I2S_BUFFER_GUARD) {
+ const uint8_t *pre = (uint8_t *)(buffer->sample_data) - 1;
Review comment:
nitpick: should be declared on function top
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org