[PATCH] ksmbd: add max connections parameter

[PATCH] ksmbd: add max connections parameter

[Namjae Jeon]

Add max connections parameter to limit number of maximum simultaneous
connections.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/ksmbd_netlink.h |  3 ++-
 fs/ksmbd/server.h        |  1 +
 fs/ksmbd/transport_ipc.c |  3 +++
 fs/ksmbd/transport_tcp.c | 19 ++++++++++++++++++-
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/fs/ksmbd/ksmbd_netlink.h b/fs/ksmbd/ksmbd_netlink.h
index b6bd8311e6b4..fb8b2d566efb 100644
--- a/fs/ksmbd/ksmbd_netlink.h
+++ b/fs/ksmbd/ksmbd_netlink.h
@@ -106,7 +106,8 @@ struct ksmbd_startup_request {
 	__u32	sub_auth[3];		/* Subauth value for Security ID */
 	__u32	smb2_max_credits;	/* MAX credits */
 	__u32	smbd_max_io_size;	/* smbd read write size */
-	__u32	reserved[127];		/* Reserved room */
+	__u32	max_connections;	/* Number of maximum simultaneous connections */
+	__u32	reserved[126];		/* Reserved room */
 	__u32	ifc_list_sz;		/* interfaces list size */
 	__s8	____payload[];
 };
diff --git a/fs/ksmbd/server.h b/fs/ksmbd/server.h
index ac9d932f8c8a..db7278181760 100644
--- a/fs/ksmbd/server.h
+++ b/fs/ksmbd/server.h
@@ -41,6 +41,7 @@ struct ksmbd_server_config {
 	unsigned int		share_fake_fscaps;
 	struct smb_sid		domain_sid;
 	unsigned int		auth_mechs;
+	unsigned int		max_connections;
 
 	char			*conf[SERVER_CONF_WORK_GROUP + 1];
 };
diff --git a/fs/ksmbd/transport_ipc.c b/fs/ksmbd/transport_ipc.c
index c9aca21637d5..40c721f9227e 100644
--- a/fs/ksmbd/transport_ipc.c
+++ b/fs/ksmbd/transport_ipc.c
@@ -308,6 +308,9 @@ static int ipc_server_config_on_startup(struct ksmbd_startup_request *req)
 	if (req->smbd_max_io_size)
 		init_smbd_max_io_size(req->smbd_max_io_size);
 
+	if (req->max_connections)
+		server_conf.max_connections = req->max_connections;
+
 	ret = ksmbd_set_netbios_name(req->netbios_name);
 	ret |= ksmbd_set_server_string(req->server_string);
 	ret |= ksmbd_set_work_group(req->work_group);
diff --git a/fs/ksmbd/transport_tcp.c b/fs/ksmbd/transport_tcp.c
index 63d55f543bd2..c60f7b8b22fe 100644
--- a/fs/ksmbd/transport_tcp.c
+++ b/fs/ksmbd/transport_tcp.c
@@ -15,6 +15,8 @@
 #define IFACE_STATE_DOWN		BIT(0)
 #define IFACE_STATE_CONFIGURED		BIT(1)
 
+static atomic_t active_num_conn;
+
 struct interface {
 	struct task_struct	*ksmbd_kthread;
 	struct socket		*ksmbd_socket;
@@ -185,8 +187,10 @@ static int ksmbd_tcp_new_connection(struct socket *client_sk)
 	struct tcp_transport *t;
 
 	t = alloc_transport(client_sk);
-	if (!t)
+	if (!t) {
+		sock_release(client_sk);
 		return -ENOMEM;
+	}
 
 	csin = KSMBD_TCP_PEER_SOCKADDR(KSMBD_TRANS(t)->conn);
 	if (kernel_getpeername(client_sk, csin) < 0) {
@@ -239,6 +243,17 @@ static int ksmbd_kthread_fn(void *p)
 			continue;
 		}
 
+		if (server_conf.max_connections) {
+			if (atomic_read(&active_num_conn) >= server_conf.max_connections) {
+				pr_info("Limit the maximum number of connections(%u)\n",
+						atomic_read(&active_num_conn));
+				sock_release(client_sk);
+				continue;
+			}
+
+			atomic_inc(&active_num_conn);
+		}
+
 		ksmbd_debug(CONN, "connect success: accepted new connection\n");
 		client_sk->sk->sk_rcvtimeo = KSMBD_TCP_RECV_TIMEOUT;
 		client_sk->sk->sk_sndtimeo = KSMBD_TCP_SEND_TIMEOUT;
@@ -365,6 +380,8 @@ static int ksmbd_tcp_writev(struct ksmbd_transport *t, struct kvec *iov,
 static void ksmbd_tcp_disconnect(struct ksmbd_transport *t)
 {
 	free_transport(TCP_TRANS(t));
+	if (server_conf.max_connections)
+		atomic_dec(&active_num_conn);
 }
 
 static void tcp_destroy_socket(struct socket *ksmbd_socket)
-- 
2.25.1

Re: [PATCH] ksmbd: add max connections parameter

[Sergey Senozhatsky]

On (22/12/27 23:59), Namjae Jeon wrote:
[..]
>  	csin = KSMBD_TCP_PEER_SOCKADDR(KSMBD_TRANS(t)->conn);
>  	if (kernel_getpeername(client_sk, csin) < 0) {
> @@ -239,6 +243,17 @@ static int ksmbd_kthread_fn(void *p)
>  			continue;
>  		}
>  
> +		if (server_conf.max_connections) {
> +			if (atomic_read(&active_num_conn) >= server_conf.max_connections) {
> +				pr_info("Limit the maximum number of connections(%u)\n",
> +						atomic_read(&active_num_conn));
> +				sock_release(client_sk);
> +				continue;
> +			}
> +
> +			atomic_inc(&active_num_conn);
> +		}

This has to be one atomic op:

	if (atomic_inc_return() >= max_connections) {
		atomic_dec
		sock_release
		continue
	}

Otherwise it's racy and it's possible to have more active
connections than the limit.

I'd also note that pr_info() is risky there, it would be safer
to use rate-limited printk().

Re: [PATCH] ksmbd: add max connections parameter

[Namjae Jeon]

2022-12-29 11:29 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (22/12/27 23:59), Namjae Jeon wrote:
> [..]
>>  	csin = KSMBD_TCP_PEER_SOCKADDR(KSMBD_TRANS(t)->conn);
>>  	if (kernel_getpeername(client_sk, csin) < 0) {
>> @@ -239,6 +243,17 @@ static int ksmbd_kthread_fn(void *p)
>>  			continue;
>>  		}
>>
>> +		if (server_conf.max_connections) {
>> +			if (atomic_read(&active_num_conn) >= server_conf.max_connections) {
>> +				pr_info("Limit the maximum number of connections(%u)\n",
>> +						atomic_read(&active_num_conn));
>> +				sock_release(client_sk);
>> +				continue;
>> +			}
>> +
>> +			atomic_inc(&active_num_conn);
>> +		}
>
> This has to be one atomic op:
>
> 	if (atomic_inc_return() >= max_connections) {
> 		atomic_dec
> 		sock_release
> 		continue
> 	}
>
> Otherwise it's racy and it's possible to have more active
> connections than the limit.
Ok.
>
> I'd also note that pr_info() is risky there, it would be safer
> to use rate-limited printk().
Ok. FIXED. I have sent v2 patch.
Thanks for your review!
>