Skip to content

Commit 4ac39c5

Browse files
committed
Merge tag 'x86_urgent_for_v6.3_rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull x86 fixes from Borislav Petkov: "There's a little bit more 'movement' in there for my taste but it needs to happen and should make the code better after it. - Check cmdline_find_option()'s return value before further processing - Clear temporary storage in the resctrl code to prevent access to an unexistent MSR - Add a simple throttling mechanism to protect the hypervisor from potentially malicious SEV guests issuing requests in rapid succession. In order to not jeopardize the sanity of everyone involved in maintaining this code, the request issuing side has received a cleanup, split in more or less trivial, small and digestible pieces. Otherwise, the code was threatening to become an unmaintainable mess. Therefore, that cleanup is marked indirectly also for stable so that there's no differences between the upstream code and the stable variant when it comes down to backporting more there" * tag 'x86_urgent_for_v6.3_rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: x86/mm: Fix use of uninitialized buffer in sme_enable() x86/resctrl: Clear staged_config[] before and after it is used virt/coco/sev-guest: Add throttling awareness virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a switch-case virt/coco/sev-guest: Do some code style cleanups virt/coco/sev-guest: Carve out the request issuing logic into a helper virt/coco/sev-guest: Remove the disable_vmpck label in handle_guest_request() virt/coco/sev-guest: Simplify extended guest request handling virt/coco/sev-guest: Check SEV_SNP attribute at probe time
2 parents 995bba4 + cbebd68 commit 4ac39c5

File tree

7 files changed

+123
-70
lines changed

7 files changed

+123
-70
lines changed

arch/x86/include/asm/sev-common.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ struct snp_psc_desc {
128128
struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY];
129129
} __packed;
130130

131-
/* Guest message request error code */
131+
/* Guest message request error codes */
132132
#define SNP_GUEST_REQ_INVALID_LEN BIT_ULL(32)
133+
#define SNP_GUEST_REQ_ERR_BUSY BIT_ULL(33)
133134

134135
#define GHCB_MSR_TERM_REQ 0x100
135136
#define GHCB_MSR_TERM_REASON_SET_POS 12

arch/x86/kernel/cpu/resctrl/ctrlmondata.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,6 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
368368
{
369369
struct resctrl_schema *s;
370370
struct rdtgroup *rdtgrp;
371-
struct rdt_domain *dom;
372371
struct rdt_resource *r;
373372
char *tok, *resname;
374373
int ret = 0;
@@ -397,10 +396,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
397396
goto out;
398397
}
399398

400-
list_for_each_entry(s, &resctrl_schema_all, list) {
401-
list_for_each_entry(dom, &s->res->domains, list)
402-
memset(dom->staged_config, 0, sizeof(dom->staged_config));
403-
}
399+
rdt_staged_configs_clear();
404400

405401
while ((tok = strsep(&buf, "\n")) != NULL) {
406402
resname = strim(strsep(&tok, ":"));
@@ -445,6 +441,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
445441
}
446442

447443
out:
444+
rdt_staged_configs_clear();
448445
rdtgroup_kn_unlock(of->kn);
449446
cpus_read_unlock();
450447
return ret ?: nbytes;

arch/x86/kernel/cpu/resctrl/internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,5 +555,6 @@ void __check_limbo(struct rdt_domain *d, bool force_free);
555555
void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
556556
void __init thread_throttle_mode_init(void);
557557
void __init mbm_config_rftype_init(const char *config);
558+
void rdt_staged_configs_clear(void);
558559

559560
#endif /* _ASM_X86_RESCTRL_INTERNAL_H */

arch/x86/kernel/cpu/resctrl/rdtgroup.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,19 @@ void rdt_last_cmd_printf(const char *fmt, ...)
7878
va_end(ap);
7979
}
8080

