From: Dan Carpenter <dan.carpenter@linaro.org>
To: Henrique Carvalho <henrique.carvalho@suse.com>
Cc: linux-cifs@vger.kernel.org
Subject: [bug report] smb: client: batch SRV_COPYCHUNK entries to cut roundtrips
Date: Thu, 18 Sep 2025 12:44:20 +0300 [thread overview]
Message-ID: <aMvUdFU-ciu7Rgc6@stanley.mountain> (raw)
Hello Henrique Carvalho,
Commit 4e3acdf94c89 ("smb: client: batch SRV_COPYCHUNK entries to cut
roundtrips") from Sep 16, 2025 (linux-next), leads to the following
Smatch static checker warning:
fs/smb/client/smb2ops.c:1937 smb2_copychunk_range()
error: uninitialized symbol 'ret_data_len'.
fs/smb/client/smb2ops.c
1844 static ssize_t
1845 smb2_copychunk_range(const unsigned int xid,
1846 struct cifsFileInfo *src_file,
1847 struct cifsFileInfo *dst_file,
1848 u64 src_off,
1849 u64 len,
1850 u64 dst_off)
1851 {
1852 int rc = 0;
1853 unsigned int ret_data_len;
1854 struct copychunk_ioctl *cc_req = NULL;
1855 struct copychunk_ioctl_rsp *cc_rsp = NULL;
1856 struct cifs_tcon *tcon;
1857 struct copychunk *chunk;
1858 u32 chunks, chunk_count, chunk_bytes;
1859 u32 copy_bytes, copy_bytes_left;
1860 u32 chunks_written, bytes_written;
1861 u64 total_bytes_left = len;
1862 u64 src_off_prev, dst_off_prev;
1863 u32 retries = 0;
1864
1865 tcon = tlink_tcon(dst_file->tlink);
1866
1867 trace_smb3_copychunk_enter(xid, src_file->fid.volatile_fid,
1868 dst_file->fid.volatile_fid, tcon->tid,
1869 tcon->ses->Suid, src_off, dst_off, len);
1870
1871 retry:
1872 chunk_count = calc_chunk_count(tcon, total_bytes_left);
1873 if (!chunk_count) {
1874 rc = -EOPNOTSUPP;
1875 goto cchunk_out;
1876 }
1877
1878 cc_req = kzalloc(struct_size(cc_req, Chunks, chunk_count), GFP_KERNEL);
1879 if (!cc_req) {
1880 rc = -ENOMEM;
1881 goto cchunk_out;
1882 }
1883
1884 /* Request a key from the server to identify the source of the copy */
1885 rc = SMB2_request_res_key(xid,
1886 tlink_tcon(src_file->tlink),
1887 src_file->fid.persistent_fid,
1888 src_file->fid.volatile_fid,
1889 cc_req);
1890
1891 /* Note: request_res_key sets res_key null only if rc != 0 */
1892 if (rc)
1893 goto cchunk_out;
1894
1895 while (total_bytes_left > 0) {
1896
1897 /* Store previous offsets to allow rewind */
1898 src_off_prev = src_off;
1899 dst_off_prev = dst_off;
1900
1901 trace_smb3_copychunk_iter(xid, src_file->fid.volatile_fid,
1902 dst_file->fid.volatile_fid, tcon->tid,
1903 tcon->ses->Suid, src_off, dst_off, len);
1904
1905 chunks = 0;
1906 copy_bytes = 0;
1907 copy_bytes_left = umin(total_bytes_left, tcon->max_bytes_copy);
1908 while (copy_bytes_left > 0 && chunks < chunk_count) {
1909 chunk = &cc_req->Chunks[chunks++];
1910
1911 chunk->SourceOffset = cpu_to_le64(src_off);
1912 chunk->TargetOffset = cpu_to_le64(dst_off);
1913
1914 chunk_bytes = umin(copy_bytes_left, tcon->max_bytes_chunk);
1915
1916 chunk->Length = cpu_to_le32(chunk_bytes);
1917 chunk->Reserved = 0;
1918
1919 src_off += chunk_bytes;
1920 dst_off += chunk_bytes;
1921
1922 copy_bytes_left -= chunk_bytes;
1923 copy_bytes += chunk_bytes;
1924 }
1925
1926 cc_req->ChunkCount = cpu_to_le32(chunks);
1927 /* Buffer is zeroed, no need to set pcchunk->Reserved = 0 */
1928
1929 /* Request server copy to target from src identified by key */
1930 kfree(cc_rsp);
1931 cc_rsp = NULL;
1932 rc = SMB2_ioctl(xid, tcon, dst_file->fid.persistent_fid,
1933 dst_file->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
1934 (char *)cc_req, struct_size(cc_req, Chunks, chunks),
1935 CIFSMaxBufSize, (char **)&cc_rsp, &ret_data_len);
1936
--> 1937 if (ret_data_len != sizeof(struct copychunk_ioctl_rsp)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Using unintialized variables doesn't work because runtime checkers like
UBSan will complain as well as static checkers.
But the other issue is that this code treats -EINVAL as having a special
meaning. -EINVAL is the most common error code. All the places which
treat -EINVAL as having a special meaning end up buggy in the end. Mostly
they start out buggy, but even the ones which aren't buggy from the start
end up buggy over time.
1938 cifs_tcon_dbg(VFS, "Copychunk invalid response: response size does not match expected size\n");
1939 rc = -EIO;
1940 goto cchunk_out;
1941 }
1942
1943 if (rc == 0) {
1944 bytes_written = le32_to_cpu(cc_rsp->TotalBytesWritten);
1945 if (bytes_written == 0) {
1946 cifs_tcon_dbg(VFS, "Copychunk invalid response: no bytes copied\n");
1947 rc = -EIO;
1948 goto cchunk_out;
1949 }
1950 /* Check if server claimed to write more than we asked */
1951 if (bytes_written > copy_bytes) {
1952 cifs_tcon_dbg(VFS, "Copychunk invalid response: unexpected value for TotalBytesWritten\n");
1953 rc = -EIO;
1954 goto cchunk_out;
1955 }
1956
1957 chunks_written = le32_to_cpu(cc_rsp->ChunksWritten);
1958 if (chunks_written > chunks) {
1959 cifs_tcon_dbg(VFS, "Copychunk invalid response: Invalid num chunks written\n");
1960 rc = -EIO;
1961 goto cchunk_out;
1962 }
1963
1964 /* Partial write: rewind */
1965 if (bytes_written < copy_bytes) {
1966 u32 delta = copy_bytes - bytes_written;
1967
1968 src_off -= delta;
1969 dst_off -= delta;
1970 }
1971
1972 total_bytes_left -= bytes_written;
1973
1974 } else if (rc == -EINVAL) {
1975 /*
1976 * Check if server is not asking us to reduce size.
1977 *
1978 * Note: As per MS-SMB2 2.2.32.1, the values returned
1979 * in cc_rsp are not strictly lower than what existed
1980 * before.
1981 *
1982 */
1983 if (le32_to_cpu(cc_rsp->ChunksWritten) < tcon->max_chunks) {
1984 cifs_tcon_dbg(VFS, "Copychunk MaxChunks updated: %u -> %u\n",
1985 tcon->max_chunks,
1986 le32_to_cpu(cc_rsp->ChunksWritten));
1987 tcon->max_chunks = le32_to_cpu(cc_rsp->ChunksWritten);
1988 }
1989 if (le32_to_cpu(cc_rsp->ChunkBytesWritten) < tcon->max_bytes_chunk) {
1990 cifs_tcon_dbg(VFS, "Copychunk MaxBytesChunk updated: %u -> %u\n",
1991 tcon->max_bytes_chunk,
1992 le32_to_cpu(cc_rsp->ChunkBytesWritten));
1993 tcon->max_bytes_chunk = le32_to_cpu(cc_rsp->ChunkBytesWritten);
1994 }
1995 if (le32_to_cpu(cc_rsp->TotalBytesWritten) < tcon->max_bytes_copy) {
1996 cifs_tcon_dbg(VFS, "Copychunk MaxBytesCopy updated: %u -> %u\n",
1997 tcon->max_bytes_copy,
1998 le32_to_cpu(cc_rsp->TotalBytesWritten));
1999 tcon->max_bytes_copy = le32_to_cpu(cc_rsp->TotalBytesWritten);
2000 }
2001
2002 trace_smb3_copychunk_err(xid, src_file->fid.volatile_fid,
2003 dst_file->fid.volatile_fid, tcon->tid,
2004 tcon->ses->Suid, src_off, dst_off, len, rc);
2005
2006 /* Rewind */
2007 if (retries++ < 2) {
2008 src_off = src_off_prev;
2009 dst_off = dst_off_prev;
2010 kfree(cc_req);
2011 cc_req = NULL;
2012 goto retry;
2013 } else
2014 goto cchunk_out;
2015 } else { /* Unexpected */
2016 trace_smb3_copychunk_err(xid, src_file->fid.volatile_fid,
2017 dst_file->fid.volatile_fid, tcon->tid,
2018 tcon->ses->Suid, src_off, dst_off, len, rc);
2019 goto cchunk_out;
2020 }
2021 }
2022
2023 trace_smb3_copychunk_done(xid, src_file->fid.volatile_fid,
2024 dst_file->fid.volatile_fid, tcon->tid,
2025 tcon->ses->Suid, src_off, dst_off, len);
2026
2027 cchunk_out:
2028 kfree(cc_req);
2029 kfree(cc_rsp);
2030 if (rc)
2031 return rc;
2032 else
2033 return len;
2034 }
regards,
dan carpenter
next reply other threads:[~2025-09-18 9:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-18 9:44 Dan Carpenter [this message]
2025-09-18 20:19 ` [bug report] smb: client: batch SRV_COPYCHUNK entries to cut roundtrips Henrique Carvalho
2025-09-18 20:59 ` Henrique Carvalho
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=aMvUdFU-ciu7Rgc6@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=henrique.carvalho@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.