fix: fix a potential out of bounds issue in fuse #6847
Conversation
| buf := resp.Data[:min(req.Size, int(int64(r.Size())-req.Offset))] | ||
| // Data has a capacity of Size | ||
| buf := resp.Data[:int(req.Size)] |
There was a problem hiding this comment.
What is the out of bounds error that this is fixing? If the issue was that buf could be empty then we still get the error.
Have we introduced another possible out of bounds error here if the response data is less than the request size? Is it really safe to remove the min check here?
There was a problem hiding this comment.
The fuse library gives us a Data buffer a capacity of Size. I believe the issue was that Size()` reported an incorrect size (-1, 0, etc.).
There was a problem hiding this comment.
I could be wrong here, but I don't think the issue was calculating buf := resp.Data[-1] because that would throw an invalid slice index instead of the index out of range error that was reported in the issue. If so then the panic was caused by the index being too large not too small, and therefore this fix likely won't resolve the issue.
Also, is it possible that req.Size is bigger than the buffer and so we'll run into an out of bounds error with this change?
There was a problem hiding this comment.
So, it should be impossible:
There was a problem hiding this comment.
But you're right, -1 would give us a different error. The only sane answer is that Size is too big.
We likely encountered a file that misreported its size. That or there was no bug here and we hit an issue somewhere else. Regardless, there's no reason not to simplify this code and this should fix the issue.
|
I'm going to merge this as it's less broken (but there's still something wrong). |
We likely encountered a file that misreported its size. That or there was no bug here and we hit an issue somewhere else. Regardless, there's no reason not to simplify this code and this should fix the issue.