Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Symlinks #412

Open
Anuskuss opened this issue Dec 25, 2022 · 12 comments
Open

Symlinks #412

Anuskuss opened this issue Dec 25, 2022 · 12 comments

Comments

@Anuskuss
Copy link

When will symlink support return? I've been using vers=1.0,unix with Samba but I'd like to switch to ksmbd and use vers=3.1.1,posix. Apparently SMB1 and symlink support has been removed from ksmbd in Linux 5.15-rc2. Can you provide those features for users that "don't care", perhaps behind a unsafe = yes option?
@KalelCooper
Copy link

KalelCooper commented Dec 25, 2022 via email

@KalelCooper
Copy link

KalelCooper commented Dec 25, 2022 via email

@Anuskuss
Copy link
Author

@KalelCooper Which kernel version does this target? I tried to apply it to linux-source-6.0.tar.xz but it failed. Also please next time wrap your code inside backticks like this:

```diff
<DIFF>
```

I tried to manually rebase the original patch but the code is too difficult for me. I suspect that your diff is for 5.15, which would be a shame because there's no 5.15 kernel in Debian and I really don't want to run my own kernel.

@KalelCooper
Copy link

The patch is for 94ea4a8
Not for the kernel, you need to replace it after patched.

diff -Naurp a/smb2pdu.c b/smb2pdu.c
---  a/smb2pdu.c
+++ b/smb2pdu.c
@@ -2760,7 +2760,7 @@ int smb2_open(struct ksmbd_work *work)
 		goto err_out1;
 	}
 
