diff mbox series

ksmbd: remove follow symlinks support

Message ID 20210920065613.5506-1-linkinjeon@kernel.org (mailing list archive)
State New, archived
Headers show
Series ksmbd: remove follow symlinks support | expand

Commit Message

Namjae Jeon Sept. 20, 2021, 6:56 a.m. UTC
This patch remove symlink support that can be vulnerable, and we
re-implement it as reparse point later.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2pdu.c | 55 ++++++++++------------------------------------
 fs/ksmbd/vfs.c     | 50 +++++++++--------------------------------
 2 files changed, 21 insertions(+), 84 deletions(-)

Comments

Ralph Boehme Sept. 20, 2021, 1:57 p.m. UTC | #1
Hi Namjae,

Am 20.09.21 um 08:56 schrieb Namjae Jeon:
> This patch remove symlink support that can be vulnerable, and we
> re-implement it as reparse point later.
> 
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Ralph Böhme <slow@samba.org>
> Cc: Steve French <smfrench@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>   fs/ksmbd/smb2pdu.c | 55 ++++++++++------------------------------------
>   fs/ksmbd/vfs.c     | 50 +++++++++--------------------------------
>   2 files changed, 21 insertions(+), 84 deletions(-)
> 
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index afc508c2656d..911c5e97678d 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -2661,17 +2661,7 @@ int smb2_open(struct ksmbd_work *work)
>   	}
>   
>   	if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) {
> -		if (test_share_config_flag(work->tcon->share_conf,
> -					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS)) {
> -			/*
> -			 * On delete request, instead of following up, need to
> -			 * look the current entity
> -			 */
> -			rc = ksmbd_vfs_kern_path(name, 0, &path, 1);
> -		} else {
> -			rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
> -		}
> -
> +		rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
>   		if (!rc) {
>   			/*
>   			 * If file exists with under flags, return access
> @@ -2693,24 +2683,11 @@ int smb2_open(struct ksmbd_work *work)
>   			}
>   		}
>   	} else {
> -		if (test_share_config_flag(work->tcon->share_conf,
> -					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS)) {
> -			/*
> -			 * Use LOOKUP_FOLLOW to follow the path of
> -			 * symlink in path buildup
> -			 */
> -			rc = ksmbd_vfs_kern_path(name, LOOKUP_FOLLOW, &path, 1);
> -			if (rc) { /* Case for broken link ?*/
> -				rc = ksmbd_vfs_kern_path(name, 0, &path, 1);
> -			}
> -		} else {
> -			rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS,
> -						 &path, 1);
> -			if (!rc && d_is_symlink(path.dentry)) {
> -				rc = -EACCES;
> -				path_put(&path);
> -				goto err_out;
> -			}
> +		rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
> +		if (!rc && d_is_symlink(path.dentry)) {
> +			rc = -EACCES;
> +			path_put(&path);
> +			goto err_out;

why the the check for d_is_symlink()? Wouldn't ksmbd_vfs_kern_path() 
just return -ELOOP if a path component is a symlink? Hence I guess the 
below if (rc) where we handle EACCESS should be expanded to handle ELOOP.

I guess I would also refactor the

if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)

logic in a previous commit to change the codeflow so there's only one 
call to ksmbd_vfs_kern_path().

>   		}
>   	}
>   
> @@ -4795,12 +4772,8 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work,
>   	struct path path;
>   	int rc = 0, len;
>   	int fs_infoclass_size = 0;
> -	int lookup_flags = LOOKUP_NO_SYMLINKS;
>   
> -	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
> -		lookup_flags = LOOKUP_FOLLOW;
> -
> -	rc = ksmbd_vfs_kern_path(share->path, lookup_flags, &path, 0);
> +	rc = ksmbd_vfs_kern_path(share->path, LOOKUP_NO_SYMLINKS, &path, 0);
>   	if (rc) {
>   		pr_err("cannot create vfs path\n");
>   		return -EIO;

why doesn't this return rc?

> @@ -5307,7 +5280,7 @@ static int smb2_rename(struct ksmbd_work *work,
>   	char *pathname = NULL;
>   	struct path path;
>   	bool file_present = true;
> -	int rc, lookup_flags = LOOKUP_NO_SYMLINKS;
> +	int rc;
>   
>   	ksmbd_debug(SMB, "setting FILE_RENAME_INFO\n");
>   	pathname = kmalloc(PATH_MAX, GFP_KERNEL);
> @@ -5376,11 +5349,8 @@ static int smb2_rename(struct ksmbd_work *work,
>   		goto out;
>   	}
>   
> -	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
> -		lookup_flags = LOOKUP_FOLLOW;
> -
>   	ksmbd_debug(SMB, "new name %s\n", new_name);
> -	rc = ksmbd_vfs_kern_path(new_name, lookup_flags, &path, 1);
> +	rc = ksmbd_vfs_kern_path(new_name, LOOKUP_NO_SYMLINKS, &path, 1);
>   	if (rc)
>   		file_present = false;

I guess this should handle ELOOP?

>   	else
> @@ -5430,7 +5400,7 @@ static int smb2_create_link(struct ksmbd_work *work,
>   	char *link_name = NULL, *target_name = NULL, *pathname = NULL;
>   	struct path path;
>   	bool file_present = true;
> -	int rc, lookup_flags = LOOKUP_NO_SYMLINKS;
> +	int rc;
>   
>   	if (buf_len < sizeof(struct smb2_file_link_info) +
>   			le32_to_cpu(file_info->FileNameLength))
> @@ -5457,11 +5427,8 @@ static int smb2_create_link(struct ksmbd_work *work,
>   		goto out;
>   	}
>   
> -	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
> -		lookup_flags = LOOKUP_FOLLOW;
> -
>   	ksmbd_debug(SMB, "target name is %s\n", target_name);
> -	rc = ksmbd_vfs_kern_path(link_name, lookup_flags, &path, 0);
> +	rc = ksmbd_vfs_kern_path(link_name, LOOKUP_NO_SYMLINKS, &path, 0);
>   	if (rc)
>   		file_present = false;

same here?

Other then that: lgtm. Oh, besides, guess this needs an accomanying 
change to ksmbd-tools?

Cheers!
-slow
Ralph Boehme Sept. 20, 2021, 2:44 p.m. UTC | #2
Am 19.09.21 um 04:13 schrieb Namjae Jeon:
> Use  LOOKUP_NO_SYMLINKS flags for default lookup to prohibit the
> middle of symlink component lookup.

maybe this patch should be squashed with the "ksmbd: remove follow
symlinks support" patch?

-slow
Steve French Sept. 20, 2021, 3:19 p.m. UTC | #3
On Mon, Sep 20, 2021 at 9:44 AM Ralph Boehme <slow@samba.org> wrote:
>
> Am 19.09.21 um 04:13 schrieb Namjae Jeon:
> > Use  LOOKUP_NO_SYMLINKS flags for default lookup to prohibit the
> > middle of symlink component lookup.
>
> maybe this patch should be squashed with the "ksmbd: remove follow
> symlinks support" patch?

These two could be merged if it makes review easier or less likely to
cause merge conflicts later (in this case that may be true since they
both touch smb2_open), but that assumes that removing ability to
follow all symlinks is agreed upon.

Removing the ability to follow symlinks may be preferable, but I can
imagine cases where the admin is exporting only via SMB3 or only read
only where symlinks could be of value inside a share and safe (if
remote users can't create symlinks outside the share).   I don't have
a strong opinion but also can imagine cases where symlinks could be
required (share exported by both nfs and smb3 e.g.) but obviously
checked to avoid escaping from the share.
Ralph Boehme Sept. 20, 2021, 3:55 p.m. UTC | #4
Am 20.09.21 um 17:19 schrieb Steve French:
> On Mon, Sep 20, 2021 at 9:44 AM Ralph Boehme <slow@samba.org> wrote:
>> 
>> Am 19.09.21 um 04:13 schrieb Namjae Jeon:
>>> Use  LOOKUP_NO_SYMLINKS flags for default lookup to prohibit the 
>>> middle of symlink component lookup.
>> 
>> maybe this patch should be squashed with the "ksmbd: remove follow 
>> symlinks support" patch?
> 
> These two could be merged if it makes review easier or less likely
> to cause merge conflicts later (in this case that may be true since
> they both touch smb2_open),

from a high level perspective having both patches in the history is at
least confusing and should be avoided.

The first patch changes the semantics of "follow symlinks" and the
second one then changes it again by basically removing the option,
enforcing "never follow symlinks" behaviour.

> but that assumes that removing ability to follow all symlinks is
> agreed upon.

Well, as discussed you could use LOOKUP_BENEATH. The only downside would
be that symlinks with absolute paths are not allowed.

Note that with the current WIP patches we either

a) don't support symlink at all ("follow symlinks = yes")

b) have no protection against follow symlinks outside of a configured
share ("follow symlinks = no")

-slow
Namjae Jeon Sept. 20, 2021, 3:57 p.m. UTC | #5
2021-09-20 22:57 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Hi Namjae,
>
> Am 20.09.21 um 08:56 schrieb Namjae Jeon:
>> This patch remove symlink support that can be vulnerable, and we
>> re-implement it as reparse point later.
>>
>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> Cc: Ralph Böhme <slow@samba.org>
>> Cc: Steve French <smfrench@gmail.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   fs/ksmbd/smb2pdu.c | 55 ++++++++++------------------------------------
>>   fs/ksmbd/vfs.c     | 50 +++++++++--------------------------------
>>   2 files changed, 21 insertions(+), 84 deletions(-)
>>
>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> index afc508c2656d..911c5e97678d 100644
>> --- a/fs/ksmbd/smb2pdu.c
>> +++ b/fs/ksmbd/smb2pdu.c
>> @@ -2661,17 +2661,7 @@ int smb2_open(struct ksmbd_work *work)
>>   	}
>>
>>   	if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) {
>> -		if (test_share_config_flag(work->tcon->share_conf,
>> -					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS)) {
>> -			/*
>> -			 * On delete request, instead of following up, need to
>> -			 * look the current entity
>> -			 */
>> -			rc = ksmbd_vfs_kern_path(name, 0, &path, 1);
>> -		} else {
>> -			rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
>> -		}
>> -
>> +		rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
>>   		if (!rc) {
>>   			/*
>>   			 * If file exists with under flags, return access
>> @@ -2693,24 +2683,11 @@ int smb2_open(struct ksmbd_work *work)
>>   			}
>>   		}
>>   	} else {
>> -		if (test_share_config_flag(work->tcon->share_conf,
>> -					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS)) {
>> -			/*
>> -			 * Use LOOKUP_FOLLOW to follow the path of
>> -			 * symlink in path buildup
>> -			 */
>> -			rc = ksmbd_vfs_kern_path(name, LOOKUP_FOLLOW, &path, 1);
>> -			if (rc) { /* Case for broken link ?*/
>> -				rc = ksmbd_vfs_kern_path(name, 0, &path, 1);
>> -			}
>> -		} else {
>> -			rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS,
>> -						 &path, 1);
>> -			if (!rc && d_is_symlink(path.dentry)) {
>> -				rc = -EACCES;
>> -				path_put(&path);
>> -				goto err_out;
>> -			}
>> +		rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
>> +		if (!rc && d_is_symlink(path.dentry)) {
>> +			rc = -EACCES;
>> +			path_put(&path);
>> +			goto err_out;
>
> why the the check for d_is_symlink()? Wouldn't ksmbd_vfs_kern_path()
> just return -ELOOP if a path component is a symlink? Hence I guess the
> below if (rc) where we handle EACCESS should be expanded to handle ELOOP.
ksmbd_vfs_kern_path() doesn't return -ELOOP if last component is a
symlink. So need to check it using d_is_symlink().
>
> I guess I would also refactor the
>
> if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)
>
> logic in a previous commit to change the codeflow so there's only one
> call to ksmbd_vfs_kern_path().
Right. Will do it on v2.
>
>>   		}
>>   	}
>>
>> @@ -4795,12 +4772,8 @@ static int smb2_get_info_filesystem(struct
>> ksmbd_work *work,
>>   	struct path path;
>>   	int rc = 0, len;
>>   	int fs_infoclass_size = 0;
>> -	int lookup_flags = LOOKUP_NO_SYMLINKS;
>>
>> -	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
>> -		lookup_flags = LOOKUP_FOLLOW;
>> -
>> -	rc = ksmbd_vfs_kern_path(share->path, lookup_flags, &path, 0);
>> +	rc = ksmbd_vfs_kern_path(share->path, LOOKUP_NO_SYMLINKS, &path, 0);
>>   	if (rc) {
>>   		pr_err("cannot create vfs path\n");
>>   		return -EIO;
>
> why doesn't this return rc?
This is not related with this patch. If needed, we can fix it on another patch.
As I remember, To return STATUS_UNEXPECTED_IO_ERROR error?
>
>> @@ -5307,7 +5280,7 @@ static int smb2_rename(struct ksmbd_work *work,
>>   	char *pathname = NULL;
>>   	struct path path;
>>   	bool file_present = true;
>> -	int rc, lookup_flags = LOOKUP_NO_SYMLINKS;
>> +	int rc;
>>
>>   	ksmbd_debug(SMB, "setting FILE_RENAME_INFO\n");
>>   	pathname = kmalloc(PATH_MAX, GFP_KERNEL);
>> @@ -5376,11 +5349,8 @@ static int smb2_rename(struct ksmbd_work *work,
>>   		goto out;
>>   	}
>>
>> -	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
>> -		lookup_flags = LOOKUP_FOLLOW;
>> -
>>   	ksmbd_debug(SMB, "new name %s\n", new_name);
>> -	rc = ksmbd_vfs_kern_path(new_name, lookup_flags, &path, 1);
>> +	rc = ksmbd_vfs_kern_path(new_name, LOOKUP_NO_SYMLINKS, &path, 1);
>>   	if (rc)
>>   		file_present = false;
>
> I guess this should handle ELOOP?
I have answered above. ksmbd_vfs_kern_path doesn't return it.
>
>>   	else
>> @@ -5430,7 +5400,7 @@ static int smb2_create_link(struct ksmbd_work
>> *work,
>>   	char *link_name = NULL, *target_name = NULL, *pathname = NULL;
>>   	struct path path;
>>   	bool file_present = true;
>> -	int rc, lookup_flags = LOOKUP_NO_SYMLINKS;
>> +	int rc;
>>
>>   	if (buf_len < sizeof(struct smb2_file_link_info) +
>>   			le32_to_cpu(file_info->FileNameLength))
>> @@ -5457,11 +5427,8 @@ static int smb2_create_link(struct ksmbd_work
>> *work,
>>   		goto out;
>>   	}
>>
>> -	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
>> -		lookup_flags = LOOKUP_FOLLOW;
>> -
>>   	ksmbd_debug(SMB, "target name is %s\n", target_name);
>> -	rc = ksmbd_vfs_kern_path(link_name, lookup_flags, &path, 0);
>> +	rc = ksmbd_vfs_kern_path(link_name, LOOKUP_NO_SYMLINKS, &path, 0);
>>   	if (rc)
>>   		file_present = false;
>
> same here?
ditto.
>
> Other then that: lgtm. Oh, besides, guess this needs an accomanying
> change to ksmbd-tools?
Yes. It is needed, but I will change ksmbd-tools also after this patch
is applied.

Thanks for your review!
>
> Cheers!
> -slow
>
> --
> Ralph Boehme, Samba Team                 https://samba.org/
> SerNet Samba Team Lead      https://sernet.de/en/team-samba
>
>
Namjae Jeon Sept. 20, 2021, 4:03 p.m. UTC | #6
2021-09-20 23:44 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 19.09.21 um 04:13 schrieb Namjae Jeon:
>> Use  LOOKUP_NO_SYMLINKS flags for default lookup to prohibit the
>> middle of symlink component lookup.
>
> maybe this patch should be squashed with the "ksmbd: remove follow
> symlinks support" patch?
I think that the subject is slightly different.
1. prohibit the middle of symlinks component on path.
2. remove LOOKUP_FOLLOW and  "follow symlink" parameter check.

Thanks!
>
> -slow
>
> --
> Ralph Boehme, Samba Team                 https://samba.org/
> SerNet Samba Team Lead      https://sernet.de/en/team-samba
>
>
Ralph Boehme Sept. 20, 2021, 4:28 p.m. UTC | #7
Am 20.09.21 um 17:57 schrieb Namjae Jeon:
> ksmbd_vfs_kern_path() doesn't return -ELOOP if last component is a
> symlink. So need to check it using d_is_symlink().

Really? I missed that. Is that a behaviour of kern_path() when passing 
LOOKUP_NO_SYMLINKS? I don't see the behaviour expressed inside 
ksmbd_vfs_kern_path(), but maybe I'm looking at the wrong branch+patchset?

-slow
Namjae Jeon Sept. 20, 2021, 4:37 p.m. UTC | #8
2021-09-21 1:28 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 20.09.21 um 17:57 schrieb Namjae Jeon:
>> ksmbd_vfs_kern_path() doesn't return -ELOOP if last component is a
>> symlink. So need to check it using d_is_symlink().
>
> Really? I missed that. Is that a behaviour of kern_path() when passing
> LOOKUP_NO_SYMLINKS?
Yes.
> I don't see the behaviour expressed inside
> ksmbd_vfs_kern_path(), but maybe I'm looking at the wrong branch+patchset?
I think that you checked correct branch and patch-set.
>
> -slow
>
> --
> Ralph Boehme, Samba Team                 https://samba.org/
> SerNet Samba Team Lead      https://sernet.de/en/team-samba
>
>
Ralph Boehme Sept. 21, 2021, 7:44 a.m. UTC | #9
Am 20.09.21 um 18:37 schrieb Namjae Jeon:
> 2021-09-21 1:28 GMT+09:00, Ralph Boehme <slow@samba.org>:
>> Am 20.09.21 um 17:57 schrieb Namjae Jeon:
>>> ksmbd_vfs_kern_path() doesn't return -ELOOP if last component is a
>>> symlink. So need to check it using d_is_symlink().
>>
>> Really? I missed that. Is that a behaviour of kern_path() when passing
>> LOOKUP_NO_SYMLINKS?
> Yes.

d'oh! Got it. We're yet to make use of those flags in Samba, so I hadn't 
really wrapped my head fully around this to understand how it works in 
detail, so I messed up the semantics a bit. :)

-slow
diff mbox series

Patch

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index afc508c2656d..911c5e97678d 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -2661,17 +2661,7 @@  int smb2_open(struct ksmbd_work *work)
 	}
 
 	if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) {
-		if (test_share_config_flag(work->tcon->share_conf,
-					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS)) {
-			/*
-			 * On delete request, instead of following up, need to
-			 * look the current entity
-			 */
-			rc = ksmbd_vfs_kern_path(name, 0, &path, 1);
-		} else {
-			rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
-		}
-
+		rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
 		if (!rc) {
 			/*
 			 * If file exists with under flags, return access
@@ -2693,24 +2683,11 @@  int smb2_open(struct ksmbd_work *work)
 			}
 		}
 	} else {
-		if (test_share_config_flag(work->tcon->share_conf,
-					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS)) {
-			/*
-			 * Use LOOKUP_FOLLOW to follow the path of
-			 * symlink in path buildup
-			 */
-			rc = ksmbd_vfs_kern_path(name, LOOKUP_FOLLOW, &path, 1);
-			if (rc) { /* Case for broken link ?*/
-				rc = ksmbd_vfs_kern_path(name, 0, &path, 1);
-			}
-		} else {
-			rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS,
-						 &path, 1);
-			if (!rc && d_is_symlink(path.dentry)) {
-				rc = -EACCES;
-				path_put(&path);
-				goto err_out;
-			}
+		rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
+		if (!rc && d_is_symlink(path.dentry)) {
+			rc = -EACCES;
+			path_put(&path);
+			goto err_out;
 		}
 	}
 
@@ -4795,12 +4772,8 @@  static int smb2_get_info_filesystem(struct ksmbd_work *work,
 	struct path path;
 	int rc = 0, len;
 	int fs_infoclass_size = 0;
-	int lookup_flags = LOOKUP_NO_SYMLINKS;
 
-	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		lookup_flags = LOOKUP_FOLLOW;
-
-	rc = ksmbd_vfs_kern_path(share->path, lookup_flags, &path, 0);
+	rc = ksmbd_vfs_kern_path(share->path, LOOKUP_NO_SYMLINKS, &path, 0);
 	if (rc) {
 		pr_err("cannot create vfs path\n");
 		return -EIO;
@@ -5307,7 +5280,7 @@  static int smb2_rename(struct ksmbd_work *work,
 	char *pathname = NULL;
 	struct path path;
 	bool file_present = true;
-	int rc, lookup_flags = LOOKUP_NO_SYMLINKS;
+	int rc;
 
 	ksmbd_debug(SMB, "setting FILE_RENAME_INFO\n");
 	pathname = kmalloc(PATH_MAX, GFP_KERNEL);
@@ -5376,11 +5349,8 @@  static int smb2_rename(struct ksmbd_work *work,
 		goto out;
 	}
 
-	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		lookup_flags = LOOKUP_FOLLOW;
-
 	ksmbd_debug(SMB, "new name %s\n", new_name);
-	rc = ksmbd_vfs_kern_path(new_name, lookup_flags, &path, 1);
+	rc = ksmbd_vfs_kern_path(new_name, LOOKUP_NO_SYMLINKS, &path, 1);
 	if (rc)
 		file_present = false;
 	else
@@ -5430,7 +5400,7 @@  static int smb2_create_link(struct ksmbd_work *work,
 	char *link_name = NULL, *target_name = NULL, *pathname = NULL;
 	struct path path;
 	bool file_present = true;
-	int rc, lookup_flags = LOOKUP_NO_SYMLINKS;
+	int rc;
 
 	if (buf_len < sizeof(struct smb2_file_link_info) +
 			le32_to_cpu(file_info->FileNameLength))
@@ -5457,11 +5427,8 @@  static int smb2_create_link(struct ksmbd_work *work,
 		goto out;
 	}
 
-	if (test_share_config_flag(share, KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		lookup_flags = LOOKUP_FOLLOW;
-
 	ksmbd_debug(SMB, "target name is %s\n", target_name);
-	rc = ksmbd_vfs_kern_path(link_name, lookup_flags, &path, 0);
+	rc = ksmbd_vfs_kern_path(link_name, LOOKUP_NO_SYMLINKS, &path, 0);
 	if (rc)
 		file_present = false;
 	else
diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
index 53047f013371..3733e4944c1d 100644
--- a/fs/ksmbd/vfs.c
+++ b/fs/ksmbd/vfs.c
@@ -164,13 +164,9 @@  int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode)
 {
 	struct path path;
 	struct dentry *dentry;
-	int err, lookup_flags = LOOKUP_NO_SYMLINKS;
-
-	if (test_share_config_flag(work->tcon->share_conf,
-				   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		lookup_flags = LOOKUP_FOLLOW;
+	int err;
 
-	dentry = kern_path_create(AT_FDCWD, name, &path, lookup_flags);
+	dentry = kern_path_create(AT_FDCWD, name, &path, LOOKUP_NO_SYMLINKS);
 	if (IS_ERR(dentry)) {
 		err = PTR_ERR(dentry);
 		if (err != -ENOENT)
@@ -205,14 +201,10 @@  int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode)
 	struct user_namespace *user_ns;
 	struct path path;
 	struct dentry *dentry;
-	int err, lookup_flags = LOOKUP_NO_SYMLINKS;
-
-	if (test_share_config_flag(work->tcon->share_conf,
-				   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		lookup_flags = LOOKUP_FOLLOW;
+	int err;
 
 	dentry = kern_path_create(AT_FDCWD, name, &path,
-				  lookup_flags | LOOKUP_DIRECTORY);
+				  LOOKUP_NO_SYMLINKS | LOOKUP_DIRECTORY);
 	if (IS_ERR(dentry)) {
 		err = PTR_ERR(dentry);
 		if (err != -EEXIST)
@@ -597,16 +589,11 @@  int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name)
 	struct path path;
 	struct dentry *parent;
 	int err;
-	int flags = LOOKUP_NO_SYMLINKS;
 
 	if (ksmbd_override_fsids(work))
 		return -ENOMEM;
 
-	if (test_share_config_flag(work->tcon->share_conf,
-				   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		flags = LOOKUP_FOLLOW;
-
-	err = kern_path(name, flags, &path);
+	err = kern_path(name, LOOKUP_NO_SYMLINKS, &path);
 	if (err) {
 		ksmbd_debug(VFS, "can't get %s, err %d\n", name, err);
 		ksmbd_revert_fsids(work);
@@ -661,16 +648,11 @@  int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname,
 	struct path oldpath, newpath;
 	struct dentry *dentry;
 	int err;
-	int flags = LOOKUP_NO_SYMLINKS;
 
 	if (ksmbd_override_fsids(work))
 		return -ENOMEM;
 
-	if (test_share_config_flag(work->tcon->share_conf,
-				   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		flags = LOOKUP_FOLLOW;
-
-	err = kern_path(oldname, flags, &oldpath);
+	err = kern_path(oldname, LOOKUP_NO_SYMLINKS, &oldpath);
 	if (err) {
 		pr_err("cannot get linux path for %s, err = %d\n",
 		       oldname, err);
@@ -678,7 +660,7 @@  int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname,
 	}
 
 	dentry = kern_path_create(AT_FDCWD, newname, &newpath,
-				  flags | LOOKUP_REVAL);
+				  LOOKUP_NO_SYMLINKS | LOOKUP_REVAL);
 	if (IS_ERR(dentry)) {
 		err = PTR_ERR(dentry);
 		pr_err("path create err for %s, err %d\n", newname, err);
@@ -797,7 +779,6 @@  int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp,
 	struct dentry *src_dent, *trap_dent, *src_child;
 	char *dst_name;
 	int err;
-	int flags = LOOKUP_NO_SYMLINKS;
 
 	dst_name = extract_last_component(newname);
 	if (!dst_name)
@@ -806,13 +787,8 @@  int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp,
 	src_dent_parent = dget_parent(fp->filp->f_path.dentry);
 	src_dent = fp->filp->f_path.dentry;
 
-	flags = LOOKUP_DIRECTORY;
-	if (test_share_config_flag(work->tcon->share_conf,
-				   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-		flags = LOOKUP_FOLLOW;
-	flags |= LOOKUP_DIRECTORY;
-
-	err = kern_path(newname, flags, &dst_path);
+	err = kern_path(newname, LOOKUP_NO_SYMLINKS | LOOKUP_DIRECTORY,
+			&dst_path);
 	if (err) {
 		ksmbd_debug(VFS, "Cannot get path for %s [%d]\n", newname, err);
 		goto out;
@@ -871,13 +847,7 @@  int ksmbd_vfs_truncate(struct ksmbd_work *work, const char *name,
 	int err = 0;
 
 	if (name) {
-		int flags = LOOKUP_NO_SYMLINKS;
-
-		if (test_share_config_flag(work->tcon->share_conf,
-					   KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS))
-			flags = LOOKUP_FOLLOW;
-
-		err = kern_path(name, flags, &path);
+		err = kern_path(name, LOOKUP_NO_SYMLINKS, &path);
 		if (err) {
 			pr_err("cannot get linux path for %s, err %d\n",
 			       name, err);