sys/linux, executor: Add KSMBD subsystem support #5524

yskelg · 2024-11-21T16:47:16Z

Introduces a new pseudo-syscall syz_ksmbd_send_req to fuzzing of the KSMBD module. The added functionality enables sending custom SMB2/3 requests to the KSMBD server on 127.0.0.1:445.

I've been updating @Notselwyn work to ensure it functions on the latest v6.12 Linux Kernel and added support for smb2_read_req. Additionally, I have updated the scripts related to the KSMBD setup to work smoothly.
Link: https://groups.google.com/g/syzkaller/c/8ZI_sY9OJEw

For reference, @namjaejeon, the upstream Linux kernel maintainer, is highly anticipating syzkaller's support for KSMBD. I plan to work with Namjae to enable KCOV support.

Install ksmbd-tools

# See https://github.com/cifsd-team/ksmbd-tools
# Debian
$ sudo apt install -y ksmbd-tools

Compile KSMBD enabled Linux kernel.

# See https://github.com/namjaejeon/ksmbd
CONFIG_CIFS
CONFIG_SMB_SERVER
CONFIG_HPFS_FS

This is setup script for KSMBD target machine.

vim /etc/systemd/system/ksmbd.service

[Unit]
Description=Ksmbd
After=network.target
StartLimitIntervalSec=0
Type=simple
Restart=always
RestartSec=1

[Service]
User=root
Group=root
ExecStart=/root/start_ksmbd.sh

[Install]
WantedBy=multi-user.target

sudo vim /root/start_ksmbd.sh
sudo chmod 777 /root/start_ksmbd.sh

#!/usr/bin/env bash

ksmbd.addshare --add --option "path = /tmp" --option 'read only = no' MyShare
ksmbd.adduser -a $MY_USER -p $MY_PASSWORD
ksmbd.mountd -n

sudo systemctl enable ksmbd.service
sudo systemctl start ksmbd.service
sudo systemctl status ksmbd.service

(Optional) Configure map to guest = bad user in ksmbd.conf. This step is crucial not only for handling negotiation requests in the future but also for verifying file write operations from properly connected smb clients, as suggested by @namjaejeon.

vim /etc/ksmbd/ksmbd.conf

; see ksmbd.conf(5) for details

[global]
	...
	; map to guest = never
	map to guest = bad user

This is the initial configuration I worked on using the pseudo syscall before addressing the review comments. I'll update it once the work is complete.

{
    "name": "QEMU-aarch64",
    "target": "linux/arm64",
    "http": ":56701",
    "rpc": ":0",
    "workdir": "/home/paran/syzkaller/workdir",
    "kernel_obj": "/home/paran/linux",
    "kernel_src": "/home/paran/linux",
    "image": "/home/paran/ubuntu-24.10-server-cloudimg-arm64.img",
    "kernel_build_src": "/home/paran/linux",
    "sshkey": "/home/paran/.ssh/ksmbd_ecdsa_key",
    "syzkaller": "/home/paran/syzkaller",
    "procs": 2,
    "max_crash_logs": 100,
    "sandbox": "setuid",
    "cover": true,
    "cover_filter": {
        "files": ["^fs/smb/"],
	"functions": ["^ksmbd"]
    },
    "reproduce": false,
    "type": "qemu",
    "vm": {
        "count": 2,
        "qemu": "/home/paran/qemu/build/qemu-system-aarch64",
        "cmdline": "panic_on_warn=1 nmi_watchdog=panic panic=86400 sysctl.kernel.hung_task_all_cpu_backtrace=1 ftrace_dump_on_oops=orig_cpu slub_debug=UZ nokaslr",
        "kernel": "/home/paran/linux/arch/arm64/boot/Image",
        "cpu": 2,
        "mem": 2048,
        "qemu_args": "-machine virt -enable-kvm -cpu host -bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd -drive if=none,file=/home/paran/ubuntu-24.10-server-cloudimg-arm64.img,id=hd0 -device virtio-blk-device,drive=hd0 -cdrom /home/paran/user-data.img -device virtio-sound-pci,audiodev=my_audiodev -audiodev alsa,id=my_audiodev -device virtio-keyboard-pci -device virtio-mouse-pci -device virtio-tablet-pci -device virtio-gpu-pci",
        "initrd": "/home/paran/initrd.img-6.11.0-9-generic"
    },
    "enable_syscalls": [
	"syz_ksmbd_send_req"
    ]
}

