[bug report] cifs: Fix writeback data corruption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux