[PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[Namjae Jeon]

Steve reported that inactive sessions are terminated after a few
seconds. ksmbd terminate when receiving -EAGAIN error from
kernel_recvmsg(). -EAGAIN means there is no data available in timeout.
So ksmbd should keep connection with unlimited retries instead of
terminating inactive sessions.

Reported-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/connection.c     |  4 ++--
 fs/ksmbd/connection.h     |  3 ++-
 fs/ksmbd/transport_rdma.c |  2 +-
 fs/ksmbd/transport_tcp.c  | 35 +++++++++++++++++++++++------------
 4 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index 5b10b03800c1..5d914715605f 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -298,7 +298,7 @@ int ksmbd_conn_handler_loop(void *p)
 		kvfree(conn->request_buf);
 		conn->request_buf = NULL;
 
-		size = t->ops->read(t, hdr_buf, sizeof(hdr_buf));
+		size = t->ops->read(t, hdr_buf, sizeof(hdr_buf), -1);
 		if (size != sizeof(hdr_buf))
 			break;
 
@@ -344,7 +344,7 @@ int ksmbd_conn_handler_loop(void *p)
 		 * We already read 4 bytes to find out PDU size, now
 		 * read in PDU
 		 */
-		size = t->ops->read(t, conn->request_buf + 4, pdu_size);
+		size = t->ops->read(t, conn->request_buf + 4, pdu_size, 2);
 		if (size < 0) {
 			pr_err("sock_read failed: %d\n", size);
 			break;
diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
index 3643354a3fa7..0e3a848defaf 100644
--- a/fs/ksmbd/connection.h
+++ b/fs/ksmbd/connection.h
@@ -114,7 +114,8 @@ struct ksmbd_transport_ops {
 	int (*prepare)(struct ksmbd_transport *t);
 	void (*disconnect)(struct ksmbd_transport *t);
 	void (*shutdown)(struct ksmbd_transport *t);
-	int (*read)(struct ksmbd_transport *t, char *buf, unsigned int size);
+	int (*read)(struct ksmbd_transport *t, char *buf,
+		    unsigned int size, int max_retries);
 	int (*writev)(struct ksmbd_transport *t, struct kvec *iovs, int niov,
 		      int size, bool need_invalidate_rkey,
 		      unsigned int remote_key);
diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c
index 096eda9ef873..c06efc020bd9 100644
--- a/fs/ksmbd/transport_rdma.c
+++ b/fs/ksmbd/transport_rdma.c
@@ -670,7 +670,7 @@ static int smb_direct_post_recv(struct smb_direct_transport *t,
 }
 
 static int smb_direct_read(struct ksmbd_transport *t, char *buf,
-			   unsigned int size)
+			   unsigned int size, int unused)
 {
 	struct smb_direct_recvmsg *recvmsg;
 	struct smb_direct_data_transfer *data_transfer;
diff --git a/fs/ksmbd/transport_tcp.c b/fs/ksmbd/transport_tcp.c
index 603893fd87f5..20e85e2701f2 100644
--- a/fs/ksmbd/transport_tcp.c
+++ b/fs/ksmbd/transport_tcp.c
@@ -291,16 +291,18 @@ static int ksmbd_tcp_run_kthread(struct interface *iface)
 
 /**
  * ksmbd_tcp_readv() - read data from socket in given iovec
- * @t:		TCP transport instance
- * @iov_orig:	base IO vector
- * @nr_segs:	number of segments in base iov
- * @to_read:	number of bytes to read from socket
+ * @t:			TCP transport instance
+ * @iov_orig:		base IO vector
+ * @nr_segs:		number of segments in base iov
+ * @to_read:		number of bytes to read from socket
+ * @max_retries:	maximum retry count
  *
  * Return:	on success return number of bytes read from socket,
  *		otherwise return error number
  */
 static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
-			   unsigned int nr_segs, unsigned int to_read)
+			   unsigned int nr_segs, unsigned int to_read,
+			   int max_retries)
 {
 	int length = 0;
 	int total_read;
@@ -308,7 +310,6 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
 	struct msghdr ksmbd_msg;
 	struct kvec *iov;
 	struct ksmbd_conn *conn = KSMBD_TRANS(t)->conn;
-	int max_retry = 2;
 
 	iov = get_conn_iovec(t, nr_segs);
 	if (!iov)
@@ -335,14 +336,23 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
 		} else if (conn->status == KSMBD_SESS_NEED_RECONNECT) {
 			total_read = -EAGAIN;
 			break;
-		} else if ((length == -ERESTARTSYS || length == -EAGAIN) &&
-			   max_retry) {
+		} else if (length == -ERESTARTSYS || length == -EAGAIN) {
+			/*
+			 * If max_retries is negative, Allow unlimited
+			 * retries to keep connection with inactive sessions.
+			 */
+			if (max_retries == 0) {
+				total_read = length;
+				break;
+			} else if (max_retries > 0) {
+				max_retries--;
+			}
+
 			usleep_range(1000, 2000);
 			length = 0;
-			max_retry--;
 			continue;
 		} else if (length <= 0) {
-			total_read = -EAGAIN;
+			total_read = length;
 			break;
 		}
 	}
