2022-04-11 18:35:23

by Jacky Li

[permalink] [raw]
Subject: [PATCH] crypto: ccp - Fix the INIT_EX data file open failure

There are 2 common cases when INIT_EX data file might not be
opened successfully and fail the sev initialization:

1. In user namespaces, normal user tasks (e.g. VMM) can change their
current->fs->root to point to arbitrary directories. While
init_ex_path is provided as a module param related to root file
system. Solution: use the root directory of init_task to avoid
accessing the wrong file.

2. Normal user tasks (e.g. VMM) don't have the privilege to access
the INIT_EX data file. Solution: open the file as root and
restore permissions immediately.

Signed-off-by: Jacky Li <[email protected]>
---
drivers/crypto/ccp/sev-dev.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 6ab93dfd478a..3aefb177715e 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -23,6 +23,7 @@
#include <linux/gfp.h>
#include <linux/cpufeature.h>
#include <linux/fs.h>
+#include <linux/fs_struct.h>

#include <asm/smp.h>

@@ -170,6 +171,31 @@ static void *sev_fw_alloc(unsigned long len)
return page_address(page);
}

+static struct file *open_file_as_root(const char *filename, int flags, umode_t mode)
+{
+ struct file *fp;
+ struct path root;
+ struct cred *cred;
+ const struct cred *old_cred;
+
+ task_lock(&init_task);
+ get_fs_root(init_task.fs, &root);
+ task_unlock(&init_task);
+
+ cred = prepare_creds();
+ if (!cred)
+ return ERR_PTR(-ENOMEM);
+ cred->fsuid = GLOBAL_ROOT_UID;
+ old_cred = override_creds(cred);
+
+ fp = file_open_root(&root, filename, flags, mode);
+ path_put(&root);
+
+ revert_creds(old_cred);
+
+ return fp;
+}
+
static int sev_read_init_ex_file(void)
{
struct sev_device *sev = psp_master->sev_data;
@@ -181,7 +207,7 @@ static int sev_read_init_ex_file(void)
if (!sev_init_ex_buffer)
return -EOPNOTSUPP;

- fp = filp_open(init_ex_path, O_RDONLY, 0);
+ fp = open_file_as_root(init_ex_path, O_RDONLY, 0);
if (IS_ERR(fp)) {
int ret = PTR_ERR(fp);

@@ -217,7 +243,7 @@ static void sev_write_init_ex_file(void)
if (!sev_init_ex_buffer)
return;

- fp = filp_open(init_ex_path, O_CREAT | O_WRONLY, 0600);
+ fp = open_file_as_root(init_ex_path, O_CREAT | O_WRONLY, 0600);
if (IS_ERR(fp)) {
dev_err(sev->dev,
"SEV: could not open file for write, error %ld\n",
--
2.35.1.1178.g4f1659d476-goog


2022-04-12 01:03:05

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix the INIT_EX data file open failure

On 4/11/22 13:00, Jacky Li wrote:
> There are 2 common cases when INIT_EX data file might not be
> opened successfully and fail the sev initialization:
>
> 1. In user namespaces, normal user tasks (e.g. VMM) can change their
> current->fs->root to point to arbitrary directories. While
> init_ex_path is provided as a module param related to root file
> system. Solution: use the root directory of init_task to avoid
> accessing the wrong file.
>
> 2. Normal user tasks (e.g. VMM) don't have the privilege to access
> the INIT_EX data file. Solution: open the file as root and
> restore permissions immediately.
>
> Signed-off-by: Jacky Li <[email protected]>

Should this have a Fixes: tag?

Thanks,
Tom

> ---
> drivers/crypto/ccp/sev-dev.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 6ab93dfd478a..3aefb177715e 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -23,6 +23,7 @@
> #include <linux/gfp.h>
> #include <linux/cpufeature.h>
> #include <linux/fs.h>
> +#include <linux/fs_struct.h>
>
> #include <asm/smp.h>
>
> @@ -170,6 +171,31 @@ static void *sev_fw_alloc(unsigned long len)
> return page_address(page);
> }
>
> +static struct file *open_file_as_root(const char *filename, int flags, umode_t mode)
> +{
> + struct file *fp;
> + struct path root;
> + struct cred *cred;
> + const struct cred *old_cred;
> +
> + task_lock(&init_task);
> + get_fs_root(init_task.fs, &root);
> + task_unlock(&init_task);
> +
> + cred = prepare_creds();
> + if (!cred)
> + return ERR_PTR(-ENOMEM);
> + cred->fsuid = GLOBAL_ROOT_UID;
> + old_cred = override_creds(cred);
> +
> + fp = file_open_root(&root, filename, flags, mode);
> + path_put(&root);
> +
> + revert_creds(old_cred);
> +
> + return fp;
> +}
> +
> static int sev_read_init_ex_file(void)
> {
> struct sev_device *sev = psp_master->sev_data;
> @@ -181,7 +207,7 @@ static int sev_read_init_ex_file(void)
> if (!sev_init_ex_buffer)
> return -EOPNOTSUPP;
>
> - fp = filp_open(init_ex_path, O_RDONLY, 0);
> + fp = open_file_as_root(init_ex_path, O_RDONLY, 0);
> if (IS_ERR(fp)) {
> int ret = PTR_ERR(fp);
>
> @@ -217,7 +243,7 @@ static void sev_write_init_ex_file(void)
> if (!sev_init_ex_buffer)
> return;
>
> - fp = filp_open(init_ex_path, O_CREAT | O_WRONLY, 0600);
> + fp = open_file_as_root(init_ex_path, O_CREAT | O_WRONLY, 0600);
> if (IS_ERR(fp)) {
> dev_err(sev->dev,
> "SEV: could not open file for write, error %ld\n",

2022-04-12 04:30:06

by Jacky Li

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix the INIT_EX data file open failure

On Mon, Apr 11, 2022 at 11:58 AM Tom Lendacky <[email protected]> wrote:
>
> On 4/11/22 13:00, Jacky Li wrote:
> > There are 2 common cases when INIT_EX data file might not be
> > opened successfully and fail the sev initialization:
> >
> > 1. In user namespaces, normal user tasks (e.g. VMM) can change their
> > current->fs->root to point to arbitrary directories. While
> > init_ex_path is provided as a module param related to root file
> > system. Solution: use the root directory of init_task to avoid
> > accessing the wrong file.
> >
> > 2. Normal user tasks (e.g. VMM) don't have the privilege to access
> > the INIT_EX data file. Solution: open the file as root and
> > restore permissions immediately.
> >
> > Signed-off-by: Jacky Li <[email protected]>
>
> Should this have a Fixes: tag?

Yes you are right, will add the Fixes: tag in v2.

Thanks,
Jacky

>
> Thanks,
> Tom