81+
void rdt_staged_configs_clear(void)
82+
{
83+
struct rdt_resource *r;
84+
struct rdt_domain *dom;
85+
86+
lockdep_assert_held(&rdtgroup_mutex);
87+
88+
for_each_alloc_capable_rdt_resource(r) {
89+
list_for_each_entry(dom, &r->domains, list)
90+
memset(dom->staged_config, 0, sizeof(dom->staged_config));
91+
}
92+
}
93+
8194
/*
8295
* Trivial allocator for CLOSIDs. Since h/w only supports a small number,
8396
* we can keep a bitmap of free CLOSIDs in a single integer.
@@ -3107,7 +3120,9 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
31073120
{
31083121
struct resctrl_schema *s;
31093122
struct rdt_resource *r;
3110-
int ret;
3123+
int ret = 0;
3124+
3125+
rdt_staged_configs_clear();
31113126

31123127
list_for_each_entry(s, &resctrl_schema_all, list) {
31133128
r = s->res;
@@ -3119,20 +3134,22 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
31193134
} else {
31203135
ret = rdtgroup_init_cat(s, rdtgrp->closid);
31213136
if (ret < 0)
3122-
return ret;
3137+
goto out;
31233138
}
31243139

31253140
ret = resctrl_arch_update_domains(r, rdtgrp->closid);
31263141
if (ret < 0) {
31273142
rdt_last_cmd_puts("Failed to initialize allocations\n");
3128-
return ret;
3143+
goto out;
31293144
}
31303145

31313146
}
31323147

31333148
rdtgrp->mode = RDT_MODE_SHAREABLE;
31343149

3135-
return 0;
3150+
out:
3151+
rdt_staged_configs_clear();
3152+
return ret;
31363153
}
31373154

31383155
static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,

arch/x86/kernel/sev.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2183,9 +2183,6 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
21832183
struct ghcb *ghcb;
21842184
int ret;
21852185

2186-
if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
2187-
return -ENODEV;
2188-
21892186
if (!fw_err)
21902187
return -EINVAL;
21912188

@@ -2212,15 +2209,26 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
22122209
if (ret)
22132210
goto e_put;
22142211

2215-
if (ghcb->save.sw_exit_info_2) {
2216-
/* Number of expected pages are returned in RBX */
2217-
if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST &&
2218-
ghcb->save.sw_exit_info_2 == SNP_GUEST_REQ_INVALID_LEN)
2219-
input->data_npages = ghcb_get_rbx(ghcb);
2212+
*fw_err = ghcb->save.sw_exit_info_2;
2213+
switch (*fw_err) {
2214+
case 0:
2215+
break;
22202216

2221-
*fw_err = ghcb->save.sw_exit_info_2;
2217+
case SNP_GUEST_REQ_ERR_BUSY:
2218+
ret = -EAGAIN;
2219+
break;
22222220

2221+
case SNP_GUEST_REQ_INVALID_LEN:
2222+
/* Number of expected pages are returned in RBX */
2223+
if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
2224+
input->data_npages = ghcb_get_rbx(ghcb);
2225+
ret = -ENOSPC;
2226+
break;
2227+
}
2228+
fallthrough;
2229+
default:
22232230
ret = -EIO;
2231+
break;
22242232
}
22252233

22262234
e_put:

arch/x86/mm/mem_encrypt_identity.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,8 @@ void __init sme_enable(struct boot_params *bp)
600600
cmdline_ptr = (const char *)((u64)bp->hdr.cmd_line_ptr |
601601
((u64)bp->ext_cmd_line_ptr << 32));
602602

603-
cmdline_find_option(cmdline_ptr, cmdline_arg, buffer, sizeof(buffer));
603+
if (cmdline_find_option(cmdline_ptr, cmdline_arg, buffer, sizeof(buffer)) < 0)
604+
return;
604605

605606
if (!strncmp(buffer, cmdline_on, sizeof(buffer)))
606607
sme_me_mask = me_mask;

drivers/virt/coco/sev-guest/sev-guest.c

Lines changed: 78 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
#define AAD_LEN 48
3232
#define MSG_HDR_VER 1
3333

34+
#define SNP_REQ_MAX_RETRY_DURATION (60*HZ)
35+
#define SNP_REQ_RETRY_DELAY (2*HZ)
36+
3437
struct snp_guest_crypto {
3538
struct crypto_aead *tfm;
3639
u8 *iv, *authtag;
@@ -318,45 +321,39 @@ static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8
318321
return __enc_payload(snp_dev, req, payload, sz);
319322
}
320323

