RE: FAILED: patch "[PATCH] Drivers: hv: vmbus: Check for ring when getting debug info" failed to apply to 4.9-stable tree

From: gregkh@linuxfoundation.org gregkh@linuxfoundation.org Sent: Sunday, January 27, 2019 7:43 AM To: Dexuan Cui decui@microsoft.com; Haiyang Zhang haiyangz@microsoft.com; KY Srinivasan kys@microsoft.com; sashal@kernel.org; Stephen Hemminger sthemmin@microsoft.com Cc: stable@vger.kernel.org Subject: FAILED: patch "[PATCH] Drivers: hv: vmbus: Check for ring when getting debug info" failed to apply to 4.9-stable tree

The patch below does not apply to the 4.9-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From ba50bf1ce9a51fc97db58b96d01306aa70bc3979 Mon Sep 17 00:00:00 2001 From: Dexuan Cui decui@microsoft.com Date: Mon, 17 Dec 2018 20:16:09 +0000 Subject: [PATCH] Drivers: hv: vmbus: Check for ring when getting debug info

fc96df16a1ce is good and can already fix the "return stack garbage" issue, but let's also improve hv_ringbuffer_get_debuginfo(), which would silently return stack garbage, if people forget to check channel->state or ring_info->ring_buffer, when using the function in the future.

Having an error check in the function would eliminate the potential risk.

Add a Fixes tag to indicate the patch depdendency.

Fixes: fc96df16a1ce ("Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels") Cc: stable@vger.kernel.org Cc: K. Y. Srinivasan kys@microsoft.com Cc: Haiyang Zhang haiyangz@microsoft.com Signed-off-by: Stephen Hemminger sthemmin@microsoft.com Signed-off-by: Dexuan Cui decui@microsoft.com Signed-off-by: Sasha Levin sashal@kernel.org

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 64d0c85d5161..1f1a55e07733 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -164,26 +164,25 @@ hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi, }

/* Get various debug metrics for the specified ring buffer. */ -void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,

• 		 struct hv_ring_buffer_debug_info *debug_info)
  

+int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,

• 		struct hv_ring_buffer_debug_info *debug_info)
  

{ u32 bytes_avail_towrite; u32 bytes_avail_toread;

• if (ring_info->ring_buffer) {

• hv_get_ringbuffer_availbytes(ring_info,
  

• 			&bytes_avail_toread,
  

• 			&bytes_avail_towrite);
  

• debug_info->bytes_avail_toread = bytes_avail_toread;
  

• debug_info->bytes_avail_towrite = bytes_avail_towrite;
  

• debug_info->current_read_index =
  

• 	ring_info->ring_buffer->read_index;
  

• debug_info->current_write_index =
  

• 	ring_info->ring_buffer->write_index;
  

• debug_info->current_interrupt_mask =
  

• 	ring_info->ring_buffer->interrupt_mask;
  

• }

• if (!ring_info->ring_buffer)

• return -EINVAL;
  

• hv_get_ringbuffer_availbytes(ring_info,

• 		     &bytes_avail_toread,
  

• 		     &bytes_avail_towrite);
  

• debug_info->bytes_avail_toread = bytes_avail_toread;
• debug_info->bytes_avail_towrite = bytes_avail_towrite;
• debug_info->current_read_index = ring_info->ring_buffer->read_index;
• debug_info->current_write_index = ring_info->ring_buffer->write_index;
• debug_info->current_interrupt_mask

• = ring_info->ring_buffer->interrupt_mask;
  

• return 0;

} EXPORT_SYMBOL_GPL(hv_ringbuffer_get_debuginfo);

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index d0ff65675292..403fee01572c 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -313,12 +313,16 @@ static ssize_t out_intr_mask_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info outbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

&outbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

• 			  &outbound);
  

• if (ret < 0)

• return ret;
  

• return sprintf(buf, "%d\n", outbound.current_interrupt_mask);

} static DEVICE_ATTR_RO(out_intr_mask); @@ -328,12 +332,15 @@ static ssize_t out_read_index_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info outbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

&outbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

• 			  &outbound);
  

• if (ret < 0)

• return ret;
  

  return sprintf(buf, "%d\n", outbound.current_read_index);

} static DEVICE_ATTR_RO(out_read_index); @@ -344,12 +351,15 @@ static ssize_t out_write_index_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info outbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

&outbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

• 			  &outbound);
  

• if (ret < 0)

• return ret;
  

  return sprintf(buf, "%d\n", outbound.current_write_index);

} static DEVICE_ATTR_RO(out_write_index); @@ -360,12 +370,15 @@ static ssize_t out_read_bytes_avail_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info outbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

&outbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

• 			  &outbound);
  

• if (ret < 0)

• return ret;
  

  return sprintf(buf, "%d\n", outbound.bytes_avail_toread);

} static DEVICE_ATTR_RO(out_read_bytes_avail); @@ -376,12 +389,15 @@ static ssize_t out_write_bytes_avail_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info outbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

&outbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,

• 			  &outbound);
  

• if (ret < 0)

• return ret;
  

  return sprintf(buf, "%d\n", outbound.bytes_avail_towrite);

} static DEVICE_ATTR_RO(out_write_bytes_avail); @@ -391,12 +407,15 @@ static ssize_t in_intr_mask_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info inbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound,

&inbound);

• if (ret < 0)

• return ret;
  

• return sprintf(buf, "%d\n", inbound.current_interrupt_mask);

} static DEVICE_ATTR_RO(in_intr_mask); @@ -406,12 +425,15 @@ static ssize_t in_read_index_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info inbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound,

&inbound);

• if (ret < 0)

• return ret;
  

• return sprintf(buf, "%d\n", inbound.current_read_index);

} static DEVICE_ATTR_RO(in_read_index); @@ -421,12 +443,15 @@ static ssize_t in_write_index_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info inbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound,

&inbound);

• if (ret < 0)

• return ret;
  

• return sprintf(buf, "%d\n", inbound.current_write_index);

} static DEVICE_ATTR_RO(in_write_index); @@ -437,12 +462,15 @@ static ssize_t in_read_bytes_avail_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info inbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound,

&inbound);

• if (ret < 0)

• return ret;
  

• return sprintf(buf, "%d\n", inbound.bytes_avail_toread);

} static DEVICE_ATTR_RO(in_read_bytes_avail); @@ -453,12 +481,15 @@ static ssize_t in_write_bytes_avail_show(struct device *dev, { struct hv_device *hv_dev = device_to_hv_device(dev); struct hv_ring_buffer_debug_info inbound;

• int ret;

  if (!hv_dev->channel) return -ENODEV;

• if (hv_dev->channel->state != CHANNEL_OPENED_STATE)

• return -EINVAL;
  

• hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);

• ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound,

&inbound);

• if (ret < 0)

• return ret;
  

• return sprintf(buf, "%d\n", inbound.bytes_avail_towrite);

} static DEVICE_ATTR_RO(in_write_bytes_avail); diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index f0885cc01db6..dcb6977afce9 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1159,8 +1159,9 @@ struct hv_ring_buffer_debug_info { u32 bytes_avail_towrite; };

-void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,

• 	    struct hv_ring_buffer_debug_info *debug_info);
  

+int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,

• 		struct hv_ring_buffer_debug_info *debug_info);
  
  

/* Vmbus interface */ #define vmbus_driver_register(driver) \

We don't need to apply this patch to the 4.9-stable tree, because: 1. This is just a simple code refactoring, which doesn't really have a functional change. 2. We have to backport at least 2 more patches to apply this patch cleanly, which doesn't deserve the effort to me.

Thanks, -- Dexuan