Cc: [email protected]
Cc: [email protected]
Cc: Namjae Jeon [email protected]

tarasmadan · 2024-11-21T17:07:06Z

Nice!
And we don't have smb2 subsystem defined to track the coverage progress. It is not a blocker, just a TODO.

tarasmadan · 2024-11-22T10:55:40Z

We have some coverage from fs/smb/client here.
We don't have any coverage from fs/smb/server because SMB_SERVER(at least) config option isn't enabled.

a-nogikh · 2024-11-22T10:49:20Z

executor/common_linux.h

+		return errno ^ 0xff80000;
+	}
+
+	// prepend kcov handle to packet


I think it should not be necessary to pass a remote kcov reference explicitly within the packet. Syzkaller calls kcov_remote_start/kcov_remote_stop automatically, so the write handler on the kernel side should already be aware of the right kcov remote reference.

I understand that it would also require rewriting the patch to the kernel as well, so don't treat this comment as a blocker for merging this PR.

a-nogikh · 2024-11-22T11:00:48Z

executor/common_linux.h

+
+#define KSMBD_BUF_SIZE 16000
+
+static long syz_ksmbd_send_req(volatile long a0, volatile long a1, volatile long a2, volatile long a3)


Do you plan to read out the ksmdb responses and somehow act upon them?

If not, we probably don't really need to introduce a pseudo system call at all. If you pass the kcov remote reference the standard way, all that remains are:

A socket call.

A connect call with the specific arguments.

A write call.

These can be expressed in syzkaller descriptions and e.g. tied together by defining some custom resource sock_ksmbd[sock] resource.

+1 for not adding a pseudo syscall. We just need to describe the tcp address, and add a special resource for the socket to tie all syscall together (socket/connect/sendmsg).

It has an additional advantage of giving more freedom to the fuzzer and exploring more scenarios. E.g. sending messages in parallel, calling shutdown, etc.

a-nogikh · 2024-11-22T11:05:14Z

executor/common_linux.h

+
+	if (a0 == 0 || a1 == 0) {
+		debug("[!]{syz_ksmbd_send_req} param empty\n");
+		return -7;


For the fuzzer, it's not really important to distinguish between these specific failure paths. I would just return -1 for all of them.

a-nogikh · 2024-11-22T11:07:03Z

pkg/vminfo/linux_syscalls.go

@@ -103,6 +103,7 @@ var linuxSyscallChecks = map[string]func(*checkContext, *prog.Syscall) string{
 	"syz_socket_connect_nvme_tcp": linuxSyzSocketConnectNvmeTCPSupported,
 	"syz_pidfd_open":              alwaysSupported,
 	"syz_create_resource":         alwaysSupported,
+	"syz_ksmbd_send_req":          alwaysSupported,


I guess that for most users it will actually be not supported, so it would be nice to be more concrete here.

What is the simplest way of determining whether the server exists?

a-nogikh · 2024-11-22T11:08:44Z

sys/linux/ksmbd.txt

+#	Id		_union_smb_req_id
+#	SessionId	int64
+#	Signature	array[int8, 16]
+#} [packed]


If these structs are not necessary, let's just drop them from the final descriptions.

a-nogikh · 2024-11-22T11:10:46Z

sys/linux/ksmbd.txt

+
+smb2_req {
+	req_base	_union_smb2_req
+	req_andx1	optional[_union_smb2_req]


Nit: if that depends on the specific values set in the header, please note that there's a way to express that in syzlang as well:

https://github.com/google/syzkaller/blob/master/docs/syscall_descriptions_syntax.md#conditional-fields

tarasmadan · 2024-11-22T13:16:05Z

#5528 to enable SMB_SERVER

Introduces a new pseudo-syscall `syz_ksmbd_send_req` to fuzzing of the KSMBD module. The added functionality enables sending custom SMB2/3 requests to the KSMBD server on 127.0.0.1:445. Cc: [email protected] Cc: [email protected] Cc: Namjae Jeon <[email protected]> Signed-off-by: Yunseong Kim <[email protected]>

A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bf ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]>

yskelg · 2024-11-25T10:26:39Z

Thank you all for your excellent comments. They have been helpful in guiding the work in a positive direction, and I will do my best to incorporate the points you mentioned.

A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bf ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]>

A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bfb9d09 ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Signed-off-by: Namjae Jeon <[email protected]>

commit 9a8c5d8 upstream. A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bf ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

commit 9a8c5d89d327ff58e9b2517f8a6afb4181d32c6e upstream. A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bfb9d09 ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit 1a955b62fd6adf91e4c4cfa05dc6f82b12f1f200)

