From: Dan Carpenter <dan.carpenter@linaro.org>
To: dhowells@redhat.com
Cc: linux-cifs@vger.kernel.org
Subject: [bug report] cifs: Fix writeback data corruption
Date: Thu, 29 Feb 2024 13:52:02 +0300 [thread overview]
Message-ID: <862d95d6-5aa5-4542-a22c-2be58fd5c733@moroto.mountain> (raw)
Hello David Howells,
The patch 374ce0748c79: "cifs: Fix writeback data corruption" from
Feb 22, 2024 (linux-next), leads to the following Smatch static
checker warning:
fs/smb/client/file.c:2869 cifs_write_back_from_locked_folio()
error: uninitialized symbol 'len'.
fs/smb/client/file.c
2741 static ssize_t cifs_write_back_from_locked_folio(struct address_space *mapping,
2742 struct writeback_control *wbc,
2743 struct xa_state *xas,
2744 struct folio *folio,
2745 unsigned long long start,
2746 unsigned long long end)
2747 {
2748 struct inode *inode = mapping->host;
2749 struct TCP_Server_Info *server;
2750 struct cifs_writedata *wdata;
2751 struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
2752 struct cifs_credits credits_on_stack;
2753 struct cifs_credits *credits = &credits_on_stack;
2754 struct cifsFileInfo *cfile = NULL;
2755 unsigned long long i_size = i_size_read(inode), max_len;
2756 unsigned int xid, wsize;
2757 size_t len;
2758 long count = wbc->nr_to_write;
2759 int rc;
2760
2761 /* The folio should be locked, dirty and not undergoing writeback. */
2762 if (!folio_clear_dirty_for_io(folio))
2763 BUG();
2764 folio_start_writeback(folio);
2765
2766 count -= folio_nr_pages(folio);
2767
2768 xid = get_xid();
2769 server = cifs_pick_channel(cifs_sb_master_tcon(cifs_sb)->ses);
2770
2771 rc = cifs_get_writable_file(CIFS_I(inode), FIND_WR_ANY, &cfile);
2772 if (rc) {
2773 cifs_dbg(VFS, "No writable handle in writepages rc=%d\n", rc);
2774 goto err_xid;
^^^^^^^^^^^^^
len isn't initialized until later
2775 }
2776
2777 rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->wsize,
2778 &wsize, credits);
2779 if (rc != 0)
2780 goto err_close;
same
2781
2782 wdata = cifs_writedata_alloc(cifs_writev_complete);
2783 if (!wdata) {
2784 rc = -ENOMEM;
2785 goto err_uncredit;
same
2786 }
2787
2788 wdata->sync_mode = wbc->sync_mode;
2789 wdata->offset = folio_pos(folio);
2790 wdata->pid = cfile->pid;
2791 wdata->credits = credits_on_stack;
2792 wdata->cfile = cfile;
2793 wdata->server = server;
2794 cfile = NULL;
2795
2796 /* Find all consecutive lockable dirty pages that have contiguous
2797 * written regions, stopping when we find a page that is not
2798 * immediately lockable, is not dirty or is missing, or we reach the
2799 * end of the range.
2800 */
2801 len = folio_size(folio);
2802 if (start < i_size) {
2803 /* Trim the write to the EOF; the extra data is ignored. Also
2804 * put an upper limit on the size of a single storedata op.
2805 */
2806 max_len = wsize;
2807 max_len = min_t(unsigned long long, max_len, end - start + 1);
2808 max_len = min_t(unsigned long long, max_len, i_size - start);
2809
2810 if (len < max_len) {
2811 int max_pages = INT_MAX;
2812
2813 #ifdef CONFIG_CIFS_SMB_DIRECT
2814 if (server->smbd_conn)
2815 max_pages = server->smbd_conn->max_frmr_depth;
2816 #endif
2817 max_pages -= folio_nr_pages(folio);
2818
2819 if (max_pages > 0)
2820 cifs_extend_writeback(mapping, xas, &count, start,
2821 max_pages, max_len, &len);
2822 }
2823 }
2824 len = min_t(unsigned long long, len, i_size - start);
2825
2826 /* We now have a contiguous set of dirty pages, each with writeback
2827 * set; the first page is still locked at this point, but all the rest
2828 * have been unlocked.
2829 */
2830 folio_unlock(folio);
2831 wdata->bytes = len;
2832
2833 if (start < i_size) {
2834 iov_iter_xarray(&wdata->iter, ITER_SOURCE, &mapping->i_pages,
2835 start, len);
2836
2837 rc = adjust_credits(wdata->server, &wdata->credits, wdata->bytes);
2838 if (rc)
2839 goto err_wdata;
2840
2841 if (wdata->cfile->invalidHandle)
2842 rc = -EAGAIN;
2843 else
2844 rc = wdata->server->ops->async_writev(wdata,
2845 cifs_writedata_release);
2846 if (rc >= 0) {
2847 kref_put(&wdata->refcount, cifs_writedata_release);
2848 goto err_close;
2849 }
2850 } else {
2851 /* The dirty region was entirely beyond the EOF. */
2852 cifs_pages_written_back(inode, start, len);
2853 rc = 0;
2854 }
2855
2856 err_wdata:
2857 kref_put(&wdata->refcount, cifs_writedata_release);
2858 err_uncredit:
2859 add_credits_and_wake_if(server, credits, 0);
2860 err_close:
2861 if (cfile)
2862 cifsFileInfo_put(cfile);
2863 err_xid:
2864 free_xid(xid);
2865 if (rc == 0) {
2866 wbc->nr_to_write = count;
2867 rc = len;
2868 } else if (is_retryable_error(rc)) {
--> 2869 cifs_pages_write_redirty(inode, start, len);
^^^
2870 } else {
2871 cifs_pages_write_failed(inode, start, len);
^^^
Uninitialized
2872 mapping_set_error(mapping, rc);
2873 }
2874 /* Indication to update ctime and mtime as close is deferred */
2875 set_bit(CIFS_INO_MODIFIED_ATTR, &CIFS_I(inode)->flags);
2876 return rc;
2877 }
regards,
dan carpenter
next reply other threads:[~2024-02-29 10:52 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-29 10:52 Dan Carpenter [this message]
2024-03-01 17:25 ` [bug report] cifs: Fix writeback data corruption David Howells
2024-03-06 16:06 ` Steve French
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=862d95d6-5aa5-4542-a22c-2be58fd5c733@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=dhowells@redhat.com \
--cc=linux-cifs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).