[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAKYAXd-7ojpN=jc+R2wwxyQsZCTBJT6tEYszb4VOgbPeWn1NKA@mail.gmail.com>
Date: Tue, 19 Aug 2025 17:00:03 +0900
From: Namjae Jeon <linkinjeon@...nel.org>
To: Yunseong Kim <ysk@...lloc.com>
Cc: Steve French <smfrench@...il.com>, Stefan Metzmacher <metze@...ba.org>,
Paulo Alcantara <pc@...guebit.org>, Sergey Senozhatsky <senozhatsky@...omium.org>, Tom Talpey <tom@...pey.com>,
linux-cifs@...r.kernel.org, syzkaller@...glegroups.com,
linux-kernel@...r.kernel.org, notselwyn@...ing.tech
Subject: Re: [PATCH v3] ksmbd: add kcov remote coverage support via ksmbd_conn
On Wed, Aug 6, 2025 at 11:41 PM Yunseong Kim <ysk@...lloc.com> wrote:
>
Hi Yunseong,
> KSMBD processes SMB requests on per-connection threads and then hands
> off work items to a kworker pool for actual command processing by
> handle_ksmbd_work(). Because each connection may enqueue multiple
> struct ksmbd_work instances, attaching the kcov handle to the work
> itself is not sufficient: we need a stable, per-connection handle.
>
> Introduce a kcov_handle field on struct ksmbd_conn (under CONFIG_KCOV)
> and initialize it when the connection is set up. In both
> ksmbd_conn_handler_loop() which only receives a struct ksmbd_conn*
> and handle_ksmbd_work() which receives a struct ksmbd_work*, start
> kcov_remote with the per-connection handle before processing and stop
> it afterward. This ensures coverage collection remains active across
> the entire asynchronous path of each SMB request.
I'm a bit unclear on the overall impact. Do you have the goal to measure
the code coverage of all ksmbd components ?
Is there the next patch set or any plan for next work, or is this patch enough
to check all functions of ksmbd with syzkaller?
Thanks.
>
> The kcov context tied to the connection itself, correctly supporting
> multiple outstanding work items per connection.
>
> In patch v2, I added the missing initialization of kcov_handle. In v3,
> I fixed an kcov_hanlde argument was previously unused on
> ksmbd_conn_set_kcov_handle().
>
> The related work for syzkaller support is currently being developed
> in the following GitHub PR:
> Link: https://github.com/google/syzkaller/pull/5524
>
> Based on earlier work by Lau.
> Link: https://pwning.tech/ksmbd-syzkaller/
>
> Cc: linux-cifs@...r.kernel.org
> Cc: notselwyn@...ing.tech
> Signed-off-by: Yunseong Kim <ysk@...lloc.com>
> ---
> fs/smb/server/connection.c | 7 ++++++-
> fs/smb/server/connection.h | 22 ++++++++++++++++++++++
> fs/smb/server/server.c | 4 ++++
> 3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c
> index 3f04a2977ba8..21352f37384f 100644
> --- a/fs/smb/server/connection.c
> +++ b/fs/smb/server/connection.c
> @@ -93,6 +93,9 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
> down_write(&conn_list_lock);
> list_add(&conn->conns_list, &conn_list);
> up_write(&conn_list_lock);
> +
> + ksmbd_conn_set_kcov_handle(conn, kcov_common_handle());
> +
> return conn;
> }
>
> @@ -322,6 +325,8 @@ int ksmbd_conn_handler_loop(void *p)
> if (t->ops->prepare && t->ops->prepare(t))
> goto out;
>
> + kcov_remote_start_common(ksmbd_conn_get_kcov_handle(conn));
> +
> max_req = server_conf.max_inflight_req;
> conn->last_active = jiffies;
> set_freezable();
> @@ -412,7 +417,7 @@ int ksmbd_conn_handler_loop(void *p)
> break;
> }
> }
> -
> + kcov_remote_stop();
> out:
> ksmbd_conn_set_releasing(conn);
> /* Wait till all reference dropped to the Server object*/
> diff --git a/fs/smb/server/connection.h b/fs/smb/server/connection.h
> index dd3e0e3f7bf0..a90bd1b3e1df 100644
> --- a/fs/smb/server/connection.h
> +++ b/fs/smb/server/connection.h
> @@ -15,6 +15,7 @@
> #include <linux/kthread.h>
> #include <linux/nls.h>
> #include <linux/unicode.h>
> +#include <linux/kcov.h>
>
> #include "smb_common.h"
> #include "ksmbd_work.h"
> @@ -109,6 +110,9 @@ struct ksmbd_conn {
> bool binding;
> atomic_t refcnt;
> bool is_aapl;
> +#ifdef CONFIG_KCOV
> + u64 kcov_handle;
> +#endif
> };
>
> struct ksmbd_conn_ops {
> @@ -246,4 +250,22 @@ static inline void ksmbd_conn_set_releasing(struct ksmbd_conn *conn)
> }
>
> void ksmbd_all_conn_set_status(u64 sess_id, u32 status);
> +
> +static inline void ksmbd_conn_set_kcov_handle(struct ksmbd_conn *conn,
> + const u64 kcov_handle)
> +{
> +#ifdef CONFIG_KCOV
> + conn->kcov_handle = kcov_handle;
> +#endif
> +}
> +
> +static inline u64 ksmbd_conn_get_kcov_handle(struct ksmbd_conn *conn)
> +{
> +#ifdef CONFIG_KCOV
> + return conn->kcov_handle;
> +#else
> + return 0;
> +#endif
> +}
> +
> #endif /* __CONNECTION_H__ */
> diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
> index 8c9c49c3a0a4..0757cd6ef4f7 100644
> --- a/fs/smb/server/server.c
> +++ b/fs/smb/server/server.c
> @@ -264,6 +264,8 @@ static void handle_ksmbd_work(struct work_struct *wk)
> struct ksmbd_work *work = container_of(wk, struct ksmbd_work, work);
> struct ksmbd_conn *conn = work->conn;
>
> + kcov_remote_start_common(ksmbd_conn_get_kcov_handle(conn));
> +
> atomic64_inc(&conn->stats.request_served);
>
> __handle_ksmbd_work(work, conn);
> @@ -271,6 +273,8 @@ static void handle_ksmbd_work(struct work_struct *wk)
> ksmbd_conn_try_dequeue_request(work);
> ksmbd_free_work_struct(work);
> ksmbd_conn_r_count_dec(conn);
> +
> + kcov_remote_stop();
> }
>
> /**
> --
> 2.50.0
>
Powered by blists - more mailing lists