Stefano Garzarella
2023-Mar-24 09:30 UTC
[syzbot] [kvm?] [net?] [virt?] general protection fault in virtio_transport_purge_skbs
On Fri, Mar 24, 2023 at 10:10?AM Arseniy Krasnov <avkrasnov at sberdevices.ru> wrote:> On 24.03.2023 12:06, Stefano Garzarella wrote: > > On Fri, Mar 24, 2023 at 9:55?AM Stefano Garzarella <sgarzare at redhat.com> wrote: > >> > >> On Fri, Mar 24, 2023 at 9:31?AM Stefano Garzarella <sgarzare at redhat.com> wrote: > >>> > >>> Hi Bobby, > >>> can you take a look at this report? > >>> > >>> It seems related to the changes we made to support skbuff. > >> > >> Could it be a problem of concurrent access to pkt_queue ? > >> > >> IIUC we should hold pkt_queue.lock when we call skb_queue_splice_init() > >> and remove pkt_list_lock. (or hold pkt_list_lock when calling > >> virtio_transport_purge_skbs, but pkt_list_lock seems useless now that > >> we use skbuff) > >> > > > > In the previous patch was missing a hunk, new one attached: > > > > #syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git fff5a5e7f528 > > > > --- a/net/vmw_vsock/vsock_loopback.c > > +++ b/net/vmw_vsock/vsock_loopback.c > > @@ -15,7 +15,6 @@ > > struct vsock_loopback { > > struct workqueue_struct *workqueue; > > > > - spinlock_t pkt_list_lock; /* protects pkt_list */ > > struct sk_buff_head pkt_queue; > > struct work_struct pkt_work; > > }; > > @@ -32,9 +31,7 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb) > > struct vsock_loopback *vsock = &the_vsock_loopback; > > int len = skb->len; > > > > - spin_lock_bh(&vsock->pkt_list_lock); > > skb_queue_tail(&vsock->pkt_queue, skb); > Hello Stefano and Bobby, > > Small remark, may be here we can use virtio_vsock_skb_queue_tail() instead of skb_queue_tail(). > skb_queue_tail() disables irqs during spinlock access, while virtio_vsock_skb_queue_tail() > uses spin_lock_bh(). vhost and virtio transports use virtio_vsock_skb_queue_tail(). >Yep, but this shouldn't be related. I would make this change in a separate patch. ;-) Thanks, Stefano