| Message ID | 20240712080329.197605-2-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: virtio: fix virtnet_sq_free_stats initialization | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next-1 |
On Fri, Jul 12, 2024 at 09:03:30AM +0100, Jean-Philippe Brucker wrote: > Commit c8bd1f7f3e61 ("virtio_net: add support for Byte Queue Limits") > added two new fields to struct virtnet_sq_free_stats, but commit > 23c81a20b998 ("net: virtio: unify code to init stats") accidentally > removed their initialization. In the worst case this can trigger the BUG > at lib/dynamic_queue_limits.c:99 because dql_completed() receives a > random value as count. Initialize the whole structure. > > Fixes: 23c81a20b998 ("net: virtio: unify code to init stats") > Reported-by: Aishwarya TCV <aishwarya.tcv@arm.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > Both these patches are still in next so it might be possible to fix it > up directly. I'd be fine with squashing but I don't think it's done in net-next. > --- > drivers/net/virtio_net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 10d8674eec5d2..f014802522e0f 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -530,7 +530,7 @@ static void __free_old_xmit(struct send_queue *sq, struct netdev_queue *txq, > unsigned int len; > void *ptr; > > - stats->bytes = stats->packets = 0; > + memset(stats, 0, sizeof(*stats)); > > while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) { > if (!is_xdp_frame(ptr)) { > > base-commit: 3fe121b622825ff8cc995a1e6b026181c48188db > -- > 2.45.2
On Fri, Jul 12, 2024 at 06:41:34AM -0400, Michael S. Tsirkin wrote: > On Fri, Jul 12, 2024 at 09:03:30AM +0100, Jean-Philippe Brucker wrote: > > Commit c8bd1f7f3e61 ("virtio_net: add support for Byte Queue Limits") > > added two new fields to struct virtnet_sq_free_stats, but commit > > 23c81a20b998 ("net: virtio: unify code to init stats") accidentally > > removed their initialization. In the worst case this can trigger the BUG > > at lib/dynamic_queue_limits.c:99 because dql_completed() receives a > > random value as count. Initialize the whole structure. > > > > Fixes: 23c81a20b998 ("net: virtio: unify code to init stats") > > Reported-by: Aishwarya TCV <aishwarya.tcv@arm.com> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > Both these patches are still in next so it might be possible to fix it > > up directly. > > I'd be fine with squashing but I don't think it's done in net-next. True, but this patch doesn't apply to net-next. And 23c81a20b998 ("net: virtio: unify code to init stats") isn't present in net-next. ...
On Fri, Jul 12, 2024 at 07:20:21PM +0100, Simon Horman wrote: > On Fri, Jul 12, 2024 at 06:41:34AM -0400, Michael S. Tsirkin wrote: > > On Fri, Jul 12, 2024 at 09:03:30AM +0100, Jean-Philippe Brucker wrote: > > > Commit c8bd1f7f3e61 ("virtio_net: add support for Byte Queue Limits") > > > added two new fields to struct virtnet_sq_free_stats, but commit > > > 23c81a20b998 ("net: virtio: unify code to init stats") accidentally > > > removed their initialization. In the worst case this can trigger the BUG > > > at lib/dynamic_queue_limits.c:99 because dql_completed() receives a > > > random value as count. Initialize the whole structure. > > > > > > Fixes: 23c81a20b998 ("net: virtio: unify code to init stats") > > > Reported-by: Aishwarya TCV <aishwarya.tcv@arm.com> > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > > > --- > > > Both these patches are still in next so it might be possible to fix it > > > up directly. > > > > I'd be fine with squashing but I don't think it's done in net-next. > > True, but this patch doesn't apply to net-next. > And 23c81a20b998 ("net: virtio: unify code to init stats") > isn't present in net-next. Oh right, it's in linux-next but it looks like it came from git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next Thanks, Jean
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 10d8674eec5d2..f014802522e0f 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -530,7 +530,7 @@ static void __free_old_xmit(struct send_queue *sq, struct netdev_queue *txq, unsigned int len; void *ptr; - stats->bytes = stats->packets = 0; + memset(stats, 0, sizeof(*stats)); while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) { if (!is_xdp_frame(ptr)) {
Commit c8bd1f7f3e61 ("virtio_net: add support for Byte Queue Limits") added two new fields to struct virtnet_sq_free_stats, but commit 23c81a20b998 ("net: virtio: unify code to init stats") accidentally removed their initialization. In the worst case this can trigger the BUG at lib/dynamic_queue_limits.c:99 because dql_completed() receives a random value as count. Initialize the whole structure. Fixes: 23c81a20b998 ("net: virtio: unify code to init stats") Reported-by: Aishwarya TCV <aishwarya.tcv@arm.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- Both these patches are still in next so it might be possible to fix it up directly. --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 3fe121b622825ff8cc995a1e6b026181c48188db