@@ -358,14 +368,15 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
  * Return:	on success return number of bytes read from socket,
  *		otherwise return error number
  */
-static int ksmbd_tcp_read(struct ksmbd_transport *t, char *buf, unsigned int to_read)
+static int ksmbd_tcp_read(struct ksmbd_transport *t, char *buf,
+			  unsigned int to_read, int max_retries)
 {
 	struct kvec iov;
 
 	iov.iov_base = buf;
 	iov.iov_len = to_read;
 
-	return ksmbd_tcp_readv(TCP_TRANS(t), &iov, 1, to_read);
+	return ksmbd_tcp_readv(TCP_TRANS(t), &iov, 1, to_read, max_retries);
 }
 
 static int ksmbd_tcp_writev(struct ksmbd_transport *t, struct kvec *iov,
-- 
2.25.1

[PATCH] ksmbd: return STATUS_NOT_SUPPORTED on unsupported smb2.0 dialect

[Namjae Jeon]

ksmbd returned "Input/output error" when mounting with vers=2.0 to
ksmbd. It should return STATUS_NOT_SUPPORTED on unsupported smb2.0
dialect.

Reported-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
index fa2b54df6ee6..079c9e76818d 100644
--- a/fs/ksmbd/smb_common.c
+++ b/fs/ksmbd/smb_common.c
@@ -434,7 +434,7 @@ int ksmbd_extract_shortname(struct ksmbd_conn *conn, const char *longname,
 
 static int __smb2_negotiate(struct ksmbd_conn *conn)
 {
-	return (conn->dialect >= SMB21_PROT_ID &&
+	return (conn->dialect >= SMB20_PROT_ID &&
 		conn->dialect <= SMB311_PROT_ID);
 }
 
@@ -465,7 +465,7 @@ int ksmbd_smb_negotiate_common(struct ksmbd_work *work, unsigned int command)
 		}
 	}
 
-	if (command == SMB2_NEGOTIATE_HE && __smb2_negotiate(conn)) {
+	if (command == SMB2_NEGOTIATE_HE) {
 		ret = smb2_handle_negotiate(work);
 		init_smb2_neg_rsp(work);
 		return ret;
-- 
2.25.1

[PATCH] ksmbd: return unsupported error on smb1 mount

[Namjae Jeon]

ksmbd disconnect connection when mounting with vers=smb1.
ksmbd should send smb1 negotiate response to client for correct
unsupported error return. This patch add needed SMB1 macros and fill
NegProt part of the response for smb1 negotiate response.

Reported-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/connection.c |  7 ++-----
 fs/ksmbd/smb_common.c | 23 ++++++++++++++++++++---
 fs/ksmbd/smb_common.h | 30 ++++++++----------------------
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index 5d914715605f..115a67d2cf78 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -319,13 +319,10 @@ int ksmbd_conn_handler_loop(void *p)
 		}
 
 		/*
-		 * Check if pdu size is valid (min : smb header size,
-		 * max : 0x00FFFFFF).
+		 * Check maximum pdu size(0x00FFFFFF).
 		 */
-		if (pdu_size < __SMB2_HEADER_STRUCTURE_SIZE ||
-		    pdu_size > MAX_STREAM_PROT_LEN) {
+		if (pdu_size > MAX_STREAM_PROT_LEN)
 			break;
-		}
 
 		/* 4 for rfc1002 length field */
 		size = pdu_size + 4;
diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
index 079c9e76818d..87fa578b141e 100644
--- a/fs/ksmbd/smb_common.c
+++ b/fs/ksmbd/smb_common.c
@@ -442,9 +442,26 @@ static int smb_handle_negotiate(struct ksmbd_work *work)
 {
 	struct smb_negotiate_rsp *neg_rsp = work->response_buf;
 
-	ksmbd_debug(SMB, "Unsupported SMB protocol\n");
-	neg_rsp->hdr.Status.CifsError = STATUS_INVALID_LOGON_TYPE;
-	return -EINVAL;
+	ksmbd_debug(SMB, "Unsupported SMB1 protocol\n");
+
+	/*
+	 * Remove 4 byte direct TCP header, add 1 byte wc, 2 byte bcc
+	 * and 2 byte DialectIndex.
+	 */
+	*(__be32 *)work->response_buf =
+		cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2);
+	neg_rsp->hdr.Status.CifsError = STATUS_SUCCESS;
+
+	neg_rsp->hdr.Command = SMB_COM_NEGOTIATE;
+	*(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER;
+	neg_rsp->hdr.Flags = SMBFLG_RESPONSE;
+	neg_rsp->hdr.Flags2 = SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS |
+		SMBFLG2_EXT_SEC | SMBFLG2_IS_LONG_NAME;
+
+	neg_rsp->hdr.WordCount = 1;
+	neg_rsp->DialectIndex = cpu_to_le16(work->conn->dialect);
+	neg_rsp->ByteCount = 0;
+	return 0;
 }
 
 int ksmbd_smb_negotiate_common(struct ksmbd_work *work, unsigned int command)
diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
index e663ab9ea759..d30ce4c1a151 100644
--- a/fs/ksmbd/smb_common.h
+++ b/fs/ksmbd/smb_common.h
@@ -158,8 +158,15 @@
 
 #define SMB1_PROTO_NUMBER		cpu_to_le32(0x424d53ff)
 #define SMB_COM_NEGOTIATE		0x72
-
 #define SMB1_CLIENT_GUID_SIZE		(16)
+
+#define SMBFLG_RESPONSE 0x80	/* this PDU is a response from server */
+
+#define SMBFLG2_IS_LONG_NAME	cpu_to_le16(0x40)
+#define SMBFLG2_EXT_SEC		cpu_to_le16(0x800)
+#define SMBFLG2_ERR_STATUS	cpu_to_le16(0x4000)
+#define SMBFLG2_UNICODE		cpu_to_le16(0x8000)
+
 struct smb_hdr {
 	__be32 smb_buf_length;
 	__u8 Protocol[4];
@@ -199,28 +206,7 @@ struct smb_negotiate_req {
 struct smb_negotiate_rsp {
 	struct smb_hdr hdr;     /* wct = 17 */
 	__le16 DialectIndex; /* 0xFFFF = no dialect acceptable */
-	__u8 SecurityMode;
-	__le16 MaxMpxCount;
-	__le16 MaxNumberVcs;
-	__le32 MaxBufferSize;
-	__le32 MaxRawSize;
-	__le32 SessionKey;
-	__le32 Capabilities;    /* see below */
-	__le32 SystemTimeLow;
-	__le32 SystemTimeHigh;
-	__le16 ServerTimeZone;
-	__u8 EncryptionKeyLength;
 	__le16 ByteCount;
-	union {
-		unsigned char EncryptionKey[8]; /* cap extended security off */
-		/* followed by Domain name - if extended security is off */
-		/* followed by 16 bytes of server GUID */
-		/* then security blob if cap_extended_security negotiated */
-		struct {
-			unsigned char GUID[SMB1_CLIENT_GUID_SIZE];
-			unsigned char SecurityBlob[1];
-		} __packed extended_response;
-	} __packed u;
 } __packed;
 
 struct filesystem_attribute_info {
-- 
2.25.1

Re: [PATCH] ksmbd: return unsupported error on smb1 mount

[Sergey Senozhatsky]

On (23/03/21 22:33), Namjae Jeon wrote:
[..]
> @@ -442,9 +442,26 @@ static int smb_handle_negotiate(struct ksmbd_work *work)
>  {
>  	struct smb_negotiate_rsp *neg_rsp = work->response_buf;
>  
> -	ksmbd_debug(SMB, "Unsupported SMB protocol\n");
> -	neg_rsp->hdr.Status.CifsError = STATUS_INVALID_LOGON_TYPE;
> -	return -EINVAL;
> +	ksmbd_debug(SMB, "Unsupported SMB1 protocol\n");
> +
> +	/*
> +	 * Remove 4 byte direct TCP header, add 1 byte wc, 2 byte bcc
> +	 * and 2 byte DialectIndex.
> +	 */
> +	*(__be32 *)work->response_buf =
> +		cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2);

	In other words cpu_to_be32(sizeof(struct smb_hdr)).

> +	neg_rsp->hdr.Status.CifsError = STATUS_SUCCESS;
> +
> +	neg_rsp->hdr.Command = SMB_COM_NEGOTIATE;
> +	*(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER;

	I assume this should say cpu_to_le32(SMB1_PROTO_NUMBER).

Re: [PATCH] ksmbd: return unsupported error on smb1 mount

[Namjae Jeon]

2023-03-23 11:53 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (23/03/21 22:33), Namjae Jeon wrote:
> [..]
>> @@ -442,9 +442,26 @@ static int smb_handle_negotiate(struct ksmbd_work
>> *work)
>>  {
>>  	struct smb_negotiate_rsp *neg_rsp = work->response_buf;
>>
>> -	ksmbd_debug(SMB, "Unsupported SMB protocol\n");
>> -	neg_rsp->hdr.Status.CifsError = STATUS_INVALID_LOGON_TYPE;
>> -	return -EINVAL;
>> +	ksmbd_debug(SMB, "Unsupported SMB1 protocol\n");
>> +
>> +	/*
>> +	 * Remove 4 byte direct TCP header, add 1 byte wc, 2 byte bcc
>> +	 * and 2 byte DialectIndex.
>> +	 */
>> +	*(__be32 *)work->response_buf =
>> +		cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2);
>
> 	In other words cpu_to_be32(sizeof(struct smb_hdr)).
Yes, that's possible too, but wouldn't this make the comments easier
to understand..?
>
>> +	neg_rsp->hdr.Status.CifsError = STATUS_SUCCESS;
>> +
>> +	neg_rsp->hdr.Command = SMB_COM_NEGOTIATE;
>> +	*(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER;
>
> 	I assume this should say cpu_to_le32(SMB1_PROTO_NUMBER).
SMB1_PROTO_NUMBER is declared as:
#define SMB1_PROTO_NUMBER		cpu_to_le32(0x424d53ff)

Thanks!
>

Re: [PATCH] ksmbd: return unsupported error on smb1 mount

[Sergey Senozhatsky]

On (23/03/23 13:10), Namjae Jeon wrote:
[..]
> >> +	/*
> >> +	 * Remove 4 byte direct TCP header, add 1 byte wc, 2 byte bcc
> >> +	 * and 2 byte DialectIndex.
> >> +	 */
> >> +	*(__be32 *)work->response_buf =
> >> +		cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2);
> >
> > 	In other words cpu_to_be32(sizeof(struct smb_hdr)).
> Yes, that's possible too, but wouldn't this make the comments easier
> to understand..?

Maybe, but the comment says "-4 + 1 + 2 + 2", which doesn't match the code.

> >> +	*(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER;
> >
> > 	I assume this should say cpu_to_le32(SMB1_PROTO_NUMBER).
> SMB1_PROTO_NUMBER is declared as:
> #define SMB1_PROTO_NUMBER		cpu_to_le32(0x424d53ff)

Oh, I see.

Re: [PATCH] ksmbd: return unsupported error on smb1 mount

[Namjae Jeon]

2023-03-23 14:00 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (23/03/23 13:10), Namjae Jeon wrote:
> [..]
>> >> +	/*
>> >> +	 * Remove 4 byte direct TCP header, add 1 byte wc, 2 byte bcc
>> >> +	 * and 2 byte DialectIndex.
>> >> +	 */
>> >> +	*(__be32 *)work->response_buf =
>> >> +		cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2);
>> >
>> > 	In other words cpu_to_be32(sizeof(struct smb_hdr)).
>> Yes, that's possible too, but wouldn't this make the comments easier
>> to understand..?
>
> Maybe, but the comment says "-4 + 1 + 2 + 2", which doesn't match the code.
wc is included in smb_hdr struct. Hm.. I will update the comment.

>
>> >> +	*(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER;
>> >
>> > 	I assume this should say cpu_to_le32(SMB1_PROTO_NUMBER).
>> SMB1_PROTO_NUMBER is declared as:
>> #define SMB1_PROTO_NUMBER		cpu_to_le32(0x424d53ff)
>
> Oh, I see.
>

[PATCH v2] ksmbd: return unsupported error on smb1 mount

[Namjae Jeon]

ksmbd disconnect connection when mounting with vers=smb1.
ksmbd should send smb1 negotiate response to client for correct
unsupported error return. This patch add needed SMB1 macros and fill
NegProt part of the response for smb1 negotiate response.

Reported-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 v2:
   - remove 1byte wc in the comment.

 fs/ksmbd/connection.c |  7 ++-----
 fs/ksmbd/smb_common.c | 23 ++++++++++++++++++++---
 fs/ksmbd/smb_common.h | 30 ++++++++----------------------
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index 5d914715605f..115a67d2cf78 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -319,13 +319,10 @@ int ksmbd_conn_handler_loop(void *p)
 		}

 		/*
-		 * Check if pdu size is valid (min : smb header size,
-		 * max : 0x00FFFFFF).
+		 * Check maximum pdu size(0x00FFFFFF).
 		 */
-		if (pdu_size < __SMB2_HEADER_STRUCTURE_SIZE ||
-		    pdu_size > MAX_STREAM_PROT_LEN) {
+		if (pdu_size > MAX_STREAM_PROT_LEN)
 			break;
-		}

 		/* 4 for rfc1002 length field */
 		size = pdu_size + 4;
diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
index 079c9e76818d..87fa578b141e 100644
--- a/fs/ksmbd/smb_common.c
+++ b/fs/ksmbd/smb_common.c
@@ -442,9 +442,26 @@ static int smb_handle_negotiate(struct ksmbd_work *work)
 {
 	struct smb_negotiate_rsp *neg_rsp = work->response_buf;

-	ksmbd_debug(SMB, "Unsupported SMB protocol\n");
-	neg_rsp->hdr.Status.CifsError = STATUS_INVALID_LOGON_TYPE;
-	return -EINVAL;
+	ksmbd_debug(SMB, "Unsupported SMB1 protocol\n");
+
+	/*
+	 * Remove 4 byte direct TCP header, add 2 byte bcc and
+	 * 2 byte DialectIndex.
+	 */
+	*(__be32 *)work->response_buf =
+		cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2);
+	neg_rsp->hdr.Status.CifsError = STATUS_SUCCESS;
+
+	neg_rsp->hdr.Command = SMB_COM_NEGOTIATE;
+	*(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER;
+	neg_rsp->hdr.Flags = SMBFLG_RESPONSE;
+	neg_rsp->hdr.Flags2 = SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS |
+		SMBFLG2_EXT_SEC | SMBFLG2_IS_LONG_NAME;
+
+	neg_rsp->hdr.WordCount = 1;
+	neg_rsp->DialectIndex = cpu_to_le16(work->conn->dialect);
+	neg_rsp->ByteCount = 0;
+	return 0;
 }

 int ksmbd_smb_negotiate_common(struct ksmbd_work *work, unsigned int command)
diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
index e663ab9ea759..d30ce4c1a151 100644
--- a/fs/ksmbd/smb_common.h
+++ b/fs/ksmbd/smb_common.h
@@ -158,8 +158,15 @@

 #define SMB1_PROTO_NUMBER		cpu_to_le32(0x424d53ff)
 #define SMB_COM_NEGOTIATE		0x72
-
 #define SMB1_CLIENT_GUID_SIZE		(16)
+
+#define SMBFLG_RESPONSE 0x80	/* this PDU is a response from server */
+
+#define SMBFLG2_IS_LONG_NAME	cpu_to_le16(0x40)
+#define SMBFLG2_EXT_SEC		cpu_to_le16(0x800)
+#define SMBFLG2_ERR_STATUS	cpu_to_le16(0x4000)
+#define SMBFLG2_UNICODE		cpu_to_le16(0x8000)
+
 struct smb_hdr {
 	__be32 smb_buf_length;
 	__u8 Protocol[4];
@@ -199,28 +206,7 @@ struct smb_negotiate_req {
 struct smb_negotiate_rsp {
 	struct smb_hdr hdr;     /* wct = 17 */
 	__le16 DialectIndex; /* 0xFFFF = no dialect acceptable */
-	__u8 SecurityMode;
-	__le16 MaxMpxCount;
-	__le16 MaxNumberVcs;
-	__le32 MaxBufferSize;
-	__le32 MaxRawSize;
-	__le32 SessionKey;
-	__le32 Capabilities;    /* see below */
-	__le32 SystemTimeLow;
-	__le32 SystemTimeHigh;
-	__le16 ServerTimeZone;
-	__u8 EncryptionKeyLength;
 	__le16 ByteCount;
-	union {
-		unsigned char EncryptionKey[8]; /* cap extended security off */
-		/* followed by Domain name - if extended security is off */
-		/* followed by 16 bytes of server GUID */
-		/* then security blob if cap_extended_security negotiated */
-		struct {
-			unsigned char GUID[SMB1_CLIENT_GUID_SIZE];
-			unsigned char SecurityBlob[1];
-		} __packed extended_response;
-	} __packed u;
 } __packed;

 struct filesystem_attribute_info {
-- 
2.34.1

Re: [PATCH v2] ksmbd: return unsupported error on smb1 mount

[Sergey Senozhatsky]

On (23/03/23 14:17), Namjae Jeon wrote:
> ksmbd disconnect connection when mounting with vers=smb1.
> ksmbd should send smb1 negotiate response to client for correct
> unsupported error return. This patch add needed SMB1 macros and fill
> NegProt part of the response for smb1 negotiate response.
> 
> Reported-by: Steve French <stfrench@microsoft.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Re: [PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[Sergey Senozhatsky]

On (23/03/21 22:33), Namjae Jeon wrote:
[..]
> @@ -335,14 +336,23 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
>  		} else if (conn->status == KSMBD_SESS_NEED_RECONNECT) {
>  			total_read = -EAGAIN;
>  			break;
> -		} else if ((length == -ERESTARTSYS || length == -EAGAIN) &&
> -			   max_retry) {
> +		} else if (length == -ERESTARTSYS || length == -EAGAIN) {
> +			/*
> +			 * If max_retries is negative, Allow unlimited
> +			 * retries to keep connection with inactive sessions.
> +			 */
> +			if (max_retries == 0) {
> +				total_read = length;
> +				break;
> +			} else if (max_retries > 0) {
> +				max_retries--;
> +			}
> +
>  			usleep_range(1000, 2000);
>  			length = 0;
> -			max_retry--;
>  			continue;
>  		} else if (length <= 0) {
> -			total_read = -EAGAIN;
> +			total_read = length;
>  			break;
>  		}
>  	}

By the way, ksmbd_tcp_readv() calls kvec_array_init() on each iteration.
Shouldn't we call it only if length > 0? That is only if the most recent
call to kernel_recvmsg() has read some data.

Re: [PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[Namjae Jeon]

2023-03-24 12:50 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (23/03/21 22:33), Namjae Jeon wrote:
> [..]
>> @@ -335,14 +336,23 @@ static int ksmbd_tcp_readv(struct tcp_transport *t,
>> struct kvec *iov_orig,
>>  		} else if (conn->status == KSMBD_SESS_NEED_RECONNECT) {
>>  			total_read = -EAGAIN;
>>  			break;
>> -		} else if ((length == -ERESTARTSYS || length == -EAGAIN) &&
>> -			   max_retry) {
>> +		} else if (length == -ERESTARTSYS || length == -EAGAIN) {
>> +			/*
>> +			 * If max_retries is negative, Allow unlimited
>> +			 * retries to keep connection with inactive sessions.
>> +			 */
>> +			if (max_retries == 0) {
>> +				total_read = length;
>> +				break;
>> +			} else if (max_retries > 0) {
>> +				max_retries--;
>> +			}
>> +
>>  			usleep_range(1000, 2000);
>>  			length = 0;
>> -			max_retry--;
>>  			continue;
>>  		} else if (length <= 0) {
>> -			total_read = -EAGAIN;
>> +			total_read = length;
>>  			break;
>>  		}
>>  	}
>
> By the way, ksmbd_tcp_readv() calls kvec_array_init() on each iteration.
> Shouldn't we call it only if length > 0? That is only if the most recent
> call to kernel_recvmsg() has read some data.
If length == to_read is equal then it is not called. And in case
length < to_read, we have to call it which reinitialize io vec again
for reading the rest of the data.
>

Re: [PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[Sergey Senozhatsky]

On (23/03/21 22:33), Namjae Jeon wrote:
> Steve reported that inactive sessions are terminated after a few
> seconds. ksmbd terminate when receiving -EAGAIN error from
> kernel_recvmsg(). -EAGAIN means there is no data available in timeout.
> So ksmbd should keep connection with unlimited retries instead of
> terminating inactive sessions.
> 
> Reported-by: Steve French <stfrench@microsoft.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Re: [PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[Sergey Senozhatsky]

On (23/03/24 13:28), Namjae Jeon wrote:
> > By the way, ksmbd_tcp_readv() calls kvec_array_init() on each iteration.
> > Shouldn't we call it only if length > 0? That is only if the most recent
> > call to kernel_recvmsg() has read some data.
> If length == to_read is equal then it is not called. And in case
> length < to_read, we have to call it which reinitialize io vec again
> for reading the rest of the data.

What I'm saying is: if length == 0 on the previous iteration then
we don't need to kvec_array_init(). But maybe I'm missing something.

Re: [PATCH] ksmbd: don't terminate inactive sessions after a few seconds

[Namjae Jeon]

2023-03-24 14:45 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (23/03/24 13:28), Namjae Jeon wrote:
>> > By the way, ksmbd_tcp_readv() calls kvec_array_init() on each
>> > iteration.
>> > Shouldn't we call it only if length > 0? That is only if the most
>> > recent
>> > call to kernel_recvmsg() has read some data.
>> If length == to_read is equal then it is not called. And in case
>> length < to_read, we have to call it which reinitialize io vec again
>> for reading the rest of the data.
>
> What I'm saying is: if length == 0 on the previous iteration then
> we don't need to kvec_array_init(). But maybe I'm missing something.
You're right. We can improve it with another patch.

Thanks for your review!
>