You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/03/07 19:50:47 UTC
[GitHub] [trafficserver] cmcfarlen opened a new pull request #8716: Check bounds before accessing Vol::evacuate array
cmcfarlen opened a new pull request #8716:
URL: https://github.com/apache/trafficserver/pull/8716
Fixes #8647
This PR does bounds checking on the `Vol::evacuate` array. While this may fix crashes related to `Dir` entries with invalid offsets, it does not address whatever happened to generate the bad offsets. So this is a partial fix at best.
There are other places where this array is accessed unchecked from `dir_evac_bucket` but these are the places that are involved in the reported crash.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #8716: Check bounds before accessing Vol::evacuate array
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #8716:
URL: https://github.com/apache/trafficserver/pull/8716#discussion_r821214094
##########
File path: iocore/cache/P_CacheVol.h
##########
@@ -206,6 +206,12 @@ struct Vol : public Continuation {
int dir_check(bool fix);
int db_check(bool fix);
+ bool
+ evac_bucket_valid(off_t bucket)
+ {
+ return (bucket >= 0 && bucket < evacuate_size);
Review comment:
How can an `off_t` be less than zero?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #8716: Check bounds before accessing Vol::evacuate array
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #8716:
URL: https://github.com/apache/trafficserver/pull/8716#issuecomment-1061273717
Looks reasonable to me.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #8716: Check bounds before accessing Vol::evacuate array
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #8716:
URL: https://github.com/apache/trafficserver/pull/8716#issuecomment-1061268205
I will look at it.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] cmcfarlen commented on a change in pull request #8716: Check bounds before accessing Vol::evacuate array
Posted by GitBox <gi...@apache.org>.
cmcfarlen commented on a change in pull request #8716:
URL: https://github.com/apache/trafficserver/pull/8716#discussion_r821219612
##########
File path: iocore/cache/P_CacheVol.h
##########
@@ -206,6 +206,12 @@ struct Vol : public Continuation {
int dir_check(bool fix);
int db_check(bool fix);
+ bool
+ evac_bucket_valid(off_t bucket)
+ {
+ return (bucket >= 0 && bucket < evacuate_size);
Review comment:
The macro `dir_evac_bucket` produces a `long long` and seem to be stored as `off_t` in other places in the code so I thought it might be a reasonable type for the argument here. `off_t` is a signed type so I check for >= 0. Ideally the macro would return an unsigned bucket, but I'm not sure the implication of that change.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall merged pull request #8716: Check bounds before accessing Vol::evacuate array
Posted by GitBox <gi...@apache.org>.
bryancall merged pull request #8716:
URL: https://github.com/apache/trafficserver/pull/8716
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org