Messages in this thread | | | Subject | Re: [Cluster-devel] [PATCH] fs: gfs2: prevent OOB access in gfs2_read_sb() | From | Anant Thazhemadam <> | Date | Wed, 14 Oct 2020 18:44:06 +0530 |
| |
On 14/10/20 6:34 pm, Andrew Price wrote: > On 13/10/2020 16:26, Anant Thazhemadam wrote: >> In gfs2_read_sb(), if the condition >> (d != sdp->sd_heightsize[x - 1] || m) >> isn't satisfied (in the first 11 iterations), the loop continues, >> and begins to perform out-of-bounds access. >> Fix this out-of-bounds access by introducing a condition in the for loop >> that ensures that no more than GFS2_MAX_META_HEIGHT + 1 elements are >> accessed. >> >> In addition to this, if the above condition is satisfied when >> x = GFS2_MAX_META_HEIGHT (which = 10), and the flow of control breaks >> out of the loop, then an out-of-bounds access is performed again while >> assigning sdp->sd_heightsize[x] = ~0 (since x would be 11 now.), and >> also the assertion that spd->sd_max_height <= GFS2_MAX_META_HEIGHT would >> fail. >> Fix this out-of-bounds access and ensure that the assertion doesn't fail >> by introducing another variable "index", which keeps track of the last >> valid value of x (pre-increment) that can be used. > > That's not quite the right approach. Your analysis below is correct: the problem stems from the block size in the superblock being zeroed by the fuzzer. So the correct fix would be to add a validation check for sb_bsize (gfs2_check_sb() is lacking somewhat). Valid values are powers of 2 between 512 and the page size. >
I see. Thanks for the review! I'll send in a v2 that implements this check soon enough.
Thanks, Anant
| |