* [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
@ 2022-08-17 19:08 Enzo Matsumiya
2022-08-17 19:49 ` Steve French
2022-08-17 21:16 ` Tom Talpey
0 siblings, 2 replies; 7+ messages in thread
From: Enzo Matsumiya @ 2022-08-17 19:08 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore
SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
sense to have it at all.
Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
must fail the request if the request flags is zero anyway.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/smb2file.c | 1 -
fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
fs/cifs/smb2pdu.c | 20 +++++++++-----------
fs/cifs/smb2proto.h | 4 ++--
4 files changed, 24 insertions(+), 36 deletions(-)
diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index f5dcc4940b6d..9dfd2dd612c2 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
nr_ioctl_req.Reserved = 0;
rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
- true /* is_fsctl */,
(char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
CIFSMaxBufSize, NULL, NULL /* no return info */);
if (rc == -EOPNOTSUPP) {
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index f406af596887..c8ada000de7f 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
struct cifs_ses *ses = tcon->ses;
rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
- FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
+ FSCTL_QUERY_NETWORK_INTERFACE_INFO,
NULL /* no data input */, 0 /* no data input */,
CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
if (rc == -EOPNOTSUPP) {
@@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
struct resume_key_req *res_key;
rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
- FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
- NULL, 0 /* no input */, CIFSMaxBufSize,
- (char **)&res_key, &ret_data_len);
+ FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
+ CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
if (rc == -EOPNOTSUPP) {
pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
@@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
- qi.info_type, true, buffer, qi.output_buffer_length,
+ qi.info_type, buffer, qi.output_buffer_length,
CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE);
free_req1_func = SMB2_ioctl_free;
@@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
retbuf = NULL;
rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
- true /* is_fsctl */, (char *)pcchunk,
- sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
- (char **)&retbuf, &ret_data_len);
+ (char *)pcchunk, sizeof(struct copychunk_ioctl),
+ CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
if (rc == 0) {
if (ret_data_len !=
sizeof(struct copychunk_ioctl_rsp)) {
@@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
- true /* is_fctl */,
&setsparse, 1, CIFSMaxBufSize, NULL, NULL);
if (rc) {
tcon->broken_sparse_sup = true;
@@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
trgtfile->fid.volatile_fid,
FSCTL_DUPLICATE_EXTENTS_TO_FILE,
- true /* is_fsctl */,
(char *)&dup_ext_buf,
sizeof(struct duplicate_extents_to_file),
CIFSMaxBufSize, NULL,
@@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid,
FSCTL_SET_INTEGRITY_INFORMATION,
- true /* is_fsctl */,
(char *)&integr_info,
sizeof(struct fsctl_set_integrity_information_req),
CIFSMaxBufSize, NULL,
@@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid,
FSCTL_SRV_ENUMERATE_SNAPSHOTS,
- true /* is_fsctl */,
NULL, 0 /* no input data */, max_response_size,
(char **)&retbuf,
&ret_data_len);
@@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
do {
rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
FSCTL_DFS_GET_REFERRALS,
- true /* is_fsctl */,
(char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
(char **)&dfs_rsp, &dfs_rsp_size);
if (!is_retryable_error(rc))
@@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
rc = SMB2_ioctl_init(tcon, server,
&rqst[1], fid.persistent_fid,
- fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
- true /* is_fctl */, NULL, 0,
+ fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
CIFSMaxBufSize -
MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE);
@@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
rc = SMB2_ioctl_init(tcon, server,
&rqst[1], COMPOUND_FID,
- COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
- true /* is_fctl */, NULL, 0,
+ COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
CIFSMaxBufSize -
MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE);
@@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
- cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
+ cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
(char *)&fsctl_buf,
sizeof(struct file_zero_data_information),
0, NULL, NULL);
@@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
- true /* is_fctl */, (char *)&fsctl_buf,
+ (char *)&fsctl_buf,
sizeof(struct file_zero_data_information),
CIFSMaxBufSize, NULL, NULL);
free_xid(xid);
@@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
in_data.length = cpu_to_le64(len);
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid,
- FSCTL_QUERY_ALLOCATED_RANGES, true,
+ FSCTL_QUERY_ALLOCATED_RANGES,
(char *)&in_data, sizeof(in_data),
1024 * sizeof(struct file_allocated_range_buffer),
(char **)&out_data, &out_data_len);
@@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid,
- FSCTL_QUERY_ALLOCATED_RANGES, true,
+ FSCTL_QUERY_ALLOCATED_RANGES,
(char *)&in_data, sizeof(in_data),
sizeof(struct file_allocated_range_buffer),
(char **)&out_data, &out_data_len);
@@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid,
- FSCTL_QUERY_ALLOCATED_RANGES, true,
+ FSCTL_QUERY_ALLOCATED_RANGES,
(char *)&in_data, sizeof(in_data),
1024 * sizeof(struct file_allocated_range_buffer),
(char **)&out_data, &out_data_len);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 9b31ea946d45..918152fb8582 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
}
rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
- FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
+ FSCTL_VALIDATE_NEGOTIATE_INFO,
(char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
(char **)&pneg_rsp, &rsplen);
if (rc == -EOPNOTSUPP) {
@@ -3056,7 +3056,7 @@ int
SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
struct smb_rqst *rqst,
u64 persistent_fid, u64 volatile_fid, u32 opcode,
- bool is_fsctl, char *in_data, u32 indatalen,
+ char *in_data, u32 indatalen,
__u32 max_response_size)
{
struct smb2_ioctl_req *req;
@@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
req->hdr.CreditCharge =
cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
SMB2_MAX_BUFFER_SIZE));
- if (is_fsctl)
- req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
- else
- req->Flags = 0;
+ /* always an FSCTL (for now) */
+ req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
/* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
@@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
*/
int
SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
- u64 volatile_fid, u32 opcode, bool is_fsctl,
- char *in_data, u32 indatalen, u32 max_out_data_len,
- char **out_data, u32 *plen /* returned data len */)
+ u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
+ u32 max_out_data_len, char **out_data,
+ u32 *plen /* returned data len */)
{
struct smb_rqst rqst;
struct smb2_ioctl_rsp *rsp = NULL;
@@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
rc = SMB2_ioctl_init(tcon, server,
&rqst, persistent_fid, volatile_fid, opcode,
- is_fsctl, in_data, indatalen, max_out_data_len);
+ in_data, indatalen, max_out_data_len);
if (rc)
goto ioctl_exit;
@@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
- FSCTL_SET_COMPRESSION, true /* is_fsctl */,
+ FSCTL_SET_COMPRESSION,
(char *)&fsctl_input /* data input */,
2 /* in data len */, CIFSMaxBufSize /* max out data */,
&ret_data /* out data */, NULL);
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 51c5bf4a338a..7001317de9dc 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
extern void SMB2_open_free(struct smb_rqst *rqst);
extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
u64 persistent_fid, u64 volatile_fid, u32 opcode,
- bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
+ char *in_data, u32 indatalen, u32 maxoutlen,
char **out_data, u32 *plen /* returned data len */);
extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
struct TCP_Server_Info *server,
struct smb_rqst *rqst,
u64 persistent_fid, u64 volatile_fid, u32 opcode,
- bool is_fsctl, char *in_data, u32 indatalen,
+ char *in_data, u32 indatalen,
__u32 max_response_size);
extern void SMB2_ioctl_free(struct smb_rqst *rqst);
extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
--
2.35.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
2022-08-17 19:08 [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl() Enzo Matsumiya
@ 2022-08-17 19:49 ` Steve French
2022-08-17 19:59 ` Enzo Matsumiya
2022-08-17 21:16 ` Tom Talpey
1 sibling, 1 reply; 7+ messages in thread
From: Steve French @ 2022-08-17 19:49 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: CIFS, Paulo Alcantara, ronnie sahlberg, Shyam Prasad N
One alternative would be to allow the user space "pass through ioctl"
to set this flag to false (in case there were cases where a server did
support it and a test program or userspace utility needs to set it).
Do both Samba and ksmbd always reject it if isFsctl is false?
On Wed, Aug 17, 2022 at 2:08 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
> sense to have it at all.
>
> Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
>
> Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
> must fail the request if the request flags is zero anyway.
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/smb2file.c | 1 -
> fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
> fs/cifs/smb2pdu.c | 20 +++++++++-----------
> fs/cifs/smb2proto.h | 4 ++--
> 4 files changed, 24 insertions(+), 36 deletions(-)
>
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index f5dcc4940b6d..9dfd2dd612c2 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
> nr_ioctl_req.Reserved = 0;
> rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
> fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
> - true /* is_fsctl */,
> (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
> CIFSMaxBufSize, NULL, NULL /* no return info */);
> if (rc == -EOPNOTSUPP) {
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index f406af596887..c8ada000de7f 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
> struct cifs_ses *ses = tcon->ses;
>
> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> - FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
> + FSCTL_QUERY_NETWORK_INTERFACE_INFO,
> NULL /* no data input */, 0 /* no data input */,
> CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
> if (rc == -EOPNOTSUPP) {
> @@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
> struct resume_key_req *res_key;
>
> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> - FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
> - NULL, 0 /* no input */, CIFSMaxBufSize,
> - (char **)&res_key, &ret_data_len);
> + FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
> + CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
>
> if (rc == -EOPNOTSUPP) {
> pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
> @@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
>
> rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
> - qi.info_type, true, buffer, qi.output_buffer_length,
> + qi.info_type, buffer, qi.output_buffer_length,
> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> free_req1_func = SMB2_ioctl_free;
> @@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
> retbuf = NULL;
> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
> - true /* is_fsctl */, (char *)pcchunk,
> - sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
> - (char **)&retbuf, &ret_data_len);
> + (char *)pcchunk, sizeof(struct copychunk_ioctl),
> + CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
> if (rc == 0) {
> if (ret_data_len !=
> sizeof(struct copychunk_ioctl_rsp)) {
> @@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
> - true /* is_fctl */,
> &setsparse, 1, CIFSMaxBufSize, NULL, NULL);
> if (rc) {
> tcon->broken_sparse_sup = true;
> @@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> trgtfile->fid.volatile_fid,
> FSCTL_DUPLICATE_EXTENTS_TO_FILE,
> - true /* is_fsctl */,
> (char *)&dup_ext_buf,
> sizeof(struct duplicate_extents_to_file),
> CIFSMaxBufSize, NULL,
> @@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
> return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> FSCTL_SET_INTEGRITY_INFORMATION,
> - true /* is_fsctl */,
> (char *)&integr_info,
> sizeof(struct fsctl_set_integrity_information_req),
> CIFSMaxBufSize, NULL,
> @@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> FSCTL_SRV_ENUMERATE_SNAPSHOTS,
> - true /* is_fsctl */,
> NULL, 0 /* no input data */, max_response_size,
> (char **)&retbuf,
> &ret_data_len);
> @@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
> do {
> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> FSCTL_DFS_GET_REFERRALS,
> - true /* is_fsctl */,
> (char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
> (char **)&dfs_rsp, &dfs_rsp_size);
> if (!is_retryable_error(rc))
> @@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl_init(tcon, server,
> &rqst[1], fid.persistent_fid,
> - fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
> - true /* is_fctl */, NULL, 0,
> + fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
> CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> @@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl_init(tcon, server,
> &rqst[1], COMPOUND_FID,
> - COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
> - true /* is_fctl */, NULL, 0,
> + COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
> CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> @@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
> fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> - cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
> + cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> (char *)&fsctl_buf,
> sizeof(struct file_zero_data_information),
> 0, NULL, NULL);
> @@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> - true /* is_fctl */, (char *)&fsctl_buf,
> + (char *)&fsctl_buf,
> sizeof(struct file_zero_data_information),
> CIFSMaxBufSize, NULL, NULL);
> free_xid(xid);
> @@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
> in_data.length = cpu_to_le64(len);
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> + FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> 1024 * sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
> @@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> + FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
> @@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> + FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> 1024 * sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 9b31ea946d45..918152fb8582 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
> }
>
> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> - FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
> + FSCTL_VALIDATE_NEGOTIATE_INFO,
> (char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
> (char **)&pneg_rsp, &rsplen);
> if (rc == -EOPNOTSUPP) {
> @@ -3056,7 +3056,7 @@ int
> SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> struct smb_rqst *rqst,
> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> - bool is_fsctl, char *in_data, u32 indatalen,
> + char *in_data, u32 indatalen,
> __u32 max_response_size)
> {
> struct smb2_ioctl_req *req;
> @@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> req->hdr.CreditCharge =
> cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
> SMB2_MAX_BUFFER_SIZE));
> - if (is_fsctl)
> - req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
> - else
> - req->Flags = 0;
> + /* always an FSCTL (for now) */
> + req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
>
> /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> @@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
> */
> int
> SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> - u64 volatile_fid, u32 opcode, bool is_fsctl,
> - char *in_data, u32 indatalen, u32 max_out_data_len,
> - char **out_data, u32 *plen /* returned data len */)
> + u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
> + u32 max_out_data_len, char **out_data,
> + u32 *plen /* returned data len */)
> {
> struct smb_rqst rqst;
> struct smb2_ioctl_rsp *rsp = NULL;
> @@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>
> rc = SMB2_ioctl_init(tcon, server,
> &rqst, persistent_fid, volatile_fid, opcode,
> - is_fsctl, in_data, indatalen, max_out_data_len);
> + in_data, indatalen, max_out_data_len);
> if (rc)
> goto ioctl_exit;
>
> @@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
> cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
>
> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> - FSCTL_SET_COMPRESSION, true /* is_fsctl */,
> + FSCTL_SET_COMPRESSION,
> (char *)&fsctl_input /* data input */,
> 2 /* in data len */, CIFSMaxBufSize /* max out data */,
> &ret_data /* out data */, NULL);
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 51c5bf4a338a..7001317de9dc 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
> extern void SMB2_open_free(struct smb_rqst *rqst);
> extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> - bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
> + char *in_data, u32 indatalen, u32 maxoutlen,
> char **out_data, u32 *plen /* returned data len */);
> extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
> struct TCP_Server_Info *server,
> struct smb_rqst *rqst,
> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> - bool is_fsctl, char *in_data, u32 indatalen,
> + char *in_data, u32 indatalen,
> __u32 max_response_size);
> extern void SMB2_ioctl_free(struct smb_rqst *rqst);
> extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
> --
> 2.35.3
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
2022-08-17 19:49 ` Steve French
@ 2022-08-17 19:59 ` Enzo Matsumiya
2022-08-17 20:14 ` Steve French
0 siblings, 1 reply; 7+ messages in thread
From: Enzo Matsumiya @ 2022-08-17 19:59 UTC (permalink / raw)
To: Steve French; +Cc: CIFS, Paulo Alcantara, ronnie sahlberg, Shyam Prasad N
On 08/17, Steve French wrote:
>One alternative would be to allow the user space "pass through ioctl"
>to set this flag to false (in case there were cases where a server did
>support it and a test program or userspace utility needs to set it).
I don't really see the point of having so.
>Do both Samba and ksmbd always reject it if isFsctl is false?
Yes.
For reference, Samba in smbd_smb2_request_ioctl_done() (source3/smbd/smb2_ioctl.c):
7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
7601 goto out;
7602 }
and in ksmbd smb2_ioctl() (fs/ksmbd/smb2pdu.c):
7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
7601 goto out;
7602 }
Cheers,
Enzo
>On Wed, Aug 17, 2022 at 2:08 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
>> sense to have it at all.
>>
>> Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
>>
>> Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
>> must fail the request if the request flags is zero anyway.
>>
>> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
>> ---
>> fs/cifs/smb2file.c | 1 -
>> fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
>> fs/cifs/smb2pdu.c | 20 +++++++++-----------
>> fs/cifs/smb2proto.h | 4 ++--
>> 4 files changed, 24 insertions(+), 36 deletions(-)
>>
>> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
>> index f5dcc4940b6d..9dfd2dd612c2 100644
>> --- a/fs/cifs/smb2file.c
>> +++ b/fs/cifs/smb2file.c
>> @@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
>> nr_ioctl_req.Reserved = 0;
>> rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
>> fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
>> - true /* is_fsctl */,
>> (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
>> CIFSMaxBufSize, NULL, NULL /* no return info */);
>> if (rc == -EOPNOTSUPP) {
>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
>> index f406af596887..c8ada000de7f 100644
>> --- a/fs/cifs/smb2ops.c
>> +++ b/fs/cifs/smb2ops.c
>> @@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
>> struct cifs_ses *ses = tcon->ses;
>>
>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>> - FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
>> + FSCTL_QUERY_NETWORK_INTERFACE_INFO,
>> NULL /* no data input */, 0 /* no data input */,
>> CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
>> if (rc == -EOPNOTSUPP) {
>> @@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
>> struct resume_key_req *res_key;
>>
>> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>> - FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
>> - NULL, 0 /* no input */, CIFSMaxBufSize,
>> - (char **)&res_key, &ret_data_len);
>> + FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
>> + CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
>>
>> if (rc == -EOPNOTSUPP) {
>> pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
>> @@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
>> rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
>>
>> rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
>> - qi.info_type, true, buffer, qi.output_buffer_length,
>> + qi.info_type, buffer, qi.output_buffer_length,
>> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
>> free_req1_func = SMB2_ioctl_free;
>> @@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
>> retbuf = NULL;
>> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>> trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
>> - true /* is_fsctl */, (char *)pcchunk,
>> - sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
>> - (char **)&retbuf, &ret_data_len);
>> + (char *)pcchunk, sizeof(struct copychunk_ioctl),
>> + CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
>> if (rc == 0) {
>> if (ret_data_len !=
>> sizeof(struct copychunk_ioctl_rsp)) {
>> @@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
>>
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
>> - true /* is_fctl */,
>> &setsparse, 1, CIFSMaxBufSize, NULL, NULL);
>> if (rc) {
>> tcon->broken_sparse_sup = true;
>> @@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
>> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>> trgtfile->fid.volatile_fid,
>> FSCTL_DUPLICATE_EXTENTS_TO_FILE,
>> - true /* is_fsctl */,
>> (char *)&dup_ext_buf,
>> sizeof(struct duplicate_extents_to_file),
>> CIFSMaxBufSize, NULL,
>> @@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
>> return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid,
>> FSCTL_SET_INTEGRITY_INFORMATION,
>> - true /* is_fsctl */,
>> (char *)&integr_info,
>> sizeof(struct fsctl_set_integrity_information_req),
>> CIFSMaxBufSize, NULL,
>> @@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid,
>> FSCTL_SRV_ENUMERATE_SNAPSHOTS,
>> - true /* is_fsctl */,
>> NULL, 0 /* no input data */, max_response_size,
>> (char **)&retbuf,
>> &ret_data_len);
>> @@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>> do {
>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>> FSCTL_DFS_GET_REFERRALS,
>> - true /* is_fsctl */,
>> (char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
>> (char **)&dfs_rsp, &dfs_rsp_size);
>> if (!is_retryable_error(rc))
>> @@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>>
>> rc = SMB2_ioctl_init(tcon, server,
>> &rqst[1], fid.persistent_fid,
>> - fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
>> - true /* is_fctl */, NULL, 0,
>> + fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
>> CIFSMaxBufSize -
>> MAX_SMB2_CREATE_RESPONSE_SIZE -
>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
>> @@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
>>
>> rc = SMB2_ioctl_init(tcon, server,
>> &rqst[1], COMPOUND_FID,
>> - COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
>> - true /* is_fctl */, NULL, 0,
>> + COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
>> CIFSMaxBufSize -
>> MAX_SMB2_CREATE_RESPONSE_SIZE -
>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
>> @@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
>> fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
>>
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> - cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
>> + cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
>> (char *)&fsctl_buf,
>> sizeof(struct file_zero_data_information),
>> 0, NULL, NULL);
>> @@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
>>
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
>> - true /* is_fctl */, (char *)&fsctl_buf,
>> + (char *)&fsctl_buf,
>> sizeof(struct file_zero_data_information),
>> CIFSMaxBufSize, NULL, NULL);
>> free_xid(xid);
>> @@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
>> in_data.length = cpu_to_le64(len);
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid,
>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
>> + FSCTL_QUERY_ALLOCATED_RANGES,
>> (char *)&in_data, sizeof(in_data),
>> 1024 * sizeof(struct file_allocated_range_buffer),
>> (char **)&out_data, &out_data_len);
>> @@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
>>
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid,
>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
>> + FSCTL_QUERY_ALLOCATED_RANGES,
>> (char *)&in_data, sizeof(in_data),
>> sizeof(struct file_allocated_range_buffer),
>> (char **)&out_data, &out_data_len);
>> @@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
>>
>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>> cfile->fid.volatile_fid,
>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
>> + FSCTL_QUERY_ALLOCATED_RANGES,
>> (char *)&in_data, sizeof(in_data),
>> 1024 * sizeof(struct file_allocated_range_buffer),
>> (char **)&out_data, &out_data_len);
>> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
>> index 9b31ea946d45..918152fb8582 100644
>> --- a/fs/cifs/smb2pdu.c
>> +++ b/fs/cifs/smb2pdu.c
>> @@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
>> }
>>
>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>> - FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
>> + FSCTL_VALIDATE_NEGOTIATE_INFO,
>> (char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
>> (char **)&pneg_rsp, &rsplen);
>> if (rc == -EOPNOTSUPP) {
>> @@ -3056,7 +3056,7 @@ int
>> SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
>> struct smb_rqst *rqst,
>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
>> - bool is_fsctl, char *in_data, u32 indatalen,
>> + char *in_data, u32 indatalen,
>> __u32 max_response_size)
>> {
>> struct smb2_ioctl_req *req;
>> @@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
>> req->hdr.CreditCharge =
>> cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
>> SMB2_MAX_BUFFER_SIZE));
>> - if (is_fsctl)
>> - req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
>> - else
>> - req->Flags = 0;
>> + /* always an FSCTL (for now) */
>> + req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
>>
>> /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>> if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>> @@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
>> */
>> int
>> SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>> - u64 volatile_fid, u32 opcode, bool is_fsctl,
>> - char *in_data, u32 indatalen, u32 max_out_data_len,
>> - char **out_data, u32 *plen /* returned data len */)
>> + u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
>> + u32 max_out_data_len, char **out_data,
>> + u32 *plen /* returned data len */)
>> {
>> struct smb_rqst rqst;
>> struct smb2_ioctl_rsp *rsp = NULL;
>> @@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>>
>> rc = SMB2_ioctl_init(tcon, server,
>> &rqst, persistent_fid, volatile_fid, opcode,
>> - is_fsctl, in_data, indatalen, max_out_data_len);
>> + in_data, indatalen, max_out_data_len);
>> if (rc)
>> goto ioctl_exit;
>>
>> @@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
>> cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
>>
>> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>> - FSCTL_SET_COMPRESSION, true /* is_fsctl */,
>> + FSCTL_SET_COMPRESSION,
>> (char *)&fsctl_input /* data input */,
>> 2 /* in data len */, CIFSMaxBufSize /* max out data */,
>> &ret_data /* out data */, NULL);
>> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
>> index 51c5bf4a338a..7001317de9dc 100644
>> --- a/fs/cifs/smb2proto.h
>> +++ b/fs/cifs/smb2proto.h
>> @@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
>> extern void SMB2_open_free(struct smb_rqst *rqst);
>> extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
>> - bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
>> + char *in_data, u32 indatalen, u32 maxoutlen,
>> char **out_data, u32 *plen /* returned data len */);
>> extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
>> struct TCP_Server_Info *server,
>> struct smb_rqst *rqst,
>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
>> - bool is_fsctl, char *in_data, u32 indatalen,
>> + char *in_data, u32 indatalen,
>> __u32 max_response_size);
>> extern void SMB2_ioctl_free(struct smb_rqst *rqst);
>> extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
>> --
>> 2.35.3
>>
>
>
>--
>Thanks,
>
>Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
2022-08-17 19:59 ` Enzo Matsumiya
@ 2022-08-17 20:14 ` Steve French
2022-08-17 21:10 ` Tom Talpey
0 siblings, 1 reply; 7+ messages in thread
From: Steve French @ 2022-08-17 20:14 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: CIFS, Paulo Alcantara, ronnie sahlberg, Shyam Prasad N
Let's think about this one more, maybe try some experiments at the
upcoming plugfest with other servers.
There is a small possibility that there may be debug workloads that
supported this on some servers, and no hurry to remove this parm.
On Wed, Aug 17, 2022 at 2:59 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> On 08/17, Steve French wrote:
> >One alternative would be to allow the user space "pass through ioctl"
> >to set this flag to false (in case there were cases where a server did
> >support it and a test program or userspace utility needs to set it).
>
> I don't really see the point of having so.
>
> >Do both Samba and ksmbd always reject it if isFsctl is false?
>
> Yes.
>
> For reference, Samba in smbd_smb2_request_ioctl_done() (source3/smbd/smb2_ioctl.c):
>
> 7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
> 7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
> 7601 goto out;
> 7602 }
>
> and in ksmbd smb2_ioctl() (fs/ksmbd/smb2pdu.c):
>
> 7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
> 7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
> 7601 goto out;
> 7602 }
>
>
> Cheers,
>
> Enzo
>
> >On Wed, Aug 17, 2022 at 2:08 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
> >>
> >> SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
> >> sense to have it at all.
> >>
> >> Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
> >>
> >> Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
> >> must fail the request if the request flags is zero anyway.
> >>
> >> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> >> ---
> >> fs/cifs/smb2file.c | 1 -
> >> fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
> >> fs/cifs/smb2pdu.c | 20 +++++++++-----------
> >> fs/cifs/smb2proto.h | 4 ++--
> >> 4 files changed, 24 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> >> index f5dcc4940b6d..9dfd2dd612c2 100644
> >> --- a/fs/cifs/smb2file.c
> >> +++ b/fs/cifs/smb2file.c
> >> @@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
> >> nr_ioctl_req.Reserved = 0;
> >> rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
> >> fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
> >> - true /* is_fsctl */,
> >> (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
> >> CIFSMaxBufSize, NULL, NULL /* no return info */);
> >> if (rc == -EOPNOTSUPP) {
> >> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> >> index f406af596887..c8ada000de7f 100644
> >> --- a/fs/cifs/smb2ops.c
> >> +++ b/fs/cifs/smb2ops.c
> >> @@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
> >> struct cifs_ses *ses = tcon->ses;
> >>
> >> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> >> - FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
> >> + FSCTL_QUERY_NETWORK_INTERFACE_INFO,
> >> NULL /* no data input */, 0 /* no data input */,
> >> CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
> >> if (rc == -EOPNOTSUPP) {
> >> @@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
> >> struct resume_key_req *res_key;
> >>
> >> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> >> - FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
> >> - NULL, 0 /* no input */, CIFSMaxBufSize,
> >> - (char **)&res_key, &ret_data_len);
> >> + FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
> >> + CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
> >>
> >> if (rc == -EOPNOTSUPP) {
> >> pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
> >> @@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> >> rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
> >>
> >> rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
> >> - qi.info_type, true, buffer, qi.output_buffer_length,
> >> + qi.info_type, buffer, qi.output_buffer_length,
> >> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
> >> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> >> free_req1_func = SMB2_ioctl_free;
> >> @@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
> >> retbuf = NULL;
> >> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> >> trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
> >> - true /* is_fsctl */, (char *)pcchunk,
> >> - sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
> >> - (char **)&retbuf, &ret_data_len);
> >> + (char *)pcchunk, sizeof(struct copychunk_ioctl),
> >> + CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
> >> if (rc == 0) {
> >> if (ret_data_len !=
> >> sizeof(struct copychunk_ioctl_rsp)) {
> >> @@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
> >>
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
> >> - true /* is_fctl */,
> >> &setsparse, 1, CIFSMaxBufSize, NULL, NULL);
> >> if (rc) {
> >> tcon->broken_sparse_sup = true;
> >> @@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
> >> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> >> trgtfile->fid.volatile_fid,
> >> FSCTL_DUPLICATE_EXTENTS_TO_FILE,
> >> - true /* is_fsctl */,
> >> (char *)&dup_ext_buf,
> >> sizeof(struct duplicate_extents_to_file),
> >> CIFSMaxBufSize, NULL,
> >> @@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
> >> return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid,
> >> FSCTL_SET_INTEGRITY_INFORMATION,
> >> - true /* is_fsctl */,
> >> (char *)&integr_info,
> >> sizeof(struct fsctl_set_integrity_information_req),
> >> CIFSMaxBufSize, NULL,
> >> @@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid,
> >> FSCTL_SRV_ENUMERATE_SNAPSHOTS,
> >> - true /* is_fsctl */,
> >> NULL, 0 /* no input data */, max_response_size,
> >> (char **)&retbuf,
> >> &ret_data_len);
> >> @@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
> >> do {
> >> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> >> FSCTL_DFS_GET_REFERRALS,
> >> - true /* is_fsctl */,
> >> (char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
> >> (char **)&dfs_rsp, &dfs_rsp_size);
> >> if (!is_retryable_error(rc))
> >> @@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> >>
> >> rc = SMB2_ioctl_init(tcon, server,
> >> &rqst[1], fid.persistent_fid,
> >> - fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
> >> - true /* is_fctl */, NULL, 0,
> >> + fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
> >> CIFSMaxBufSize -
> >> MAX_SMB2_CREATE_RESPONSE_SIZE -
> >> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> >> @@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
> >>
> >> rc = SMB2_ioctl_init(tcon, server,
> >> &rqst[1], COMPOUND_FID,
> >> - COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
> >> - true /* is_fctl */, NULL, 0,
> >> + COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
> >> CIFSMaxBufSize -
> >> MAX_SMB2_CREATE_RESPONSE_SIZE -
> >> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> >> @@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
> >> fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
> >>
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> - cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
> >> + cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> >> (char *)&fsctl_buf,
> >> sizeof(struct file_zero_data_information),
> >> 0, NULL, NULL);
> >> @@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
> >>
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> >> - true /* is_fctl */, (char *)&fsctl_buf,
> >> + (char *)&fsctl_buf,
> >> sizeof(struct file_zero_data_information),
> >> CIFSMaxBufSize, NULL, NULL);
> >> free_xid(xid);
> >> @@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
> >> in_data.length = cpu_to_le64(len);
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid,
> >> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> >> + FSCTL_QUERY_ALLOCATED_RANGES,
> >> (char *)&in_data, sizeof(in_data),
> >> 1024 * sizeof(struct file_allocated_range_buffer),
> >> (char **)&out_data, &out_data_len);
> >> @@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
> >>
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid,
> >> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> >> + FSCTL_QUERY_ALLOCATED_RANGES,
> >> (char *)&in_data, sizeof(in_data),
> >> sizeof(struct file_allocated_range_buffer),
> >> (char **)&out_data, &out_data_len);
> >> @@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
> >>
> >> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >> cfile->fid.volatile_fid,
> >> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> >> + FSCTL_QUERY_ALLOCATED_RANGES,
> >> (char *)&in_data, sizeof(in_data),
> >> 1024 * sizeof(struct file_allocated_range_buffer),
> >> (char **)&out_data, &out_data_len);
> >> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> >> index 9b31ea946d45..918152fb8582 100644
> >> --- a/fs/cifs/smb2pdu.c
> >> +++ b/fs/cifs/smb2pdu.c
> >> @@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
> >> }
> >>
> >> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> >> - FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
> >> + FSCTL_VALIDATE_NEGOTIATE_INFO,
> >> (char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
> >> (char **)&pneg_rsp, &rsplen);
> >> if (rc == -EOPNOTSUPP) {
> >> @@ -3056,7 +3056,7 @@ int
> >> SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> >> struct smb_rqst *rqst,
> >> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> >> - bool is_fsctl, char *in_data, u32 indatalen,
> >> + char *in_data, u32 indatalen,
> >> __u32 max_response_size)
> >> {
> >> struct smb2_ioctl_req *req;
> >> @@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> >> req->hdr.CreditCharge =
> >> cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
> >> SMB2_MAX_BUFFER_SIZE));
> >> - if (is_fsctl)
> >> - req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
> >> - else
> >> - req->Flags = 0;
> >> + /* always an FSCTL (for now) */
> >> + req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
> >>
> >> /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> >> if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> >> @@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
> >> */
> >> int
> >> SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> >> - u64 volatile_fid, u32 opcode, bool is_fsctl,
> >> - char *in_data, u32 indatalen, u32 max_out_data_len,
> >> - char **out_data, u32 *plen /* returned data len */)
> >> + u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
> >> + u32 max_out_data_len, char **out_data,
> >> + u32 *plen /* returned data len */)
> >> {
> >> struct smb_rqst rqst;
> >> struct smb2_ioctl_rsp *rsp = NULL;
> >> @@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> >>
> >> rc = SMB2_ioctl_init(tcon, server,
> >> &rqst, persistent_fid, volatile_fid, opcode,
> >> - is_fsctl, in_data, indatalen, max_out_data_len);
> >> + in_data, indatalen, max_out_data_len);
> >> if (rc)
> >> goto ioctl_exit;
> >>
> >> @@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
> >> cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
> >>
> >> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> >> - FSCTL_SET_COMPRESSION, true /* is_fsctl */,
> >> + FSCTL_SET_COMPRESSION,
> >> (char *)&fsctl_input /* data input */,
> >> 2 /* in data len */, CIFSMaxBufSize /* max out data */,
> >> &ret_data /* out data */, NULL);
> >> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> >> index 51c5bf4a338a..7001317de9dc 100644
> >> --- a/fs/cifs/smb2proto.h
> >> +++ b/fs/cifs/smb2proto.h
> >> @@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
> >> extern void SMB2_open_free(struct smb_rqst *rqst);
> >> extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
> >> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> >> - bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
> >> + char *in_data, u32 indatalen, u32 maxoutlen,
> >> char **out_data, u32 *plen /* returned data len */);
> >> extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
> >> struct TCP_Server_Info *server,
> >> struct smb_rqst *rqst,
> >> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> >> - bool is_fsctl, char *in_data, u32 indatalen,
> >> + char *in_data, u32 indatalen,
> >> __u32 max_response_size);
> >> extern void SMB2_ioctl_free(struct smb_rqst *rqst);
> >> extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
> >> --
> >> 2.35.3
> >>
> >
> >
> >--
> >Thanks,
> >
> >Steve
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
2022-08-17 20:14 ` Steve French
@ 2022-08-17 21:10 ` Tom Talpey
2022-08-18 4:33 ` Steve French
0 siblings, 1 reply; 7+ messages in thread
From: Tom Talpey @ 2022-08-17 21:10 UTC (permalink / raw)
To: Steve French, Enzo Matsumiya
Cc: CIFS, Paulo Alcantara, ronnie sahlberg, Shyam Prasad N
Sending that bit violates the protocol. So if you are keeping
it in reserve, what protocol extension will use it? And, will
it be documented like MS-SMB2??
I'm with Enzo IOW.
Tom.
On 8/17/2022 4:14 PM, Steve French wrote:
> Let's think about this one more, maybe try some experiments at the
> upcoming plugfest with other servers.
>
> There is a small possibility that there may be debug workloads that
> supported this on some servers, and no hurry to remove this parm.
>
> On Wed, Aug 17, 2022 at 2:59 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> On 08/17, Steve French wrote:
>>> One alternative would be to allow the user space "pass through ioctl"
>>> to set this flag to false (in case there were cases where a server did
>>> support it and a test program or userspace utility needs to set it).
>>
>> I don't really see the point of having so.
>>
>>> Do both Samba and ksmbd always reject it if isFsctl is false?
>>
>> Yes.
>>
>> For reference, Samba in smbd_smb2_request_ioctl_done() (source3/smbd/smb2_ioctl.c):
>>
>> 7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
>> 7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
>> 7601 goto out;
>> 7602 }
>>
>> and in ksmbd smb2_ioctl() (fs/ksmbd/smb2pdu.c):
>>
>> 7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
>> 7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
>> 7601 goto out;
>> 7602 }
>>
>>
>> Cheers,
>>
>> Enzo
>>
>>> On Wed, Aug 17, 2022 at 2:08 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>>>
>>>> SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
>>>> sense to have it at all.
>>>>
>>>> Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
>>>>
>>>> Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
>>>> must fail the request if the request flags is zero anyway.
>>>>
>>>> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
>>>> ---
>>>> fs/cifs/smb2file.c | 1 -
>>>> fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
>>>> fs/cifs/smb2pdu.c | 20 +++++++++-----------
>>>> fs/cifs/smb2proto.h | 4 ++--
>>>> 4 files changed, 24 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
>>>> index f5dcc4940b6d..9dfd2dd612c2 100644
>>>> --- a/fs/cifs/smb2file.c
>>>> +++ b/fs/cifs/smb2file.c
>>>> @@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
>>>> nr_ioctl_req.Reserved = 0;
>>>> rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
>>>> fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
>>>> - true /* is_fsctl */,
>>>> (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
>>>> CIFSMaxBufSize, NULL, NULL /* no return info */);
>>>> if (rc == -EOPNOTSUPP) {
>>>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
>>>> index f406af596887..c8ada000de7f 100644
>>>> --- a/fs/cifs/smb2ops.c
>>>> +++ b/fs/cifs/smb2ops.c
>>>> @@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
>>>> struct cifs_ses *ses = tcon->ses;
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>>>> - FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
>>>> + FSCTL_QUERY_NETWORK_INTERFACE_INFO,
>>>> NULL /* no data input */, 0 /* no data input */,
>>>> CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
>>>> if (rc == -EOPNOTSUPP) {
>>>> @@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
>>>> struct resume_key_req *res_key;
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>>>> - FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
>>>> - NULL, 0 /* no input */, CIFSMaxBufSize,
>>>> - (char **)&res_key, &ret_data_len);
>>>> + FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
>>>> + CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
>>>>
>>>> if (rc == -EOPNOTSUPP) {
>>>> pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
>>>> @@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
>>>> rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
>>>>
>>>> rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
>>>> - qi.info_type, true, buffer, qi.output_buffer_length,
>>>> + qi.info_type, buffer, qi.output_buffer_length,
>>>> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
>>>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
>>>> free_req1_func = SMB2_ioctl_free;
>>>> @@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
>>>> retbuf = NULL;
>>>> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>>>> trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
>>>> - true /* is_fsctl */, (char *)pcchunk,
>>>> - sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
>>>> - (char **)&retbuf, &ret_data_len);
>>>> + (char *)pcchunk, sizeof(struct copychunk_ioctl),
>>>> + CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
>>>> if (rc == 0) {
>>>> if (ret_data_len !=
>>>> sizeof(struct copychunk_ioctl_rsp)) {
>>>> @@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
>>>> - true /* is_fctl */,
>>>> &setsparse, 1, CIFSMaxBufSize, NULL, NULL);
>>>> if (rc) {
>>>> tcon->broken_sparse_sup = true;
>>>> @@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
>>>> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>>>> trgtfile->fid.volatile_fid,
>>>> FSCTL_DUPLICATE_EXTENTS_TO_FILE,
>>>> - true /* is_fsctl */,
>>>> (char *)&dup_ext_buf,
>>>> sizeof(struct duplicate_extents_to_file),
>>>> CIFSMaxBufSize, NULL,
>>>> @@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
>>>> return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid,
>>>> FSCTL_SET_INTEGRITY_INFORMATION,
>>>> - true /* is_fsctl */,
>>>> (char *)&integr_info,
>>>> sizeof(struct fsctl_set_integrity_information_req),
>>>> CIFSMaxBufSize, NULL,
>>>> @@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid,
>>>> FSCTL_SRV_ENUMERATE_SNAPSHOTS,
>>>> - true /* is_fsctl */,
>>>> NULL, 0 /* no input data */, max_response_size,
>>>> (char **)&retbuf,
>>>> &ret_data_len);
>>>> @@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>>>> do {
>>>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>>>> FSCTL_DFS_GET_REFERRALS,
>>>> - true /* is_fsctl */,
>>>> (char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
>>>> (char **)&dfs_rsp, &dfs_rsp_size);
>>>> if (!is_retryable_error(rc))
>>>> @@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>>>>
>>>> rc = SMB2_ioctl_init(tcon, server,
>>>> &rqst[1], fid.persistent_fid,
>>>> - fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
>>>> - true /* is_fctl */, NULL, 0,
>>>> + fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
>>>> CIFSMaxBufSize -
>>>> MAX_SMB2_CREATE_RESPONSE_SIZE -
>>>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
>>>> @@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
>>>>
>>>> rc = SMB2_ioctl_init(tcon, server,
>>>> &rqst[1], COMPOUND_FID,
>>>> - COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
>>>> - true /* is_fctl */, NULL, 0,
>>>> + COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
>>>> CIFSMaxBufSize -
>>>> MAX_SMB2_CREATE_RESPONSE_SIZE -
>>>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
>>>> @@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
>>>> fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> - cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
>>>> + cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
>>>> (char *)&fsctl_buf,
>>>> sizeof(struct file_zero_data_information),
>>>> 0, NULL, NULL);
>>>> @@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
>>>> - true /* is_fctl */, (char *)&fsctl_buf,
>>>> + (char *)&fsctl_buf,
>>>> sizeof(struct file_zero_data_information),
>>>> CIFSMaxBufSize, NULL, NULL);
>>>> free_xid(xid);
>>>> @@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
>>>> in_data.length = cpu_to_le64(len);
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid,
>>>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
>>>> + FSCTL_QUERY_ALLOCATED_RANGES,
>>>> (char *)&in_data, sizeof(in_data),
>>>> 1024 * sizeof(struct file_allocated_range_buffer),
>>>> (char **)&out_data, &out_data_len);
>>>> @@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid,
>>>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
>>>> + FSCTL_QUERY_ALLOCATED_RANGES,
>>>> (char *)&in_data, sizeof(in_data),
>>>> sizeof(struct file_allocated_range_buffer),
>>>> (char **)&out_data, &out_data_len);
>>>> @@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>>>> cfile->fid.volatile_fid,
>>>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
>>>> + FSCTL_QUERY_ALLOCATED_RANGES,
>>>> (char *)&in_data, sizeof(in_data),
>>>> 1024 * sizeof(struct file_allocated_range_buffer),
>>>> (char **)&out_data, &out_data_len);
>>>> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
>>>> index 9b31ea946d45..918152fb8582 100644
>>>> --- a/fs/cifs/smb2pdu.c
>>>> +++ b/fs/cifs/smb2pdu.c
>>>> @@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
>>>> }
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>>>> - FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
>>>> + FSCTL_VALIDATE_NEGOTIATE_INFO,
>>>> (char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
>>>> (char **)&pneg_rsp, &rsplen);
>>>> if (rc == -EOPNOTSUPP) {
>>>> @@ -3056,7 +3056,7 @@ int
>>>> SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
>>>> struct smb_rqst *rqst,
>>>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
>>>> - bool is_fsctl, char *in_data, u32 indatalen,
>>>> + char *in_data, u32 indatalen,
>>>> __u32 max_response_size)
>>>> {
>>>> struct smb2_ioctl_req *req;
>>>> @@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
>>>> req->hdr.CreditCharge =
>>>> cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
>>>> SMB2_MAX_BUFFER_SIZE));
>>>> - if (is_fsctl)
>>>> - req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
>>>> - else
>>>> - req->Flags = 0;
>>>> + /* always an FSCTL (for now) */
>>>> + req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
>>>>
>>>> /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>> if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>> @@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
>>>> */
>>>> int
>>>> SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>>>> - u64 volatile_fid, u32 opcode, bool is_fsctl,
>>>> - char *in_data, u32 indatalen, u32 max_out_data_len,
>>>> - char **out_data, u32 *plen /* returned data len */)
>>>> + u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
>>>> + u32 max_out_data_len, char **out_data,
>>>> + u32 *plen /* returned data len */)
>>>> {
>>>> struct smb_rqst rqst;
>>>> struct smb2_ioctl_rsp *rsp = NULL;
>>>> @@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>>>>
>>>> rc = SMB2_ioctl_init(tcon, server,
>>>> &rqst, persistent_fid, volatile_fid, opcode,
>>>> - is_fsctl, in_data, indatalen, max_out_data_len);
>>>> + in_data, indatalen, max_out_data_len);
>>>> if (rc)
>>>> goto ioctl_exit;
>>>>
>>>> @@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
>>>> cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
>>>>
>>>> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>>>> - FSCTL_SET_COMPRESSION, true /* is_fsctl */,
>>>> + FSCTL_SET_COMPRESSION,
>>>> (char *)&fsctl_input /* data input */,
>>>> 2 /* in data len */, CIFSMaxBufSize /* max out data */,
>>>> &ret_data /* out data */, NULL);
>>>> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
>>>> index 51c5bf4a338a..7001317de9dc 100644
>>>> --- a/fs/cifs/smb2proto.h
>>>> +++ b/fs/cifs/smb2proto.h
>>>> @@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
>>>> extern void SMB2_open_free(struct smb_rqst *rqst);
>>>> extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
>>>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
>>>> - bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
>>>> + char *in_data, u32 indatalen, u32 maxoutlen,
>>>> char **out_data, u32 *plen /* returned data len */);
>>>> extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
>>>> struct TCP_Server_Info *server,
>>>> struct smb_rqst *rqst,
>>>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
>>>> - bool is_fsctl, char *in_data, u32 indatalen,
>>>> + char *in_data, u32 indatalen,
>>>> __u32 max_response_size);
>>>> extern void SMB2_ioctl_free(struct smb_rqst *rqst);
>>>> extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
>>>> --
>>>> 2.35.3
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>>
>>> Steve
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
2022-08-17 19:08 [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl() Enzo Matsumiya
2022-08-17 19:49 ` Steve French
@ 2022-08-17 21:16 ` Tom Talpey
1 sibling, 0 replies; 7+ messages in thread
From: Tom Talpey @ 2022-08-17 21:16 UTC (permalink / raw)
To: Enzo Matsumiya, linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore
It might be worth considering changing the name from SMB2_ioctl()
to be SMB2_fsctl(). Maybe for extra credit, even though there should
be no callers:
int SMB2_ioctl()
{
return -EOPNOTSUPP;
}
Reviewed-by: Tom Talpey <tom@talpey.com>
On 8/17/2022 3:08 PM, Enzo Matsumiya wrote:
> SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
> sense to have it at all.
>
> Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
>
> Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
> must fail the request if the request flags is zero anyway.
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/smb2file.c | 1 -
> fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
> fs/cifs/smb2pdu.c | 20 +++++++++-----------
> fs/cifs/smb2proto.h | 4 ++--
> 4 files changed, 24 insertions(+), 36 deletions(-)
>
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index f5dcc4940b6d..9dfd2dd612c2 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
> nr_ioctl_req.Reserved = 0;
> rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
> fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
> - true /* is_fsctl */,
> (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
> CIFSMaxBufSize, NULL, NULL /* no return info */);
> if (rc == -EOPNOTSUPP) {
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index f406af596887..c8ada000de7f 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
> struct cifs_ses *ses = tcon->ses;
>
> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> - FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
> + FSCTL_QUERY_NETWORK_INTERFACE_INFO,
> NULL /* no data input */, 0 /* no data input */,
> CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
> if (rc == -EOPNOTSUPP) {
> @@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
> struct resume_key_req *res_key;
>
> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> - FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
> - NULL, 0 /* no input */, CIFSMaxBufSize,
> - (char **)&res_key, &ret_data_len);
> + FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
> + CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
>
> if (rc == -EOPNOTSUPP) {
> pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
> @@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
>
> rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
> - qi.info_type, true, buffer, qi.output_buffer_length,
> + qi.info_type, buffer, qi.output_buffer_length,
> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> free_req1_func = SMB2_ioctl_free;
> @@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
> retbuf = NULL;
> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
> - true /* is_fsctl */, (char *)pcchunk,
> - sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
> - (char **)&retbuf, &ret_data_len);
> + (char *)pcchunk, sizeof(struct copychunk_ioctl),
> + CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
> if (rc == 0) {
> if (ret_data_len !=
> sizeof(struct copychunk_ioctl_rsp)) {
> @@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
> - true /* is_fctl */,
> &setsparse, 1, CIFSMaxBufSize, NULL, NULL);
> if (rc) {
> tcon->broken_sparse_sup = true;
> @@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> trgtfile->fid.volatile_fid,
> FSCTL_DUPLICATE_EXTENTS_TO_FILE,
> - true /* is_fsctl */,
> (char *)&dup_ext_buf,
> sizeof(struct duplicate_extents_to_file),
> CIFSMaxBufSize, NULL,
> @@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
> return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> FSCTL_SET_INTEGRITY_INFORMATION,
> - true /* is_fsctl */,
> (char *)&integr_info,
> sizeof(struct fsctl_set_integrity_information_req),
> CIFSMaxBufSize, NULL,
> @@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> FSCTL_SRV_ENUMERATE_SNAPSHOTS,
> - true /* is_fsctl */,
> NULL, 0 /* no input data */, max_response_size,
> (char **)&retbuf,
> &ret_data_len);
> @@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
> do {
> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> FSCTL_DFS_GET_REFERRALS,
> - true /* is_fsctl */,
> (char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
> (char **)&dfs_rsp, &dfs_rsp_size);
> if (!is_retryable_error(rc))
> @@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl_init(tcon, server,
> &rqst[1], fid.persistent_fid,
> - fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
> - true /* is_fctl */, NULL, 0,
> + fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
> CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> @@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl_init(tcon, server,
> &rqst[1], COMPOUND_FID,
> - COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
> - true /* is_fctl */, NULL, 0,
> + COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
> CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> @@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
> fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> - cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
> + cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> (char *)&fsctl_buf,
> sizeof(struct file_zero_data_information),
> 0, NULL, NULL);
> @@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> - true /* is_fctl */, (char *)&fsctl_buf,
> + (char *)&fsctl_buf,
> sizeof(struct file_zero_data_information),
> CIFSMaxBufSize, NULL, NULL);
> free_xid(xid);
> @@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
> in_data.length = cpu_to_le64(len);
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> + FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> 1024 * sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
> @@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> + FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
> @@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> + FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> 1024 * sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 9b31ea946d45..918152fb8582 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
> }
>
> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> - FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
> + FSCTL_VALIDATE_NEGOTIATE_INFO,
> (char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
> (char **)&pneg_rsp, &rsplen);
> if (rc == -EOPNOTSUPP) {
> @@ -3056,7 +3056,7 @@ int
> SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> struct smb_rqst *rqst,
> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> - bool is_fsctl, char *in_data, u32 indatalen,
> + char *in_data, u32 indatalen,
> __u32 max_response_size)
> {
> struct smb2_ioctl_req *req;
> @@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> req->hdr.CreditCharge =
> cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
> SMB2_MAX_BUFFER_SIZE));
> - if (is_fsctl)
> - req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
> - else
> - req->Flags = 0;
> + /* always an FSCTL (for now) */
> + req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
>
> /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> @@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
> */
> int
> SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> - u64 volatile_fid, u32 opcode, bool is_fsctl,
> - char *in_data, u32 indatalen, u32 max_out_data_len,
> - char **out_data, u32 *plen /* returned data len */)
> + u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
> + u32 max_out_data_len, char **out_data,
> + u32 *plen /* returned data len */)
> {
> struct smb_rqst rqst;
> struct smb2_ioctl_rsp *rsp = NULL;
> @@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>
> rc = SMB2_ioctl_init(tcon, server,
> &rqst, persistent_fid, volatile_fid, opcode,
> - is_fsctl, in_data, indatalen, max_out_data_len);
> + in_data, indatalen, max_out_data_len);
> if (rc)
> goto ioctl_exit;
>
> @@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
> cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
>
> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> - FSCTL_SET_COMPRESSION, true /* is_fsctl */,
> + FSCTL_SET_COMPRESSION,
> (char *)&fsctl_input /* data input */,
> 2 /* in data len */, CIFSMaxBufSize /* max out data */,
> &ret_data /* out data */, NULL);
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 51c5bf4a338a..7001317de9dc 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
> extern void SMB2_open_free(struct smb_rqst *rqst);
> extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> - bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
> + char *in_data, u32 indatalen, u32 maxoutlen,
> char **out_data, u32 *plen /* returned data len */);
> extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
> struct TCP_Server_Info *server,
> struct smb_rqst *rqst,
> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> - bool is_fsctl, char *in_data, u32 indatalen,
> + char *in_data, u32 indatalen,
> __u32 max_response_size);
> extern void SMB2_ioctl_free(struct smb_rqst *rqst);
> extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl()
2022-08-17 21:10 ` Tom Talpey
@ 2022-08-18 4:33 ` Steve French
0 siblings, 0 replies; 7+ messages in thread
From: Steve French @ 2022-08-18 4:33 UTC (permalink / raw)
To: Tom Talpey
Cc: Enzo Matsumiya, CIFS, Paulo Alcantara, ronnie sahlberg, Shyam Prasad N
tentatively merged into cifs-2.6.git for-next
On Wed, Aug 17, 2022 at 4:10 PM Tom Talpey <tom@talpey.com> wrote:
>
> Sending that bit violates the protocol. So if you are keeping
> it in reserve, what protocol extension will use it? And, will
> it be documented like MS-SMB2??
>
> I'm with Enzo IOW.
>
> Tom.
>
> On 8/17/2022 4:14 PM, Steve French wrote:
> > Let's think about this one more, maybe try some experiments at the
> > upcoming plugfest with other servers.
> >
> > There is a small possibility that there may be debug workloads that
> > supported this on some servers, and no hurry to remove this parm.
> >
> > On Wed, Aug 17, 2022 at 2:59 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
> >>
> >> On 08/17, Steve French wrote:
> >>> One alternative would be to allow the user space "pass through ioctl"
> >>> to set this flag to false (in case there were cases where a server did
> >>> support it and a test program or userspace utility needs to set it).
> >>
> >> I don't really see the point of having so.
> >>
> >>> Do both Samba and ksmbd always reject it if isFsctl is false?
> >>
> >> Yes.
> >>
> >> For reference, Samba in smbd_smb2_request_ioctl_done() (source3/smbd/smb2_ioctl.c):
> >>
> >> 7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
> >> 7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
> >> 7601 goto out;
> >> 7602 }
> >>
> >> and in ksmbd smb2_ioctl() (fs/ksmbd/smb2pdu.c):
> >>
> >> 7599 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
> >> 7600 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
> >> 7601 goto out;
> >> 7602 }
> >>
> >>
> >> Cheers,
> >>
> >> Enzo
> >>
> >>> On Wed, Aug 17, 2022 at 2:08 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
> >>>>
> >>>> SMB2_ioctl() is always called with is_fsctl = true, so doesn't make any
> >>>> sense to have it at all.
> >>>>
> >>>> Thus, always set SMB2_0_IOCTL_IS_FSCTL flag on the request.
> >>>>
> >>>> Also, as per MS-SMB2 3.3.5.15 "Receiving an SMB2 IOCTL Request", servers
> >>>> must fail the request if the request flags is zero anyway.
> >>>>
> >>>> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> >>>> ---
> >>>> fs/cifs/smb2file.c | 1 -
> >>>> fs/cifs/smb2ops.c | 35 +++++++++++++----------------------
> >>>> fs/cifs/smb2pdu.c | 20 +++++++++-----------
> >>>> fs/cifs/smb2proto.h | 4 ++--
> >>>> 4 files changed, 24 insertions(+), 36 deletions(-)
> >>>>
> >>>> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> >>>> index f5dcc4940b6d..9dfd2dd612c2 100644
> >>>> --- a/fs/cifs/smb2file.c
> >>>> +++ b/fs/cifs/smb2file.c
> >>>> @@ -61,7 +61,6 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
> >>>> nr_ioctl_req.Reserved = 0;
> >>>> rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
> >>>> fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
> >>>> - true /* is_fsctl */,
> >>>> (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
> >>>> CIFSMaxBufSize, NULL, NULL /* no return info */);
> >>>> if (rc == -EOPNOTSUPP) {
> >>>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> >>>> index f406af596887..c8ada000de7f 100644
> >>>> --- a/fs/cifs/smb2ops.c
> >>>> +++ b/fs/cifs/smb2ops.c
> >>>> @@ -681,7 +681,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
> >>>> struct cifs_ses *ses = tcon->ses;
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> >>>> - FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
> >>>> + FSCTL_QUERY_NETWORK_INTERFACE_INFO,
> >>>> NULL /* no data input */, 0 /* no data input */,
> >>>> CIFSMaxBufSize, (char **)&out_buf, &ret_data_len);
> >>>> if (rc == -EOPNOTSUPP) {
> >>>> @@ -1323,9 +1323,8 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
> >>>> struct resume_key_req *res_key;
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> >>>> - FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
> >>>> - NULL, 0 /* no input */, CIFSMaxBufSize,
> >>>> - (char **)&res_key, &ret_data_len);
> >>>> + FSCTL_SRV_REQUEST_RESUME_KEY, NULL, 0 /* no input */,
> >>>> + CIFSMaxBufSize, (char **)&res_key, &ret_data_len);
> >>>>
> >>>> if (rc == -EOPNOTSUPP) {
> >>>> pr_warn_once("Server share %s does not support copy range\n", tcon->treeName);
> >>>> @@ -1467,7 +1466,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> >>>> rqst[1].rq_nvec = SMB2_IOCTL_IOV_SIZE;
> >>>>
> >>>> rc = SMB2_ioctl_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
> >>>> - qi.info_type, true, buffer, qi.output_buffer_length,
> >>>> + qi.info_type, buffer, qi.output_buffer_length,
> >>>> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
> >>>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> >>>> free_req1_func = SMB2_ioctl_free;
> >>>> @@ -1643,9 +1642,8 @@ smb2_copychunk_range(const unsigned int xid,
> >>>> retbuf = NULL;
> >>>> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> >>>> trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
> >>>> - true /* is_fsctl */, (char *)pcchunk,
> >>>> - sizeof(struct copychunk_ioctl), CIFSMaxBufSize,
> >>>> - (char **)&retbuf, &ret_data_len);
> >>>> + (char *)pcchunk, sizeof(struct copychunk_ioctl),
> >>>> + CIFSMaxBufSize, (char **)&retbuf, &ret_data_len);
> >>>> if (rc == 0) {
> >>>> if (ret_data_len !=
> >>>> sizeof(struct copychunk_ioctl_rsp)) {
> >>>> @@ -1805,7 +1803,6 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
> >>>> - true /* is_fctl */,
> >>>> &setsparse, 1, CIFSMaxBufSize, NULL, NULL);
> >>>> if (rc) {
> >>>> tcon->broken_sparse_sup = true;
> >>>> @@ -1888,7 +1885,6 @@ smb2_duplicate_extents(const unsigned int xid,
> >>>> rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
> >>>> trgtfile->fid.volatile_fid,
> >>>> FSCTL_DUPLICATE_EXTENTS_TO_FILE,
> >>>> - true /* is_fsctl */,
> >>>> (char *)&dup_ext_buf,
> >>>> sizeof(struct duplicate_extents_to_file),
> >>>> CIFSMaxBufSize, NULL,
> >>>> @@ -1923,7 +1919,6 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
> >>>> return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid,
> >>>> FSCTL_SET_INTEGRITY_INFORMATION,
> >>>> - true /* is_fsctl */,
> >>>> (char *)&integr_info,
> >>>> sizeof(struct fsctl_set_integrity_information_req),
> >>>> CIFSMaxBufSize, NULL,
> >>>> @@ -1976,7 +1971,6 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid,
> >>>> FSCTL_SRV_ENUMERATE_SNAPSHOTS,
> >>>> - true /* is_fsctl */,
> >>>> NULL, 0 /* no input data */, max_response_size,
> >>>> (char **)&retbuf,
> >>>> &ret_data_len);
> >>>> @@ -2699,7 +2693,6 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
> >>>> do {
> >>>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> >>>> FSCTL_DFS_GET_REFERRALS,
> >>>> - true /* is_fsctl */,
> >>>> (char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
> >>>> (char **)&dfs_rsp, &dfs_rsp_size);
> >>>> if (!is_retryable_error(rc))
> >>>> @@ -2906,8 +2899,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> >>>>
> >>>> rc = SMB2_ioctl_init(tcon, server,
> >>>> &rqst[1], fid.persistent_fid,
> >>>> - fid.volatile_fid, FSCTL_GET_REPARSE_POINT,
> >>>> - true /* is_fctl */, NULL, 0,
> >>>> + fid.volatile_fid, FSCTL_GET_REPARSE_POINT, NULL, 0,
> >>>> CIFSMaxBufSize -
> >>>> MAX_SMB2_CREATE_RESPONSE_SIZE -
> >>>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> >>>> @@ -3087,8 +3079,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
> >>>>
> >>>> rc = SMB2_ioctl_init(tcon, server,
> >>>> &rqst[1], COMPOUND_FID,
> >>>> - COMPOUND_FID, FSCTL_GET_REPARSE_POINT,
> >>>> - true /* is_fctl */, NULL, 0,
> >>>> + COMPOUND_FID, FSCTL_GET_REPARSE_POINT, NULL, 0,
> >>>> CIFSMaxBufSize -
> >>>> MAX_SMB2_CREATE_RESPONSE_SIZE -
> >>>> MAX_SMB2_CLOSE_RESPONSE_SIZE);
> >>>> @@ -3358,7 +3349,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
> >>>> fsctl_buf.BeyondFinalZero = cpu_to_le64(offset + len);
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> - cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA, true,
> >>>> + cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> >>>> (char *)&fsctl_buf,
> >>>> sizeof(struct file_zero_data_information),
> >>>> 0, NULL, NULL);
> >>>> @@ -3421,7 +3412,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> >>>> - true /* is_fctl */, (char *)&fsctl_buf,
> >>>> + (char *)&fsctl_buf,
> >>>> sizeof(struct file_zero_data_information),
> >>>> CIFSMaxBufSize, NULL, NULL);
> >>>> free_xid(xid);
> >>>> @@ -3481,7 +3472,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,
> >>>> in_data.length = cpu_to_le64(len);
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid,
> >>>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> >>>> + FSCTL_QUERY_ALLOCATED_RANGES,
> >>>> (char *)&in_data, sizeof(in_data),
> >>>> 1024 * sizeof(struct file_allocated_range_buffer),
> >>>> (char **)&out_data, &out_data_len);
> >>>> @@ -3802,7 +3793,7 @@ static loff_t smb3_llseek(struct file *file, struct cifs_tcon *tcon, loff_t offs
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid,
> >>>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> >>>> + FSCTL_QUERY_ALLOCATED_RANGES,
> >>>> (char *)&in_data, sizeof(in_data),
> >>>> sizeof(struct file_allocated_range_buffer),
> >>>> (char **)&out_data, &out_data_len);
> >>>> @@ -3862,7 +3853,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> >>>> cfile->fid.volatile_fid,
> >>>> - FSCTL_QUERY_ALLOCATED_RANGES, true,
> >>>> + FSCTL_QUERY_ALLOCATED_RANGES,
> >>>> (char *)&in_data, sizeof(in_data),
> >>>> 1024 * sizeof(struct file_allocated_range_buffer),
> >>>> (char **)&out_data, &out_data_len);
> >>>> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> >>>> index 9b31ea946d45..918152fb8582 100644
> >>>> --- a/fs/cifs/smb2pdu.c
> >>>> +++ b/fs/cifs/smb2pdu.c
> >>>> @@ -1173,7 +1173,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
> >>>> }
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> >>>> - FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
> >>>> + FSCTL_VALIDATE_NEGOTIATE_INFO,
> >>>> (char *)pneg_inbuf, inbuflen, CIFSMaxBufSize,
> >>>> (char **)&pneg_rsp, &rsplen);
> >>>> if (rc == -EOPNOTSUPP) {
> >>>> @@ -3056,7 +3056,7 @@ int
> >>>> SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> >>>> struct smb_rqst *rqst,
> >>>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> >>>> - bool is_fsctl, char *in_data, u32 indatalen,
> >>>> + char *in_data, u32 indatalen,
> >>>> __u32 max_response_size)
> >>>> {
> >>>> struct smb2_ioctl_req *req;
> >>>> @@ -3131,10 +3131,8 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> >>>> req->hdr.CreditCharge =
> >>>> cpu_to_le16(DIV_ROUND_UP(max(indatalen, max_response_size),
> >>>> SMB2_MAX_BUFFER_SIZE));
> >>>> - if (is_fsctl)
> >>>> - req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
> >>>> - else
> >>>> - req->Flags = 0;
> >>>> + /* always an FSCTL (for now) */
> >>>> + req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
> >>>>
> >>>> /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> >>>> if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> >>>> @@ -3161,9 +3159,9 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
> >>>> */
> >>>> int
> >>>> SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> >>>> - u64 volatile_fid, u32 opcode, bool is_fsctl,
> >>>> - char *in_data, u32 indatalen, u32 max_out_data_len,
> >>>> - char **out_data, u32 *plen /* returned data len */)
> >>>> + u64 volatile_fid, u32 opcode, char *in_data, u32 indatalen,
> >>>> + u32 max_out_data_len, char **out_data,
> >>>> + u32 *plen /* returned data len */)
> >>>> {
> >>>> struct smb_rqst rqst;
> >>>> struct smb2_ioctl_rsp *rsp = NULL;
> >>>> @@ -3205,7 +3203,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> >>>>
> >>>> rc = SMB2_ioctl_init(tcon, server,
> >>>> &rqst, persistent_fid, volatile_fid, opcode,
> >>>> - is_fsctl, in_data, indatalen, max_out_data_len);
> >>>> + in_data, indatalen, max_out_data_len);
> >>>> if (rc)
> >>>> goto ioctl_exit;
> >>>>
> >>>> @@ -3297,7 +3295,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
> >>>> cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
> >>>>
> >>>> rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
> >>>> - FSCTL_SET_COMPRESSION, true /* is_fsctl */,
> >>>> + FSCTL_SET_COMPRESSION,
> >>>> (char *)&fsctl_input /* data input */,
> >>>> 2 /* in data len */, CIFSMaxBufSize /* max out data */,
> >>>> &ret_data /* out data */, NULL);
> >>>> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> >>>> index 51c5bf4a338a..7001317de9dc 100644
> >>>> --- a/fs/cifs/smb2proto.h
> >>>> +++ b/fs/cifs/smb2proto.h
> >>>> @@ -137,13 +137,13 @@ extern int SMB2_open_init(struct cifs_tcon *tcon,
> >>>> extern void SMB2_open_free(struct smb_rqst *rqst);
> >>>> extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
> >>>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> >>>> - bool is_fsctl, char *in_data, u32 indatalen, u32 maxoutlen,
> >>>> + char *in_data, u32 indatalen, u32 maxoutlen,
> >>>> char **out_data, u32 *plen /* returned data len */);
> >>>> extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
> >>>> struct TCP_Server_Info *server,
> >>>> struct smb_rqst *rqst,
> >>>> u64 persistent_fid, u64 volatile_fid, u32 opcode,
> >>>> - bool is_fsctl, char *in_data, u32 indatalen,
> >>>> + char *in_data, u32 indatalen,
> >>>> __u32 max_response_size);
> >>>> extern void SMB2_ioctl_free(struct smb_rqst *rqst);
> >>>> extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
> >>>> --
> >>>> 2.35.3
> >>>>
> >>>
> >>>
> >>> --
> >>> Thanks,
> >>>
> >>> Steve
> >
> >
> >
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-18 4:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-17 19:08 [PATCH] cifs: remove useless parameter 'is_fsctl' from SMB2_ioctl() Enzo Matsumiya
2022-08-17 19:49 ` Steve French
2022-08-17 19:59 ` Enzo Matsumiya
2022-08-17 20:14 ` Steve French
2022-08-17 21:10 ` Tom Talpey
2022-08-18 4:33 ` Steve French
2022-08-17 21:16 ` Tom Talpey
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.