commit 9a8c5d8 upstream. A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bf ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

commit 9a8c5d89d327ff58e9b2517f8a6afb4181d32c6e upstream. A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bfb9d09 ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit a96f9eb7add30ba0fafcfe7b7aca090978196800)

commit 9a8c5d89d327ff58e9b2517f8a6afb4181d32c6e upstream. A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: google/syzkaller#5524 Fixes: ee426bf ("ksmbd: add refcnt to ksmbd_conn struct") Cc: [email protected] Cc: [email protected] # v6.6.55+, v6.10.14+, v6.11.3+ Cc: [email protected] Signed-off-by: Yunseong Kim <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

tarasmadan mentioned this pull request Nov 21, 2024

subsystems: add smb2 definition #5525

Closed

a-nogikh reviewed Nov 22, 2024

View reviewed changes

yskelg force-pushed the syzkaller-ksmbd branch from 5f70782 to d0872de Compare November 24, 2024 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/linux, executor: Add KSMBD subsystem support #5524

sys/linux, executor: Add KSMBD subsystem support #5524

yskelg commented Nov 21, 2024 •

edited

Loading

tarasmadan commented Nov 21, 2024

tarasmadan commented Nov 22, 2024

a-nogikh Nov 22, 2024

a-nogikh Nov 22, 2024

dvyukov Nov 25, 2024

a-nogikh Nov 22, 2024

a-nogikh Nov 22, 2024

a-nogikh Nov 22, 2024

a-nogikh Nov 22, 2024

tarasmadan commented Nov 22, 2024

yskelg commented Nov 25, 2024


		#define KSMBD_BUF_SIZE 16000

		static long syz_ksmbd_send_req(volatile long a0, volatile long a1, volatile long a2, volatile long a3)

sys/linux, executor: Add KSMBD subsystem support #5524

Are you sure you want to change the base?

sys/linux, executor: Add KSMBD subsystem support #5524

Conversation

yskelg commented Nov 21, 2024 • edited Loading

tarasmadan commented Nov 21, 2024

tarasmadan commented Nov 22, 2024

a-nogikh Nov 22, 2024

Choose a reason for hiding this comment

a-nogikh Nov 22, 2024

Choose a reason for hiding this comment

dvyukov Nov 25, 2024

Choose a reason for hiding this comment

a-nogikh Nov 22, 2024

Choose a reason for hiding this comment

a-nogikh Nov 22, 2024

Choose a reason for hiding this comment

a-nogikh Nov 22, 2024

Choose a reason for hiding this comment

a-nogikh Nov 22, 2024

Choose a reason for hiding this comment

tarasmadan commented Nov 22, 2024

yskelg commented Nov 25, 2024

yskelg commented Nov 21, 2024 •

edited

Loading