Joseph Qi
2019-Jul-26 09:38 UTC
[Ocfs2-devel] [PATCH 2/3] fs: ocfs2: Fix a possible null-pointer dereference in ocfs2_write_end_nolock()
On 19/7/26 11:37, Jia-Ju Bai wrote:> In ocfs2_write_end_nolock(), there are an if statement on lines 1976, > 2047 and 2058, to check whether handle is NULL: > if (handle) > > When handle is NULL, it is used on line 2045: > ocfs2_update_inode_fsync_trans(handle, inode, 1); > oi->i_sync_tid = handle->h_transaction->t_tid; > > Thus, a possible null-pointer dereference may occur. > > To fix this bug, handle is checked before calling > ocfs2_update_inode_fsync_trans(). > > This bug is found by a static analysis tool STCheck written by us. > > Signed-off-by: Jia-Ju Bai <baijiaju1990 at gmail.com>Looks good. Reviewed-by: Joseph Qi <joseph.qi at linux.alibaba.com>> --- > fs/ocfs2/aops.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index a4c905d6b575..5473bd99043e 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -2042,7 +2042,8 @@ int ocfs2_write_end_nolock(struct address_space *mapping, > inode->i_mtime = inode->i_ctime = current_time(inode); > di->i_mtime = di->i_ctime = cpu_to_le64(inode->i_mtime.tv_sec); > di->i_mtime_nsec = di->i_ctime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec); > - ocfs2_update_inode_fsync_trans(handle, inode, 1); > + if (handle) > + ocfs2_update_inode_fsync_trans(handle, inode, 1); > } > if (handle) > ocfs2_journal_dirty(handle, wc->w_di_bh); >
Changwei Ge
2019-Aug-05 02:38 UTC
[Ocfs2-devel] [PATCH 2/3] fs: ocfs2: Fix a possible null-pointer dereference in ocfs2_write_end_nolock()
Hi Jia-ju, Could you please point out how ->w_handle can be NULL if we are changing disk inode? I just checked the ocfs2 code but can't find any clue ... In my opinion, it's impossible to change disk inode without an existed journal transaction. If truly so, it's a another problem. Thanks, Changwei On 2019/7/26 5:38 ??, Joseph Qi wrote:> > On 19/7/26 11:37, Jia-Ju Bai wrote: >> In ocfs2_write_end_nolock(), there are an if statement on lines 1976, >> 2047 and 2058, to check whether handle is NULL: >> if (handle) >> >> When handle is NULL, it is used on line 2045: >> ocfs2_update_inode_fsync_trans(handle, inode, 1); >> oi->i_sync_tid = handle->h_transaction->t_tid; >> >> Thus, a possible null-pointer dereference may occur. >> >> To fix this bug, handle is checked before calling >> ocfs2_update_inode_fsync_trans(). >> >> This bug is found by a static analysis tool STCheck written by us. >> >> Signed-off-by: Jia-Ju Bai <baijiaju1990 at gmail.com> > Looks good. > Reviewed-by: Joseph Qi <joseph.qi at linux.alibaba.com> > >> --- >> fs/ocfs2/aops.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c >> index a4c905d6b575..5473bd99043e 100644 >> --- a/fs/ocfs2/aops.c >> +++ b/fs/ocfs2/aops.c >> @@ -2042,7 +2042,8 @@ int ocfs2_write_end_nolock(struct address_space *mapping, >> inode->i_mtime = inode->i_ctime = current_time(inode); >> di->i_mtime = di->i_ctime = cpu_to_le64(inode->i_mtime.tv_sec); >> di->i_mtime_nsec = di->i_ctime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec); >> - ocfs2_update_inode_fsync_trans(handle, inode, 1); >> + if (handle) >> + ocfs2_update_inode_fsync_trans(handle, inode, 1); >> } >> if (handle) >> ocfs2_journal_dirty(handle, wc->w_di_bh); >> > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel at oss.oracle.com > https://oss.oracle.com/mailman/listinfo/ocfs2-devel