Dan Carpenter
2015-Sep-21 16:24 UTC
[Ocfs2-devel] ocfs2: do not change i_size in write_end for direct io
Hello Ryan Ding,
This is a semi-automatic email about new static checker warnings.
The patch 3d598f72dc44: "ocfs2: do not change i_size in write_end for
direct io" from Sep 17, 2015, leads to the following Smatch complaint:
fs/ocfs2/aops.c:2063 ocfs2_write_end_nolock()
error: we previously assumed 'handle' could be null (see line 1994)
fs/ocfs2/aops.c
1993
1994 if (handle) {
Patch adds check.
1995 ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode),
1996 wc->w_di_bh, OCFS2_JOURNAL_ACCESS_WRITE);
1997 if (ret) {
1998 copied = ret;
1999 mlog_errno(ret);
2000 goto out;
2001 }
2002 }
2003
2004 if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
2005 ocfs2_write_end_inline(inode, pos, len, &copied, di, wc);
2006 goto out_write_size;
2007 }
2008
2009 if (unlikely(copied < len) && wc->w_target_page) {
2010 if (!PageUptodate(wc->w_target_page))
2011 copied = 0;
2012
2013 ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
2014 start+len);
2015 }
2016 if (wc->w_target_page)
2017 flush_dcache_page(wc->w_target_page);
2018
2019 for(i = 0; i < wc->w_num_pages; i++) {
2020 tmppage = wc->w_pages[i];
2021
2022 /* This is the direct io target page. */
2023 if (tmppage == NULL)
2024 continue;
2025
2026 if (tmppage == wc->w_target_page) {
2027 from = wc->w_target_from;
2028 to = wc->w_target_to;
2029
2030 BUG_ON(from > PAGE_CACHE_SIZE ||
2031 to > PAGE_CACHE_SIZE ||
2032 to < from);
2033 } else {
2034 /*
2035 * Pages adjacent to the target (if any) imply
2036 * a hole-filling write in which case we want
2037 * to flush their entire range.
2038 */
2039 from = 0;
2040 to = PAGE_CACHE_SIZE;
2041 }
2042
2043 if (page_has_buffers(tmppage)) {
2044 if (handle && ocfs2_should_order_data(inode))
2045 ocfs2_jbd2_file_inode(handle, inode);
2046 block_commit_write(tmppage, from, to);
2047 }
2048 }
2049
2050 out_write_size:
2051 /* Direct io do not update i_size here. */
2052 if (wc->w_type != OCFS2_WRITE_DIRECT) {
2053 pos += copied;
2054 if (pos > i_size_read(inode)) {
2055 i_size_write(inode, pos);
2056 mark_inode_dirty(inode);
2057 }
2058 inode->i_blocks = ocfs2_inode_sector_count(inode);
2059 di->i_size = cpu_to_le64((u64)i_size_read(inode));
2060 inode->i_mtime = inode->i_ctime = CURRENT_TIME;
2061 di->i_mtime = di->i_ctime =
cpu_to_le64(inode->i_mtime.tv_sec);
2062 di->i_mtime_nsec = di->i_ctime_nsec =
cpu_to_le32(inode->i_mtime.tv_nsec);
2063 ocfs2_update_inode_fsync_trans(handle, inode, 1);
^^^^^^
Unchecked dereference inside function call.
2064 }
2065 if (handle)
regards,
dan carpenter
Ryan Ding
2015-Sep-23 02:14 UTC
[Ocfs2-devel] ocfs2: do not change i_size in write_end for direct io
HiDan, On 09/22/2015 12:24 AM, Dan Carpenter wrote:> Hello Ryan Ding, > > This is a semi-automatic email about new static checker warnings. > > The patch 3d598f72dc44: "ocfs2: do not change i_size in write_end for > direct io" from Sep 17, 2015, leads to the following Smatch complaint: > > fs/ocfs2/aops.c:2063 ocfs2_write_end_nolock() > error: we previously assumed 'handle' could be null (see line 1994) > > fs/ocfs2/aops.c > 1993 > 1994 if (handle) { > > Patch adds check. > > 1995 ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), > 1996 wc->w_di_bh, OCFS2_JOURNAL_ACCESS_WRITE); > 1997 if (ret) { > 1998 copied = ret; > 1999 mlog_errno(ret); > 2000 goto out; > 2001 } > 2002 } > 2003 > 2004 if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) { > 2005 ocfs2_write_end_inline(inode, pos, len, &copied, di, wc); > 2006 goto out_write_size; > 2007 } > 2008 > 2009 if (unlikely(copied < len) && wc->w_target_page) { > 2010 if (!PageUptodate(wc->w_target_page)) > 2011 copied = 0; > 2012 > 2013 ocfs2_zero_new_buffers(wc->w_target_page, start+copied, > 2014 start+len); > 2015 } > 2016 if (wc->w_target_page) > 2017 flush_dcache_page(wc->w_target_page); > 2018 > 2019 for(i = 0; i < wc->w_num_pages; i++) { > 2020 tmppage = wc->w_pages[i]; > 2021 > 2022 /* This is the direct io target page. */ > 2023 if (tmppage == NULL) > 2024 continue; > 2025 > 2026 if (tmppage == wc->w_target_page) { > 2027 from = wc->w_target_from; > 2028 to = wc->w_target_to; > 2029 > 2030 BUG_ON(from > PAGE_CACHE_SIZE || > 2031 to > PAGE_CACHE_SIZE || > 2032 to < from); > 2033 } else { > 2034 /* > 2035 * Pages adjacent to the target (if any) imply > 2036 * a hole-filling write in which case we want > 2037 * to flush their entire range. > 2038 */ > 2039 from = 0; > 2040 to = PAGE_CACHE_SIZE; > 2041 } > 2042 > 2043 if (page_has_buffers(tmppage)) { > 2044 if (handle && ocfs2_should_order_data(inode)) > 2045 ocfs2_jbd2_file_inode(handle, inode); > 2046 block_commit_write(tmppage, from, to); > 2047 } > 2048 } > 2049 > 2050 out_write_size: > 2051 /* Direct io do not update i_size here. */ > 2052 if (wc->w_type != OCFS2_WRITE_DIRECT) { > 2053 pos += copied; > 2054 if (pos > i_size_read(inode)) { > 2055 i_size_write(inode, pos); > 2056 mark_inode_dirty(inode); > 2057 } > 2058 inode->i_blocks = ocfs2_inode_sector_count(inode); > 2059 di->i_size = cpu_to_le64((u64)i_size_read(inode)); > 2060 inode->i_mtime = inode->i_ctime = CURRENT_TIME; > 2061 di->i_mtime = di->i_ctime = cpu_to_le64(inode->i_mtime.tv_sec); > 2062 di->i_mtime_nsec = di->i_ctime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec); > 2063 ocfs2_update_inode_fsync_trans(handle, inode, 1); > ^^^^^^ > Unchecked dereference inside function call.Previous check (wc->w_type != OCFS2_WRITE_DIRECT) will make sure 'handle' is not NULL. Since only direct io may have a NULL handle. Regards, Ryan> > 2064 } > 2065 if (handle) > > regards, > dan carpenter