321-
static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
322-
u8 type, void *req_buf, size_t req_sz, void *resp_buf,
323-
u32 resp_sz, __u64 *fw_err)
324+
static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err)
324325
{
325-
unsigned long err;
326-
u64 seqno;
326+
unsigned long err = 0xff, override_err = 0;
327+
unsigned long req_start = jiffies;
328+
unsigned int override_npages = 0;
327329
int rc;
328330

329-
/* Get message sequence and verify that its a non-zero */
330-
seqno = snp_get_msg_seqno(snp_dev);
331-
if (!seqno)
332-
return -EIO;
333-
334-
memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
335-
336-
/* Encrypt the userspace provided payload */
337-
rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
338-
if (rc)
339-
return rc;
340-
331+
retry_request:
341332
/*
342333
* Call firmware to process the request. In this function the encrypted
343334
* message enters shared memory with the host. So after this call the
344335
* sequence number must be incremented or the VMPCK must be deleted to
345336
* prevent reuse of the IV.
346337
*/
347338
rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
339+
switch (rc) {
340+
case -ENOSPC:
341+
/*
342+
* If the extended guest request fails due to having too
343+
* small of a certificate data buffer, retry the same
344+
* guest request without the extended data request in
345+
* order to increment the sequence number and thus avoid
346+
* IV reuse.
347+
*/
348+
override_npages = snp_dev->input.data_npages;
349+
exit_code = SVM_VMGEXIT_GUEST_REQUEST;
348350

349-
/*
350-
* If the extended guest request fails due to having too small of a
351-
* certificate data buffer, retry the same guest request without the
352-
* extended data request in order to increment the sequence number
353-
* and thus avoid IV reuse.
354-
*/
355-
if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST &&
356-
err == SNP_GUEST_REQ_INVALID_LEN) {
357-
const unsigned int certs_npages = snp_dev->input.data_npages;
358-
359-
exit_code = SVM_VMGEXIT_GUEST_REQUEST;
351+
/*
352+
* Override the error to inform callers the given extended
353+
* request buffer size was too small and give the caller the
354+
* required buffer size.
355+
*/
356+
override_err = SNP_GUEST_REQ_INVALID_LEN;
360357

361358
/*
362359
* If this call to the firmware succeeds, the sequence number can
@@ -366,15 +363,20 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
366363
* of the VMPCK and the error code being propagated back to the
367364
* user as an ioctl() return code.
368365
*/
369-
rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
366+
goto retry_request;
370367

371-
/*
372-
* Override the error to inform callers the given extended
373-
* request buffer size was too small and give the caller the
374-
* required buffer size.
375-
*/
376-
err = SNP_GUEST_REQ_INVALID_LEN;
377-
snp_dev->input.data_npages = certs_npages;
368+
/*
369+
* The host may return SNP_GUEST_REQ_ERR_EBUSY if the request has been
370+
* throttled. Retry in the driver to avoid returning and reusing the
371+
* message sequence number on a different message.
372+
*/
373+
case -EAGAIN:
374+
if (jiffies - req_start > SNP_REQ_MAX_RETRY_DURATION) {
375+
rc = -ETIMEDOUT;
376+
break;
377+
}
378+
schedule_timeout_killable(SNP_REQ_RETRY_DELAY);
379+
goto retry_request;
378380
}
379381

380382
/*
@@ -386,37 +388,60 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
386388
snp_inc_msg_seqno(snp_dev);
387389

388390
if (fw_err)
389-
*fw_err = err;
391+
*fw_err = override_err ?: err;
392+
393+
if (override_npages)
394+
snp_dev->input.data_npages = override_npages;
390395

391396
/*
392397
* If an extended guest request was issued and the supplied certificate
393398
* buffer was not large enough, a standard guest request was issued to
394399
* prevent IV reuse. If the standard request was successful, return -EIO
395400
* back to the caller as would have originally been returned.
396401
*/
397-
if (!rc && err == SNP_GUEST_REQ_INVALID_LEN)
402+
if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN)
403+
return -EIO;
404+
405+
return rc;
406+
}
407+
408+
static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
409+
u8 type, void *req_buf, size_t req_sz, void *resp_buf,
410+
u32 resp_sz, __u64 *fw_err)
411+
{
412+
u64 seqno;
413+
int rc;
414+
415+
/* Get message sequence and verify that its a non-zero */
416+
seqno = snp_get_msg_seqno(snp_dev);
417+
if (!seqno)
398418
return -EIO;
399419

420+
memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
421+
422+
/* Encrypt the userspace provided payload */
423+
rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
424+
if (rc)
425+
return rc;
426+
427+
rc = __handle_guest_request(snp_dev, exit_code, fw_err);
400428
if (rc) {
401-
dev_alert(snp_dev->dev,
402-
"Detected error from ASP request. rc: %d, fw_err: %llu\n",
403-
rc, *fw_err);
404-
goto disable_vmpck;
429+
if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
430+
return rc;
431+
432+
dev_alert(snp_dev->dev, "Detected error from ASP request. rc: %d, fw_err: %llu\n", rc, *fw_err);
433+
snp_disable_vmpck(snp_dev);
434+
return rc;
405435
}
406436

407437
rc = verify_and_dec_payload(snp_dev, resp_buf, resp_sz);
408438
if (rc) {
409-
dev_alert(snp_dev->dev,
410-
"Detected unexpected decode failure from ASP. rc: %d\n",
411-
rc);
412-
goto disable_vmpck;
439+
dev_alert(snp_dev->dev, "Detected unexpected decode failure from ASP. rc: %d\n", rc);
440+
snp_disable_vmpck(snp_dev);
441+
return rc;
413442
}
414443

415444
return 0;
416-
417-
disable_vmpck:
418-
snp_disable_vmpck(snp_dev);
419-
return rc;
420445
}
421446

422447
static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
@@ -703,6 +728,9 @@ static int __init sev_guest_probe(struct platform_device *pdev)
703728
void __iomem *mapping;
704729
int ret;
705730

731+
if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
732+
return -ENODEV;
733+
706734
if (!dev->platform_data)
707735
return -ENODEV;
708736

0 commit comments

Comments
 (0)