* [bug report] iomap: write iomap validity checks
@ 2023-01-20 13:47 Dan Carpenter
2023-01-20 16:16 ` Matthew Wilcox
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2023-01-20 13:47 UTC (permalink / raw)
To: dchinner; +Cc: linux-fsdevel
Hello Dave Chinner,
The patch d7b64041164c: "iomap: write iomap validity checks" from Nov
29, 2022, leads to the following Smatch static checker warning:
fs/iomap/buffered-io.c:829 iomap_write_iter()
error: uninitialized symbol 'folio'.
fs/iomap/buffered-io.c
818 if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) {
819 status = -EFAULT;
820 break;
821 }
822
823 status = iomap_write_begin(iter, pos, bytes, &folio);
^^^^^^^
The iomap_write_begin() function can succeed without initializing
*foliop. It's next to the big comment.
824 if (unlikely(status))
825 break;
826 if (iter->iomap.flags & IOMAP_F_STALE)
827 break;
828
--> 829 page = folio_file_page(folio, pos >> PAGE_SHIFT);
^^^^^
830 if (mapping_writably_mapped(mapping))
831 flush_dcache_page(page);
832
833 copied = copy_page_from_iter_atomic(page, offset, bytes, i);
834
835 status = iomap_write_end(iter, pos, bytes, copied, folio);
836
837 if (unlikely(copied != status))
838 iov_iter_revert(i, copied - status);
839
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] iomap: write iomap validity checks
2023-01-20 13:47 [bug report] iomap: write iomap validity checks Dan Carpenter
@ 2023-01-20 16:16 ` Matthew Wilcox
2023-01-23 6:55 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Matthew Wilcox @ 2023-01-20 16:16 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dchinner, linux-fsdevel
On Fri, Jan 20, 2023 at 04:47:04PM +0300, Dan Carpenter wrote:
> Hello Dave Chinner,
>
> The patch d7b64041164c: "iomap: write iomap validity checks" from Nov
> 29, 2022, leads to the following Smatch static checker warning:
>
> fs/iomap/buffered-io.c:829 iomap_write_iter()
> error: uninitialized symbol 'folio'.
>
> fs/iomap/buffered-io.c
> 818 if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) {
> 819 status = -EFAULT;
> 820 break;
> 821 }
> 822
> 823 status = iomap_write_begin(iter, pos, bytes, &folio);
> ^^^^^^^
> The iomap_write_begin() function can succeed without initializing
> *foliop. It's next to the big comment.
Yes, but if it does, it sets IOMAP_F_STALE
> 824 if (unlikely(status))
> 825 break;
> 826 if (iter->iomap.flags & IOMAP_F_STALE)
> 827 break;
so it breaks out here. Maybe we should return an errno from
iomap_write_begin() to make life easier for the static checking tools?
> --> 829 page = folio_file_page(folio, pos >> PAGE_SHIFT);
> ^^^^^
>
> 830 if (mapping_writably_mapped(mapping))
> 831 flush_dcache_page(page);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] iomap: write iomap validity checks
2023-01-20 16:16 ` Matthew Wilcox
@ 2023-01-23 6:55 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2023-01-23 6:55 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: dchinner, linux-fsdevel
On Fri, Jan 20, 2023 at 04:16:55PM +0000, Matthew Wilcox wrote:
> On Fri, Jan 20, 2023 at 04:47:04PM +0300, Dan Carpenter wrote:
> > Hello Dave Chinner,
> >
> > The patch d7b64041164c: "iomap: write iomap validity checks" from Nov
> > 29, 2022, leads to the following Smatch static checker warning:
> >
> > fs/iomap/buffered-io.c:829 iomap_write_iter()
> > error: uninitialized symbol 'folio'.
> >
> > fs/iomap/buffered-io.c
> > 818 if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) {
> > 819 status = -EFAULT;
> > 820 break;
> > 821 }
> > 822
> > 823 status = iomap_write_begin(iter, pos, bytes, &folio);
> > ^^^^^^^
> > The iomap_write_begin() function can succeed without initializing
> > *foliop. It's next to the big comment.
>
> Yes, but if it does, it sets IOMAP_F_STALE
>
> > 824 if (unlikely(status))
> > 825 break;
> > 826 if (iter->iomap.flags & IOMAP_F_STALE)
> > 827 break;
>
> so it breaks out here. Maybe we should return an errno from
> iomap_write_begin() to make life easier for the static checking tools?
>
Thanks for taking a look at this. I say just leave it as-is.
I know how to modify Smatch to handle this correctly, but it's quite a
lot of typing involved.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-23 6:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20 13:47 [bug report] iomap: write iomap validity checks Dan Carpenter
2023-01-20 16:16 ` Matthew Wilcox
2023-01-23 6:55 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).