Symlinks

[Anuskuss]

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]

On Sun, Dec 25, 2022 at 9:35 AM Anuskuss @.***> wrote:

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?

This is a patch that I made symlink merely work again. But I'm not familiar with kernel nor the ksmbd. So, use at your own risk.

[KalelCooper]

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]

@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](https://patchwork.kernel.org/project/cifs-client/patch/20210920065613.5506-1-linkinjeon@kernel.org/#24458797) 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]

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]

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]

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]

@Anuskuss See the reason why symlink is removed. https://github.com/cifsd-team/ksmbd/issues/562#issuecomment-1161370961 And I don't have the motivation to work on symlink support in a hurry. How are you going to utilize ksmbd ?

[Anuskuss]

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]

@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]

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]

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

[Anuskuss]

@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.