* [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure
@ 2022-04-14 16:23 Jacky Li
2022-04-15 13:49 ` Tom Lendacky
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jacky Li @ 2022-04-14 16:23 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.
Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support")
Signed-off-by: Jacky Li <jackyli@google.com>
Reviewed-by: Peter Gonda <pgonda@google.com>
---
Changelog since v1:
- Added Fixes tag and Reviewed-By tag.
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.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure
2022-04-14 16:23 [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
@ 2022-04-15 13:49 ` Tom Lendacky
2022-04-15 16:20 ` Peter Gonda
2022-04-21 13:26 ` Tom Lendacky
2022-04-29 5:48 ` Herbert Xu
2 siblings, 1 reply; 6+ messages in thread
From: Tom Lendacky @ 2022-04-15 13:49 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/14/22 11:23, 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.
>
> Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support")
> Signed-off-by: Jacky Li <jackyli@google.com>
> Reviewed-by: Peter Gonda <pgonda@google.com>
Looks good, just a quick question. Should there be any type of access
checks before switching credentials? Should we check access to /dev/sev or
such? Or is the capability to load the module enough?
Thanks,
Tom
> ---
> Changelog since v1:
> - Added Fixes tag and Reviewed-By tag.
>
> drivers/crypto/ccp/sev-dev.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure
2022-04-15 13:49 ` Tom Lendacky
@ 2022-04-15 16:20 ` Peter Gonda
2022-04-21 13:26 ` Tom Lendacky
0 siblings, 1 reply; 6+ messages in thread
From: Peter Gonda @ 2022-04-15 16:20 UTC (permalink / raw)
To: Tom Lendacky
Cc: Jacky Li, Brijesh Singh, John Allen, Herbert Xu, David S. Miller,
Marc Orr, Alper Gun, Linux Crypto Mailing List, LKML
On Fri, Apr 15, 2022 at 7:49 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 4/14/22 11:23, 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.
> >
> > Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support")
> > Signed-off-by: Jacky Li <jackyli@google.com>
> > Reviewed-by: Peter Gonda <pgonda@google.com>
>
> Looks good, just a quick question. Should there be any type of access
> checks before switching credentials? Should we check access to /dev/sev or
> such? Or is the capability to load the module enough?
I thought this was fine because regardless of if an admin sets
psp_init_on_probe=true or false, their intention is that people who
have rw access to /dev/sev can use the commands which require the PSP
to be init. In the case of psp_init_on_probe=false only rw users can
cause the file to be created. The case of psp_init_on_probe=true seems
a little less clear to me but if a user can modprobe ccp that seems
like sufficient privilege to create the file. What do you think, Tom?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure
2022-04-15 16:20 ` Peter Gonda
@ 2022-04-21 13:26 ` Tom Lendacky
0 siblings, 0 replies; 6+ messages in thread
From: Tom Lendacky @ 2022-04-21 13:26 UTC (permalink / raw)
To: Peter Gonda
Cc: Jacky Li, Brijesh Singh, John Allen, Herbert Xu, David S. Miller,
Marc Orr, Alper Gun, Linux Crypto Mailing List, LKML
On 4/15/22 11:20, Peter Gonda wrote:
> On Fri, Apr 15, 2022 at 7:49 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> On 4/14/22 11:23, 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.
>>>
>>> Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support")
>>> Signed-off-by: Jacky Li <jackyli@google.com>
>>> Reviewed-by: Peter Gonda <pgonda@google.com>
>>
>> Looks good, just a quick question. Should there be any type of access
>> checks before switching credentials? Should we check access to /dev/sev or
>> such? Or is the capability to load the module enough?
>
> I thought this was fine because regardless of if an admin sets
> psp_init_on_probe=true or false, their intention is that people who
> have rw access to /dev/sev can use the commands which require the PSP
> to be init. In the case of psp_init_on_probe=false only rw users can
> cause the file to be created. The case of psp_init_on_probe=true seems
> a little less clear to me but if a user can modprobe ccp that seems
> like sufficient privilege to create the file. What do you think, Tom?
Sorry, lost this in my Inbox... That seems reasonable to me, let me add
my ack to the first email.
Thanks,
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure
2022-04-14 16:23 [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
2022-04-15 13:49 ` Tom Lendacky
@ 2022-04-21 13:26 ` Tom Lendacky
2022-04-29 5:48 ` Herbert Xu
2 siblings, 0 replies; 6+ messages in thread
From: Tom Lendacky @ 2022-04-21 13:26 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/14/22 11:23, 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.
>
> Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support")
> Signed-off-by: Jacky Li <jackyli@google.com>
> Reviewed-by: Peter Gonda <pgonda@google.com>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> Changelog since v1:
> - Added Fixes tag and Reviewed-By tag.
>
> drivers/crypto/ccp/sev-dev.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure
2022-04-14 16:23 [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
2022-04-15 13:49 ` Tom Lendacky
2022-04-21 13:26 ` Tom Lendacky
@ 2022-04-29 5:48 ` Herbert Xu
2 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2022-04-29 5:48 UTC (permalink / raw)
To: Jacky Li
Cc: Brijesh Singh, Tom Lendacky, John Allen, David S. Miller,
Marc Orr, Alper Gun, Peter Gonda, linux-crypto, linux-kernel
On Thu, Apr 14, 2022 at 04:23:25PM +0000, 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.
>
> Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support")
> Signed-off-by: Jacky Li <jackyli@google.com>
> Reviewed-by: Peter Gonda <pgonda@google.com>
> ---
> Changelog since v1:
> - Added Fixes tag and Reviewed-By tag.
>
> drivers/crypto/ccp/sev-dev.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-29 5:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-14 16:23 [PATCH v2] crypto: ccp - Fix the INIT_EX data file open failure Jacky Li
2022-04-15 13:49 ` Tom Lendacky
2022-04-15 16:20 ` Peter Gonda
2022-04-21 13:26 ` Tom Lendacky
2022-04-21 13:26 ` Tom Lendacky
2022-04-29 5:48 ` Herbert Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.