From: Namjae Jeon <linkinjeon@kernel.org>
To: linux-cifs@vger.kernel.org
Cc: "Namjae Jeon" <linkinjeon@kernel.org>,
"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
"Ralph Böhme" <slow@samba.org>,
"Steve French" <smfrench@gmail.com>
Subject: [PATCH] ksmbd: remove follow symlinks support
Date: Mon, 20 Sep 2021 15:56:13 +0900 [thread overview]
Message-ID: <20210920065613.5506-1-linkinjeon@kernel.org> (raw)
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;
}
}
@@ -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);
--
2.25.1
next reply other threads:[~2021-09-20 6:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-20 6:56 Namjae Jeon [this message]
2021-09-20 13:57 ` [PATCH] ksmbd: remove follow symlinks support Ralph Boehme
2021-09-20 15:57 ` Namjae Jeon
2021-09-20 16:28 ` Ralph Boehme
2021-09-20 16:37 ` Namjae Jeon
2021-09-21 7:44 ` Ralph Boehme
2021-09-20 14:44 ` Ralph Boehme
2021-09-20 15:19 ` Steve French
2021-09-20 15:55 ` Ralph Boehme
2021-09-20 16:03 ` Namjae Jeon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210920065613.5506-1-linkinjeon@kernel.org \
--to=linkinjeon@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=ronniesahlberg@gmail.com \
--cc=slow@samba.org \
--cc=smfrench@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).