On 8/25/2024 8:21 AM, Michael Kelley wrote:
From: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx> Sent: Thursday, August 22, 2024 4:09 AM
From: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
For primary VMBus channels primary_channel pointer is always NULL. This
pointer is valid only for the secondry channels.
Fix NULL pointer dereference by retrieving the device_obj from the parent
in the absence of a valid primary_channel pointer.
Fixes: ca3cda6fcf1e ("uio_hv_generic: add rescind support")
Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx>
---
drivers/uio/uio_hv_generic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index b45653752301..c99890c16d29 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -109,7 +109,8 @@ static void hv_uio_channel_cb(void *context)
*/
static void hv_uio_rescind(struct vmbus_channel *channel)
{
- struct hv_device *hv_dev = channel->primary_channel->device_obj;
+ struct hv_device *hv_dev = channel->primary_channel ?
+ channel->primary_channel->device_obj : channel->device_obj;
It looks to me like hv_uio_rescind() is called only for the primary
channel. That makes sense, because waking up the reader should
presumably be done once for the device, not once for each channel.
Rather than generalizing the function so it works for both primary
and secondary channels, I'd suggest checking if the channel is a
secondary channel. If so, output a warning message or do WARN(),
and then return immediately, as some there's some kind of
programming error.
Thanks for reviewing Michael. By design, there is different handling for
secondary channel rescind in channel_mgmt.c [1] and this callback is not
expected to be called. I will make the change in my next patch to make
it a warning.
[1]
} else if (channel->primary_channel != NULL) {
/*
* Sub-channel is being rescinded. Following is the channel
* close sequence when initiated from the driveri (refer to
* vmbus_close() for details):
* 1. Close all sub-channels first
* 2. Then close the primary channel.
*/
Looking at the history of the code, it appears that rescinding a UIO
device could never have worked. Is that your conclusion as well,
or am I missing something?
Michael
Most likely, yes.
Regards,
Naman
