drivers/hv/connection.c | 4 ++++ 1 file changed, 4 insertions(+)
relid2channel() assumes vmbus channel array to be allocated when called.
However, in cases such as kdump/kexec, not all relids will be reset by the host.
When the second kernel boots and if the guest receives a vmbus interrupt during
vmbus driver initialization before vmbus_connect() is called, before it finishes,
or if it fails, the vmbus interrupt service routine is called which in turn calls
relid2channel() and can cause a null pointer dereference.
Print a warning and error out in relid2channel() for a channel id that's invalid
in the second kernel.
Fixes: 8b6a877c060e ("Drivers: hv: vmbus: Replace the per-CPU channel lists with a global array of channels")
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
Changes from v2:
* Changed commit message
* Use pr_warn_once() instead of WARN() with a shorter message
for less overall noise
---
drivers/hv/connection.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 9dc27e5d367a..da51b50787df 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -409,6 +409,10 @@ void vmbus_disconnect(void)
*/
struct vmbus_channel *relid2channel(u32 relid)
{
+ if (vmbus_connection.channels == NULL) {
+ pr_warn_once("relid2channel: relid=%d: No channels mapped!\n", relid);
+ return NULL;
+ }
if (WARN_ON(relid >= MAX_CHANNEL_RELIDS))
return NULL;
return READ_ONCE(vmbus_connection.channels[relid]);
--
2.38.1
> From: Mohammed Gamal <mgamal@redhat.com> > Sent: Friday, February 17, 2023 12:44 PM > ... > --- a/drivers/hv/connection.c > +++ b/drivers/hv/connection.c > @@ -409,6 +409,10 @@ void vmbus_disconnect(void) > */ > struct vmbus_channel *relid2channel(u32 relid) > { > + if (vmbus_connection.channels == NULL) { > + pr_warn_once("relid2channel: relid=%d: No channels mapped!\n", > relid); Looks good to me except that the line exceeds 80 characters. Please run "scripts/checkpatch.pl" to detect that. For this patch, I guess Wei may be willing to help fix it. > + return NULL; > + } > if (WARN_ON(relid >= MAX_CHANNEL_RELIDS)) > return NULL; > return READ_ONCE(vmbus_connection.channels[relid]); > -- Reviewed-by: Dexuan Cui <decui@microsoft.com>
On Fri, Feb 17, 2023 at 11:17:04PM +0000, Dexuan Cui wrote: > > From: Mohammed Gamal <mgamal@redhat.com> > > Sent: Friday, February 17, 2023 12:44 PM > > ... > > --- a/drivers/hv/connection.c > > +++ b/drivers/hv/connection.c > > @@ -409,6 +409,10 @@ void vmbus_disconnect(void) > > */ > > struct vmbus_channel *relid2channel(u32 relid) > > { > > + if (vmbus_connection.channels == NULL) { > > + pr_warn_once("relid2channel: relid=%d: No channels mapped!\n", > > relid); > > Looks good to me except that the line exceeds 80 characters. > Please run "scripts/checkpatch.pl" to detect that. > For this patch, I guess Wei may be willing to help fix it. > > > + return NULL; > > + } > > if (WARN_ON(relid >= MAX_CHANNEL_RELIDS)) > > return NULL; > > return READ_ONCE(vmbus_connection.channels[relid]); > > -- > > Reviewed-by: Dexuan Cui <decui@microsoft.com> I will pick this up via hyperv-fixes.
On Mon, Feb 20, 2023 at 02:23:50PM +0000, Wei Liu wrote: > On Fri, Feb 17, 2023 at 11:17:04PM +0000, Dexuan Cui wrote: > > > From: Mohammed Gamal <mgamal@redhat.com> > > > Sent: Friday, February 17, 2023 12:44 PM > > > ... > > > --- a/drivers/hv/connection.c > > > +++ b/drivers/hv/connection.c > > > @@ -409,6 +409,10 @@ void vmbus_disconnect(void) > > > */ > > > struct vmbus_channel *relid2channel(u32 relid) > > > { > > > + if (vmbus_connection.channels == NULL) { > > > + pr_warn_once("relid2channel: relid=%d: No channels mapped!\n", > > > relid); > > > > Looks good to me except that the line exceeds 80 characters. > > Please run "scripts/checkpatch.pl" to detect that. > > For this patch, I guess Wei may be willing to help fix it. > > > > > + return NULL; > > > + } > > > if (WARN_ON(relid >= MAX_CHANNEL_RELIDS)) > > > return NULL; > > > return READ_ONCE(vmbus_connection.channels[relid]); > > > -- > > > > Reviewed-by: Dexuan Cui <decui@microsoft.com> > > I will pick this up via hyperv-fixes. Now applied to hyperv-fixes. Thanks, Wei.
On Fri, Feb 17, 2023 at 11:17:04PM +0000, Dexuan Cui wrote: > > From: Mohammed Gamal <mgamal@redhat.com> > > Sent: Friday, February 17, 2023 12:44 PM > > ... > > --- a/drivers/hv/connection.c > > +++ b/drivers/hv/connection.c > > @@ -409,6 +409,10 @@ void vmbus_disconnect(void) > > */ > > struct vmbus_channel *relid2channel(u32 relid) > > { > > + if (vmbus_connection.channels == NULL) { > > + pr_warn_once("relid2channel: relid=%d: No channels mapped!\n", > > relid); > > Looks good to me except that the line exceeds 80 characters. > Please run "scripts/checkpatch.pl" to detect that. FWIW the max line length has been bumped to 100 in checkpatch.pl. > For this patch, I guess Wei may be willing to help fix it. > > > + return NULL; > > + } > > if (WARN_ON(relid >= MAX_CHANNEL_RELIDS)) > > return NULL; > > return READ_ONCE(vmbus_connection.channels[relid]); > > -- > > Reviewed-by: Dexuan Cui <decui@microsoft.com>
© 2016 - 2025 Red Hat, Inc.