Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH] crypto: ccp - Fix the INIT_EX data file open failure
@ 2022-04-11 18:00 Jacky Li
  2022-04-11 18:58 ` Tom Lendacky
  2022-04-13 15:40 ` Peter Gonda
  0 siblings, 2 replies; 4+ messages in thread
From: Jacky Li @ 2022-04-11 18:00 UTC (permalink / raw)
  To: Brijesh Singh, Tom Lendacky, John Allen
  Cc: Herbert Xu, David S. Miller, Marc Orr, Alper Gun, Peter Gonda,
	linux-crypto, linux-kernel, Jacky Li

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 <jackyli@google.com>
---
 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: ccp - Fix the INIT_EX data file open failure
  2022-04-11 18:00 [PATCH] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
@ 2022-04-11 18:58 ` Tom Lendacky
  2022-04-11 19:35   ` Jacky Li
  2022-04-13 15:40 ` Peter Gonda
  1 sibling, 1 reply; 4+ messages in thread
From: Tom Lendacky @ 2022-04-11 18:58 UTC (permalink / raw)
  To: Jacky Li, Brijesh Singh, John Allen
  Cc: Herbert Xu, David S. Miller, Marc Orr, Alper Gun, Peter Gonda,
	linux-crypto, linux-kernel

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 <jackyli@google.com>

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",

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: ccp - Fix the INIT_EX data file open failure
  2022-04-11 18:58 ` Tom Lendacky
@ 2022-04-11 19:35   ` Jacky Li
  0 siblings, 0 replies; 4+ messages in thread
From: Jacky Li @ 2022-04-11 19:35 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: Brijesh Singh, John Allen, Herbert Xu, David S. Miller, Marc Orr,
	Alper Gun, Peter Gonda, linux-crypto, linux-kernel

On Mon, Apr 11, 2022 at 11:58 AM Tom Lendacky <thomas.lendacky@amd.com> 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 <jackyli@google.com>
>
> Should this have a Fixes: tag?

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

Thanks,
Jacky

>
> Thanks,
> Tom

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: ccp - Fix the INIT_EX data file open failure
  2022-04-11 18:00 [PATCH] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
  2022-04-11 18:58 ` Tom Lendacky
@ 2022-04-13 15:40 ` Peter Gonda
  1 sibling, 0 replies; 4+ messages in thread
From: Peter Gonda @ 2022-04-13 15:40 UTC (permalink / raw)
  To: Jacky Li
  Cc: Brijesh Singh, Tom Lendacky, John Allen, Herbert Xu,
	David S. Miller, Marc Orr, Alper Gun, Linux Crypto Mailing List,
	LKML

On Mon, Apr 11, 2022 at 12:00 PM Jacky Li <jackyli@google.com> 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 <jackyli@google.com>

Reviewed-by: Peter Gonda <pgonda@google.com>

Agreed about the fixes tag.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-13 15:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-11 18:00 [PATCH] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
2022-04-11 18:58 ` Tom Lendacky
2022-04-11 19:35   ` Jacky Li
2022-04-13 15:40 ` Peter Gonda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox