| Message ID | 20221227145954.9663-1-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ksmbd: add max connections parameter | expand |
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().
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! >
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)
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(-)