From: Rick Edgecombe rick.p.edgecombe@intel.com
In CoCo VMs it is possible for the untrusted host to cause set_memory_encrypted() or set_memory_decrypted() to fail such that an error is returned and the resulting memory is shared. Callers need to take care to handle these errors to avoid returning decrypted (shared) memory to the page allocator, which could lead to functional or security issues.
VMBus code could free decrypted pages if set_memory_encrypted()/decrypted() fails. Leak the pages if this happens.
Signed-off-by: Rick Edgecombe rick.p.edgecombe@intel.com Signed-off-by: Michael Kelley mhklinux@outlook.com Reviewed-by: Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com Link: https://lore.kernel.org/r/20240311161558.1310-2-mhklinux@outlook.com Signed-off-by: Wei Liu wei.liu@kernel.org Message-ID: 20240311161558.1310-2-mhklinux@outlook.com [Xiangyu: Modified to apply on 6.1.y] Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com --- drivers/hv/connection.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index da51b50787df..c74088b69a5f 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -243,8 +243,17 @@ int vmbus_connect(void) ret |= set_memory_decrypted((unsigned long) vmbus_connection.monitor_pages[1], 1); - if (ret) + if (ret) { + /* + * If set_memory_decrypted() fails, the encryption state + * of the memory is unknown. So leak the memory instead + * of risking returning decrypted memory to the free list. + * For simplicity, always handle both pages the same. + */ + vmbus_connection.monitor_pages[0] = NULL; + vmbus_connection.monitor_pages[1] = NULL; goto cleanup; + }
/* * Isolation VM with AMD SNP needs to access monitor page via
From: Kenton Groombridge concord@gentoo.org
req->n_channels must be set before req->channels[] can be used.
This patch fixes one of the issues encountered in [1].
[ 83.964255] UBSAN: array-index-out-of-bounds in net/mac80211/scan.c:364:4 [ 83.964258] index 0 is out of range for type 'struct ieee80211_channel *[]' [...] [ 83.964264] Call Trace: [ 83.964267] <TASK> [ 83.964269] dump_stack_lvl+0x3f/0xc0 [ 83.964274] __ubsan_handle_out_of_bounds+0xec/0x110 [ 83.964278] ieee80211_prep_hw_scan+0x2db/0x4b0 [ 83.964281] __ieee80211_start_scan+0x601/0x990 [ 83.964291] nl80211_trigger_scan+0x874/0x980 [ 83.964295] genl_family_rcv_msg_doit+0xe8/0x160 [ 83.964298] genl_rcv_msg+0x240/0x270 [...]
[1] https://bugzilla.kernel.org/show_bug.cgi?id=218810
Co-authored-by: Kees Cook keescook@chromium.org Signed-off-by: Kees Cook kees@kernel.org Signed-off-by: Kenton Groombridge concord@gentoo.org Link: https://msgid.link/20240605152218.236061-1-concord@gentoo.org Signed-off-by: Johannes Berg johannes.berg@intel.com [Xiangyu: Modified to apply on 6.1.y and 6.6.y] Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com --- V1 -> V2: add v6.6 support --- net/mac80211/scan.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c index 62c22ff329ad..d81b49fb6458 100644 --- a/net/mac80211/scan.c +++ b/net/mac80211/scan.c @@ -357,7 +357,8 @@ static bool ieee80211_prep_hw_scan(struct ieee80211_sub_if_data *sdata) struct cfg80211_scan_request *req; struct cfg80211_chan_def chandef; u8 bands_used = 0; - int i, ielen, n_chans; + int i, ielen; + u32 *n_chans; u32 flags = 0;
req = rcu_dereference_protected(local->scan_req, @@ -367,34 +368,34 @@ static bool ieee80211_prep_hw_scan(struct ieee80211_sub_if_data *sdata) return false;
if (ieee80211_hw_check(&local->hw, SINGLE_SCAN_ON_ALL_BANDS)) { + local->hw_scan_req->req.n_channels = req->n_channels; + for (i = 0; i < req->n_channels; i++) { local->hw_scan_req->req.channels[i] = req->channels[i]; bands_used |= BIT(req->channels[i]->band); } - - n_chans = req->n_channels; } else { do { if (local->hw_scan_band == NUM_NL80211_BANDS) return false;
- n_chans = 0; + n_chans = &local->hw_scan_req->req.n_channels; + *n_chans = 0;
for (i = 0; i < req->n_channels; i++) { if (req->channels[i]->band != local->hw_scan_band) continue; - local->hw_scan_req->req.channels[n_chans] = + local->hw_scan_req->req.channels[(*n_chans)++] = req->channels[i]; - n_chans++; + bands_used |= BIT(req->channels[i]->band); }
local->hw_scan_band++; - } while (!n_chans); + } while (!*n_chans); }
- local->hw_scan_req->req.n_channels = n_chans; ieee80211_prepare_scan_chandef(&chandef, req->scan_width);
if (req->flags & NL80211_SCAN_FLAG_MIN_PREQ_CONTENT)
On Wed, Oct 09, 2024 at 04:16:27PM +0800, Xiangyu Chen wrote:
From: Kenton Groombridge concord@gentoo.org
req->n_channels must be set before req->channels[] can be used.
This patch fixes one of the issues encountered in [1].
[ 83.964255] UBSAN: array-index-out-of-bounds in net/mac80211/scan.c:364:4 [ 83.964258] index 0 is out of range for type 'struct ieee80211_channel *[]' [...] [ 83.964264] Call Trace: [ 83.964267] <TASK> [ 83.964269] dump_stack_lvl+0x3f/0xc0 [ 83.964274] __ubsan_handle_out_of_bounds+0xec/0x110 [ 83.964278] ieee80211_prep_hw_scan+0x2db/0x4b0 [ 83.964281] __ieee80211_start_scan+0x601/0x990 [ 83.964291] nl80211_trigger_scan+0x874/0x980 [ 83.964295] genl_family_rcv_msg_doit+0xe8/0x160 [ 83.964298] genl_rcv_msg+0x240/0x270 [...]
[1] https://bugzilla.kernel.org/show_bug.cgi?id=218810
Co-authored-by: Kees Cook keescook@chromium.org Signed-off-by: Kees Cook kees@kernel.org Signed-off-by: Kenton Groombridge concord@gentoo.org Link: https://msgid.link/20240605152218.236061-1-concord@gentoo.org Signed-off-by: Johannes Berg johannes.berg@intel.com [Xiangyu: Modified to apply on 6.1.y and 6.6.y] Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com
V1 -> V2: add v6.6 support
No hint as to what the git id of this is in Linus's tree, so now dropping :(
On Wed, Oct 09, 2024 at 04:16:26PM +0800, Xiangyu Chen wrote:
From: Rick Edgecombe rick.p.edgecombe@intel.com
In CoCo VMs it is possible for the untrusted host to cause set_memory_encrypted() or set_memory_decrypted() to fail such that an error is returned and the resulting memory is shared. Callers need to take care to handle these errors to avoid returning decrypted (shared) memory to the page allocator, which could lead to functional or security issues.
VMBus code could free decrypted pages if set_memory_encrypted()/decrypted() fails. Leak the pages if this happens.
Signed-off-by: Rick Edgecombe rick.p.edgecombe@intel.com Signed-off-by: Michael Kelley mhklinux@outlook.com Reviewed-by: Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com Link: https://lore.kernel.org/r/20240311161558.1310-2-mhklinux@outlook.com Signed-off-by: Wei Liu wei.liu@kernel.org Message-ID: 20240311161558.1310-2-mhklinux@outlook.com [Xiangyu: Modified to apply on 6.1.y] Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com
drivers/hv/connection.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Are you sure? This is _VERY_ different from what you suggested for 5.15.y and what is in mainline. Also, you didn't show the git id for the upstream commit.
Please work to figure this out and resend working versions for ALL affected branches as new patches.
thanks,
greg k-h
Hello Greg,
On 10/9/24 21:33, Greg KH wrote:
CAUTION: This email comes from a non Wind River email account! Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Wed, Oct 09, 2024 at 04:16:26PM +0800, Xiangyu Chen wrote:
From: Rick Edgecombe rick.p.edgecombe@intel.com
In CoCo VMs it is possible for the untrusted host to cause set_memory_encrypted() or set_memory_decrypted() to fail such that an error is returned and the resulting memory is shared. Callers need to take care to handle these errors to avoid returning decrypted (shared) memory to the page allocator, which could lead to functional or security issues.
VMBus code could free decrypted pages if set_memory_encrypted()/decrypted() fails. Leak the pages if this happens.
Signed-off-by: Rick Edgecombe rick.p.edgecombe@intel.com Signed-off-by: Michael Kelley mhklinux@outlook.com Reviewed-by: Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com Link: https://lore.kernel.org/r/20240311161558.1310-2-mhklinux@outlook.com Signed-off-by: Wei Liu wei.liu@kernel.org Message-ID: 20240311161558.1310-2-mhklinux@outlook.com [Xiangyu: Modified to apply on 6.1.y] Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com
drivers/hv/connection.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Are you sure? This is _VERY_ different from what you suggested for 5.15.y and what is in mainline. Also, you didn't show the git id for the upstream commit.
This commit is a fix for CVE-2024-36913, currently, if we fully apply the commit, we have to backport the
following commits from upstream:
a5ddb745 : Drivers: hv: vmbus: Remove second mapping of VMBus monitor pages
d786e00d : drivers: hv, hyperv_fb: Untangle and refactor Hyper-V panic notifiers
9c318a1d : Drivers: hv: move panic report code from vmbus to hv early init code
a6fe0438 : Drivers: hv: Change hv_free_hyperv_page() to take void * argument
03f5a999 : Drivers: hv: vmbus: Leak pages if set_memory_encrypted() fails
Some of them are features, it might not be merged to current stable branch, so another solution
is modify the connect.c by manual.
From the upstream commit 03f5a999(Drivers: hv: vmbus: Leak pages if set_memory_encrypted() fails) we can see that
the commit aim to check set_memory_decrypted() result, if fails, the encryption of memory state is unknown, so leak the
memory.
The commit modified 2 functions, vmbus_connect() and vmbus_disconnect().
In vmbus_connect(), when set_memory_decrypted() fails, marking vmbus_connection.monitor_pages[0]/[1] to NULL.
In vmbus_disconnect(), checking the monitor_pages[0]/[1] is valid, and checking set_memory_encrypted() status, if fails, free and
leak it.
On current v6.1 branch, vmbus_disconnect() will free those memory whatever set_memory_encrypted() is success or fails, so we can just
add a monitor_pages valid checking in it.
Could you please give a suggestion that which solution is following the stable-branch rule, I will resend a V2 patch, thanks.
Br,
Xiangyu
Please work to figure this out and resend working versions for ALL affected branches as new patches.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Greg KH -
Xiangyu Chen