-	rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, 1);
+	rc = ksmbd_vfs_kern_path(work, name, req->DesiredAccess & FILE_DELETE_LE ? LOOKUP_NO_SYMLINKS : LOOKUP_FOLLOW, &path, 1);
 	if (!rc) {
 		if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) {
 			/*
@@ -3675,6 +3675,17 @@ static int process_query_dir_entries(str
 	int			rc;
 	int			i;
 
+	char dir_path_buf[512 + 128];
+	const char *dir_path = d_path(&priv->dir_fp->filp->f_path, dir_path_buf, sizeof(dir_path_buf));
+	if (IS_ERR(dir_path)) {
+		dir_path = dir_path_buf;
+	} else {
+		for (i = 0; *dir_path && i < sizeof(dir_path_buf) - 1; ++i, ++dir_path) {
+			dir_path_buf[i] = *dir_path;
+		} if (dir_path_buf[i] != '/') dir_path_buf[i++] = '/'; dir_path_buf[i] = '\0';
+		dir_path = dir_path_buf + i;
+	}
+
 	for (i = 0; i < priv->d_info->num_entry; i++) {
 		struct dentry *dent;
 
@@ -3706,7 +3717,22 @@ static int process_query_dir_entries(str
 			continue;
 		}
 
-		ksmbd_kstat.kstat = &kstat;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 12, 0)
+		generic_fillattr(user_ns, d_inode(dent), &kstat);
+#else
+		generic_fillattr(d_inode(dent), &kstat);
+#endif
+
+		if (S_ISLNK(kstat.mode) && dir_path + dent->d_name.len + 1 < dir_path_buf + sizeof(dir_path_buf)) {
+			struct path path;
+			memcpy((void*)dir_path, dent->d_name.name, dent->d_name.len + 1);
+			if (kern_path(dir_path_buf, LOOKUP_FOLLOW, &path) == 0) {
+				vfs_getattr_nosec(&path, &kstat, 0, 0);
+				path_put(&path);
+			}
+		}
+
+		ksmbd_kstat.kstat = &kstat;		
 		if (priv->info_level != FILE_NAMES_INFORMATION)
 			ksmbd_vfs_fill_dentry_attrs(priv->work,
 						    user_ns,
diff -Naurp a/vfs.c b/vfs.c
---  a/vfs.c
+++ b/vfs.c
@@ -2219,12 +2219,6 @@ int ksmbd_vfs_fill_dentry_attrs(struct k
 	u64 time;
 	int rc;
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 12, 0)
-	generic_fillattr(user_ns, d_inode(dentry), ksmbd_kstat->kstat);
-#else
-	generic_fillattr(d_inode(dentry), ksmbd_kstat->kstat);
-#endif
-
 	time = ksmbd_UnixTimeToNT(ksmbd_kstat->kstat->ctime);
 	ksmbd_kstat->create_time = time;

@Anuskuss
Copy link
Author

Anuskuss commented Dec 26, 2022 •
edited
Loading

I can't get it built unfortunately:

make[2]: *** No rule to make target 'ksmbd_spnego_negtokeninit.asn1.c', needed by 'ksmbd_spnego_negtokeninit.asn1.o'. Stop.

Edit: I can build it just fine on my desktop (Arch) but I can't insert that module into my server (Debian):

systemd[1]: Starting ksmbd userspace daemon...
kernel: ksmbd: module_layout: kernel tainted.
kernel: ksmbd: version magic '6.0.0-arch1-1 SMP preempt mod_unload ' should be '6.0.0-0.deb11.2-amd64 SMP preempt mod_unload modversions '
modprobe[481]: modprobe: ERROR: could not insert 'ksmbd': Exec format error
systemd[1]: Finished ksmbd userspace daemon.

Any idea what that build error is?

@Anuskuss
Copy link
Author

Okay so I went back to compiling the kernel again and I got it to work.

sudo apt install -t bullseye-backports linux-source-6.0 linux-headers-6.0.0-0.deb11.2-amd64 git
tar xf /usr/src/linux-source-6.0.tar.xz
cd linux-source-6.0
rm -r fs/ksmbd
git clone https://github.com/namjaejeon/ksmbd fs/ksmbd
patch -p1 -d fs/ksmbd/ < ../KalelCooper.diff
cp /boot/config-6.0.0-0.deb11.2-amd64 .config
cp /usr/src/linux-headers-6.0.0-0.deb11.2-amd64/Module.symvers .
echo CONFIG_SMB_INSECURE_SERVER=y >> .config
make oldconfig scripts prepare modules_prepare
make M=fs/ksmbd
sudo make -C fs/ksmbd install

But it's not great. The symlink target works now but it doesn't let me know that it's a symlink. New symlinks create some custom XSym files.
I also tried going the SMB1 route via CONFIG_SMB_INSECURE_SERVER but unfortunately there's no unix extension yet (only the newer posix).

@namjaejeon Thoughts?

@namjaejeon
Copy link
Owner

@Anuskuss See the reason why symlink is removed. cifsd-team#562 (comment)
And I don't have the motivation to work on symlink support in a hurry. How are you going to utilize ksmbd ?

@Anuskuss
Copy link
Author

In my small home network with mostly Windows desktops. I use symlinks a lot and I always need to know if I'm working with a real file or not. It's also convenient because it allows me to create symlinks without having to SSH into my server.

Would you consider implementing unix extensions? It's only available in SMB1 and that's gated behind CONFIG_SMB_INSECURE_SERVER anyway which pretty clearly signals that it's potentially unsafe. Well, thinking about it, maybe unix support is not worth it because that's already in Samba. I was only really interested in ksmbd because of the posix support, so maybe my opinion doesn't matter here.

@namjaejeon
Copy link
Owner

@Anuskuss Ah, You are still using SMB1 ? I don't want to work SMB1 anymore. It is being deprecated, in even windows... Beside security issues. It is no meaningful to invest the time for this.

@Anuskuss
Copy link
Author

I would use SMB3 but neither ksmbd nor Samba (fully) support posix yet. So yeah, I'll probably stay on Samba's vers=1.0,unix until any SMB server gets support for vers=3.1.1,posix.

@namjaejeon
Copy link
Owner

@Anuskuss Okay, I will work symlink support on only SMB2/3, not SMB1. If It is complete, I will inform you.

@Anuskuss
Copy link
Author

@namjaejeon Since you seem to be willing to listen to my problems, I decided to open another issue about the other thing that kinda bothers me. Feel free to close of course.

@Anuskuss Anuskuss reopened this Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Anuskuss @namjaejeon @KalelCooper