| Message ID | tencent_2D332A9E751B474B521BD22569BA27BB0D08@qq.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V3] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow | expand |
On 10/16/24 11:22 AM, Edward Adam Davis wrote: > Syzbot reported a kernel BUG in ocfs2_truncate_inline. > There are two reasons for this: first, the parameter value passed is greater > than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline > are "unsigned int". > > So, we need to add a sanity check for byte_start and byte_len right before > ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater > than ocfs2_max_inline_data_with_xattr return -EFBIG. > > Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7 > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > --- > V1 -> V2: move sanity check to ocfs2_remove_inode_range > V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX > > fs/ocfs2/file.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c > index ad131a2fc58e..9327aa2f1bf4 100644 > --- a/fs/ocfs2/file.c > +++ b/fs/ocfs2/file.c > @@ -1784,6 +1784,12 @@ int ocfs2_remove_inode_range(struct inode *inode, > return 0; > > if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) { > + int max_inl = ocfs2_max_inline_data_with_xattr(inode->i_sb, di); Or rename it to 'id_count' refer to 'struct ocfs2_inline_data'. Better to leave a blank line here. > + if (byte_start > max_inl || byte_start + byte_len > max_inl) { > + ret = -EFBIG; Seems 'EINVAL' is more proper here. Please do corresponding change in commit log. > + mlog_errno(ret); > + goto out; > + } Better to leave a blank line. Thanks, Joseph > ret = ocfs2_truncate_inline(inode, di_bh, byte_start, > byte_start + byte_len, 0); > if (ret) {
On 10/16/24 7:43 PM, Edward Adam Davis wrote: > Syzbot reported a kernel BUG in ocfs2_truncate_inline. > There are two reasons for this: first, the parameter value passed is greater > than ocfs2_max_inline_data_with_xattr, second, the start and end parameters > of ocfs2_truncate_inline are "unsigned int". > > So, we need to add a sanity check for byte_start and byte_len right before > ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater > than ocfs2_max_inline_data_with_xattr return -EINVAL. > > Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7 > Signed-off-by: Edward Adam Davis <eadavis@qq.com> Looks fine. Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> > --- > V1 -> V2: move sanity check to ocfs2_remove_inode_range > V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX > V3 -> V4: rename variable, modify return value and comments > > fs/ocfs2/file.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c > index ad131a2fc58e..47121ee4b4df 100644 > --- a/fs/ocfs2/file.c > +++ b/fs/ocfs2/file.c > @@ -1784,6 +1784,14 @@ int ocfs2_remove_inode_range(struct inode *inode, > return 0; > > if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) { > + int id_count = ocfs2_max_inline_data_with_xattr(inode->i_sb, di); > + > + if (byte_start > id_count || byte_start + byte_len > id_count) { > + ret = -EINVAL; > + mlog_errno(ret); > + goto out; > + } > + > ret = ocfs2_truncate_inline(inode, di_bh, byte_start, > byte_start + byte_len, 0); > if (ret) {
On 10/16/24 7:47 PM, Joseph Qi wrote: > > > On 10/16/24 7:43 PM, Edward Adam Davis wrote: >> Syzbot reported a kernel BUG in ocfs2_truncate_inline. >> There are two reasons for this: first, the parameter value passed is greater >> than ocfs2_max_inline_data_with_xattr, second, the start and end parameters >> of ocfs2_truncate_inline are "unsigned int". >> >> So, we need to add a sanity check for byte_start and byte_len right before >> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater >> than ocfs2_max_inline_data_with_xattr return -EINVAL. >> >> Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com >> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7 >> Signed-off-by: Edward Adam Davis <eadavis@qq.com> > > Looks fine. > Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> > Fixes: 1afc32b95233 ("ocfs2: Write support for inline data") Cc: <stable@vger.kernel.org> >> --- >> V1 -> V2: move sanity check to ocfs2_remove_inode_range >> V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX >> V3 -> V4: rename variable, modify return value and comments >> >> fs/ocfs2/file.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c >> index ad131a2fc58e..47121ee4b4df 100644 >> --- a/fs/ocfs2/file.c >> +++ b/fs/ocfs2/file.c >> @@ -1784,6 +1784,14 @@ int ocfs2_remove_inode_range(struct inode *inode, >> return 0; >> >> if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) { >> + int id_count = ocfs2_max_inline_data_with_xattr(inode->i_sb, di); >> + >> + if (byte_start > id_count || byte_start + byte_len > id_count) { >> + ret = -EINVAL; >> + mlog_errno(ret); >> + goto out; >> + } >> + >> ret = ocfs2_truncate_inline(inode, di_bh, byte_start, >> byte_start + byte_len, 0); >> if (ret) { >
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index ad131a2fc58e..9327aa2f1bf4 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -1784,6 +1784,12 @@ int ocfs2_remove_inode_range(struct inode *inode, return 0; if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) { + int max_inl = ocfs2_max_inline_data_with_xattr(inode->i_sb, di); + if (byte_start > max_inl || byte_start + byte_len > max_inl) { + ret = -EFBIG; + mlog_errno(ret); + goto out; + } ret = ocfs2_truncate_inline(inode, di_bh, byte_start, byte_start + byte_len, 0); if (ret) {
Syzbot reported a kernel BUG in ocfs2_truncate_inline. There are two reasons for this: first, the parameter value passed is greater than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline are "unsigned int". So, we need to add a sanity check for byte_start and byte_len right before ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater than ocfs2_max_inline_data_with_xattr return -EFBIG. Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7 Signed-off-by: Edward Adam Davis <eadavis@qq.com> --- V1 -> V2: move sanity check to ocfs2_remove_inode_range V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX fs/ocfs2/file.c | 6 ++++++ 1 file changed, 6 insertions(+)