| Message ID | 20180409080646.23621-5-lsahlber@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | cifs: compounding series | expand |
2018-04-09 1:06 GMT-07:00 Ronnie Sahlberg <lsahlber@redhat.com>: > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/smb2ops.c | 10 ++++++---- > fs/cifs/smb2pdu.c | 9 +++++---- > fs/cifs/smb2proto.h | 2 +- > 3 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index def84fed3571..b4ae932ea134 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -1451,6 +1451,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, > __u8 oplock = SMB2_OPLOCK_LEVEL_NONE; > struct cifs_open_parms oparms; > struct cifs_fid fid; > + struct kvec err_iov = {NULL, 0}; > struct smb2_err_rsp *err_buf = NULL; > struct smb2_symlink_err_rsp *symlink; > unsigned int sub_len; > @@ -1473,15 +1474,16 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, > oparms.fid = &fid; > oparms.reconnect = false; > > - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_buf); > + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_iov); > > if (!rc || !err_buf) { > kfree(utf16_path); > return -ENOENT; > } > > + err_buf = err_iov.iov_base; > if (le32_to_cpu(err_buf->ByteCount) < sizeof(struct smb2_symlink_err_rsp) || > - get_rfc1002_length(err_buf) + server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE) { > + err_iov.iov_len + server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE) { > kfree(utf16_path); > return -ENOENT; > } > @@ -1494,13 +1496,13 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, > print_len = le16_to_cpu(symlink->PrintNameLength); > print_offset = le16_to_cpu(symlink->PrintNameOffset); > > - if (get_rfc1002_length(err_buf) + server->vals->header_preamble_size < > + if (err_iov.iov_len + server->vals->header_preamble_size < > SMB2_SYMLINK_STRUCT_SIZE + sub_offset + sub_len) { > kfree(utf16_path); > return -ENOENT; > } > > - if (get_rfc1002_length(err_buf) + server->vals->header_preamble_size < > + if (err_iov.iov_len + server->vals->header_preamble_size < > SMB2_SYMLINK_STRUCT_SIZE + print_offset + print_len) { > kfree(utf16_path); > return -ENOENT; > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 30dade197e7e..dc66b93131f3 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -1701,7 +1701,7 @@ alloc_path_with_tree_prefix(__le16 **out_path, int *out_size, int *out_len, > int > SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, > __u8 *oplock, struct smb2_file_all_info *buf, > - struct smb2_err_rsp **err_buf) > + struct kvec *err_iov) > { > struct smb2_create_req *req; > struct smb2_create_rsp *rsp; > @@ -1841,9 +1841,10 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, > > if (rc != 0) { > cifs_stats_fail_inc(tcon, SMB2_CREATE_HE); > - if (err_buf && rsp) > - *err_buf = kmemdup(rsp, get_rfc1002_length(rsp) + 4, > - GFP_KERNEL); > + if (err_iov && rsp) { > + *err_iov = rsp_iov; > + rsp = NULL; Should we also change a resp_buf_type here to CIFS_NO_BUFFER? This will prevent cifs_small_buf_release() to complain that a null pointer is passed. > + } > goto creat_exit; > } > > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h > index cbcce3f7e86f..8ba24a95db71 100644 > --- a/fs/cifs/smb2proto.h > +++ b/fs/cifs/smb2proto.h > @@ -122,7 +122,7 @@ extern int SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon); > extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, > __le16 *path, __u8 *oplock, > struct smb2_file_all_info *buf, > - struct smb2_err_rsp **err_buf); > + struct kvec *err_iov); > 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, > -- > 2.13.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Other than the notice above, the patch looks good. Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> -- Best regards, Pavel Shilovsky -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Pavel Shilovsky" <piastryyy@gmail.com> > To: "Ronnie Sahlberg" <lsahlber@redhat.com> > Cc: "linux-cifs" <linux-cifs@vger.kernel.org>, "Steve French" <smfrench@gmail.com> > Sent: Friday, 13 April, 2018 8:08:40 AM > Subject: Re: [PATCH 04/21] cifs: Change SMB2_open to return an iov for the error parameter > > 2018-04-09 1:06 GMT-07:00 Ronnie Sahlberg <lsahlber@redhat.com>: > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > > --- > > fs/cifs/smb2ops.c | 10 ++++++---- > > fs/cifs/smb2pdu.c | 9 +++++---- > > fs/cifs/smb2proto.h | 2 +- > > 3 files changed, 12 insertions(+), 9 deletions(-) > > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > > index def84fed3571..b4ae932ea134 100644 > > --- a/fs/cifs/smb2ops.c > > +++ b/fs/cifs/smb2ops.c > > @@ -1451,6 +1451,7 @@ smb2_query_symlink(const unsigned int xid, struct > > cifs_tcon *tcon, > > __u8 oplock = SMB2_OPLOCK_LEVEL_NONE; > > struct cifs_open_parms oparms; > > struct cifs_fid fid; > > + struct kvec err_iov = {NULL, 0}; > > struct smb2_err_rsp *err_buf = NULL; > > struct smb2_symlink_err_rsp *symlink; > > unsigned int sub_len; > > @@ -1473,15 +1474,16 @@ smb2_query_symlink(const unsigned int xid, struct > > cifs_tcon *tcon, > > oparms.fid = &fid; > > oparms.reconnect = false; > > > > - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_buf); > > + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_iov); > > > > if (!rc || !err_buf) { > > kfree(utf16_path); > > return -ENOENT; > > } > > > > + err_buf = err_iov.iov_base; > > if (le32_to_cpu(err_buf->ByteCount) < sizeof(struct > > smb2_symlink_err_rsp) || > > - get_rfc1002_length(err_buf) + > > server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE) { > > + err_iov.iov_len + server->vals->header_preamble_size < > > SMB2_SYMLINK_STRUCT_SIZE) { > > kfree(utf16_path); > > return -ENOENT; > > } > > @@ -1494,13 +1496,13 @@ smb2_query_symlink(const unsigned int xid, struct > > cifs_tcon *tcon, > > print_len = le16_to_cpu(symlink->PrintNameLength); > > print_offset = le16_to_cpu(symlink->PrintNameOffset); > > > > - if (get_rfc1002_length(err_buf) + > > server->vals->header_preamble_size < > > + if (err_iov.iov_len + server->vals->header_preamble_size < > > SMB2_SYMLINK_STRUCT_SIZE + sub_offset + sub_len) { > > kfree(utf16_path); > > return -ENOENT; > > } > > > > - if (get_rfc1002_length(err_buf) + > > server->vals->header_preamble_size < > > + if (err_iov.iov_len + server->vals->header_preamble_size < > > SMB2_SYMLINK_STRUCT_SIZE + print_offset + > > print_len) { > > kfree(utf16_path); > > return -ENOENT; > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > > index 30dade197e7e..dc66b93131f3 100644 > > --- a/fs/cifs/smb2pdu.c > > +++ b/fs/cifs/smb2pdu.c > > @@ -1701,7 +1701,7 @@ alloc_path_with_tree_prefix(__le16 **out_path, int > > *out_size, int *out_len, > > int > > SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 > > *path, > > __u8 *oplock, struct smb2_file_all_info *buf, > > - struct smb2_err_rsp **err_buf) > > + struct kvec *err_iov) > > { > > struct smb2_create_req *req; > > struct smb2_create_rsp *rsp; > > @@ -1841,9 +1841,10 @@ SMB2_open(const unsigned int xid, struct > > cifs_open_parms *oparms, __le16 *path, > > > > if (rc != 0) { > > cifs_stats_fail_inc(tcon, SMB2_CREATE_HE); > > - if (err_buf && rsp) > > - *err_buf = kmemdup(rsp, get_rfc1002_length(rsp) + > > 4, > > - GFP_KERNEL); > > + if (err_iov && rsp) { > > + *err_iov = rsp_iov; > > + rsp = NULL; > > Should we also change a resp_buf_type here to CIFS_NO_BUFFER? This > will prevent cifs_small_buf_release() to complain that a null pointer > is passed. Yes that is a good idea. Steve, can you fixup this patch when you merge, or do you want me to re-send it ? > > > + } > > goto creat_exit; > > } > > > > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h > > index cbcce3f7e86f..8ba24a95db71 100644 > > --- a/fs/cifs/smb2proto.h > > +++ b/fs/cifs/smb2proto.h > > @@ -122,7 +122,7 @@ extern int SMB2_tdis(const unsigned int xid, struct > > cifs_tcon *tcon); > > extern int SMB2_open(const unsigned int xid, struct cifs_open_parms > > *oparms, > > __le16 *path, __u8 *oplock, > > struct smb2_file_all_info *buf, > > - struct smb2_err_rsp **err_buf); > > + struct kvec *err_iov); > > 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, > > -- > > 2.13.3 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > Other than the notice above, the patch looks good. > > Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> > > -- > Best regards, > Pavel Shilovsky > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index def84fed3571..b4ae932ea134 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -1451,6 +1451,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, __u8 oplock = SMB2_OPLOCK_LEVEL_NONE; struct cifs_open_parms oparms; struct cifs_fid fid; + struct kvec err_iov = {NULL, 0}; struct smb2_err_rsp *err_buf = NULL; struct smb2_symlink_err_rsp *symlink; unsigned int sub_len; @@ -1473,15 +1474,16 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_buf); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_iov); if (!rc || !err_buf) { kfree(utf16_path); return -ENOENT; } + err_buf = err_iov.iov_base; if (le32_to_cpu(err_buf->ByteCount) < sizeof(struct smb2_symlink_err_rsp) || - get_rfc1002_length(err_buf) + server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE) { + err_iov.iov_len + server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE) { kfree(utf16_path); return -ENOENT; } @@ -1494,13 +1496,13 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, print_len = le16_to_cpu(symlink->PrintNameLength); print_offset = le16_to_cpu(symlink->PrintNameOffset); - if (get_rfc1002_length(err_buf) + server->vals->header_preamble_size < + if (err_iov.iov_len + server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE + sub_offset + sub_len) { kfree(utf16_path); return -ENOENT; } - if (get_rfc1002_length(err_buf) + server->vals->header_preamble_size < + if (err_iov.iov_len + server->vals->header_preamble_size < SMB2_SYMLINK_STRUCT_SIZE + print_offset + print_len) { kfree(utf16_path); return -ENOENT; diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 30dade197e7e..dc66b93131f3 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1701,7 +1701,7 @@ alloc_path_with_tree_prefix(__le16 **out_path, int *out_size, int *out_len, int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, __u8 *oplock, struct smb2_file_all_info *buf, - struct smb2_err_rsp **err_buf) + struct kvec *err_iov) { struct smb2_create_req *req; struct smb2_create_rsp *rsp; @@ -1841,9 +1841,10 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, if (rc != 0) { cifs_stats_fail_inc(tcon, SMB2_CREATE_HE); - if (err_buf && rsp) - *err_buf = kmemdup(rsp, get_rfc1002_length(rsp) + 4, - GFP_KERNEL); + if (err_iov && rsp) { + *err_iov = rsp_iov; + rsp = NULL; + } goto creat_exit; } diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index cbcce3f7e86f..8ba24a95db71 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -122,7 +122,7 @@ extern int SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon); extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, __u8 *oplock, struct smb2_file_all_info *buf, - struct smb2_err_rsp **err_buf); + struct kvec *err_iov); 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,
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/smb2ops.c | 10 ++++++---- fs/cifs/smb2pdu.c | 9 +++++---- fs/cifs/smb2proto.h | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-)