Messages in this thread |  | | From | Hyunchul Lee <> | | Date | Thu, 7 May 2026 09:37:21 +0900 | | Subject | Re: [PATCH v2] ntfs: fix out-of-bounds write in ntfs_rl_collapse_range() merge path |
| |
2026년 5월 6일 (수) 오후 6:24, DaeMyung Kang <charsyam@gmail.com>님이 작성: > > ntfs_rl_collapse_range() merges the run on the left of the collapsed > region with the run on its right when they are contiguous. The contiguous > check chooses a clamped index when @new_1st_cnt is 0: > > i = new_1st_cnt == 0 ? 1 : new_1st_cnt; > if (ntfs_rle_lcn_contiguous(&new_rl[i - 1], &new_rl[i])) { > > but the merge itself uses the unclamped value: > > s_rl = &new_rl[new_1st_cnt - 1]; > s_rl->length += s_rl[1].length; > > When @new_1st_cnt is 0 this computes &new_rl[-1] and writes 8 bytes > before the kvcalloc() runlist buffer. The path is reachable through > fallocate(FALLOC_FL_COLLAPSE_RANGE) starting at vcn 0 against an > attribute whose first run after the collapsed region and the following > run are holes. In that case ntfs_rle_lcn_contiguous() returns true > because both checked entries are LCN_HOLE, so the merge path is entered > with @new_1st_cnt still 0. Such consecutive holes do not occur on a > well-formed runlist (NTFS keeps runlists coalesced in memory), so this > OOB path is only reachable from a crafted volume. > > A normal runlist has no element to the left of vcn 0, so the left/right > merge is not valid when @new_1st_cnt is 0. Require @new_1st_cnt to be > positive before checking or performing the merge. This skips the merge > entirely in that case instead of clamping the merge target. > > The out-of-bounds write can corrupt an adjacent slab object. On a > non-KASAN kernel, it is reachable after a crafted NTFS volume has been > mounted read-write with the legacy fs/ntfs driver, by a local user that > has write access to the crafted file. > > Fixes: 11ccc9107dc4 ("ntfs: update runlist handling and cluster allocator") > Suggested-by: Hyunchul Lee <hyc.lee@gmail.com> > Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
> --- > Changes since v1: > - Skip the merge entirely when @new_1st_cnt == 0 instead of using a > clamped index, per review feedback. The clamped form silently > changed the operation's semantics from "merge the run left of the > collapsed region with the run right of it" to "coalesce two > right-side runs"; the new form closes a path that is only > reachable on malformed runlists. Behaviour on well-formed runlists > is unchanged. > > fs/ntfs/runlist.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/fs/ntfs/runlist.c b/fs/ntfs/runlist.c > index da21dbeaaf66..e7de3d01257e 100644 > --- a/fs/ntfs/runlist.c > +++ b/fs/ntfs/runlist.c > @@ -2056,10 +2056,11 @@ struct runlist_element *ntfs_rl_collapse_range(struct runlist_element *dst_rl, i > * consists of holes. > */ > merge_cnt = 0; > - i = new_1st_cnt == 0 ? 1 : new_1st_cnt; > - if (ntfs_rle_lcn_contiguous(&new_rl[i - 1], &new_rl[i])) { > - /* Merge right and left */ > - s_rl = &new_rl[new_1st_cnt - 1]; > + if (new_1st_cnt > 0 && > + ntfs_rle_lcn_contiguous(&new_rl[new_1st_cnt - 1], > + &new_rl[new_1st_cnt])) { > + /* Merge right and left. */ > + s_rl = &new_rl[new_1st_cnt - 1]; > s_rl->length += s_rl[1].length; > merge_cnt = 1; > } > -- > 2.43.0 >
-- Thanks, Hyunchul
|  |