Re: general protection fault in tomoyo_socket_sendmsg_permission

[Shuah Khan]

On 11/13/20 5:00 AM, Hillf Danton wrote:

Thu, 12 Nov 2020 23:21:26 -0800

syzbot found the following issue on:

HEAD commit:    9dbc1c03 Merge tag 'xfs-5.10-fixes-3' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10453034500000
kernel config:  https://syzkaller.appspot.com/x/.config?x=1735b7978b1c3721
dashboard link: https://syzkaller.appspot.com/bug?extid=95ce4b142579611ef0a9
compiler:       gcc (GCC) 10.1.0-syz 20200507
userspace arch: i386

Unfortunately, I don't have any reproducer for this issue yet.

I would like to see the reproducer for this. I just can't reproduce
this problem.

Fix 96c2737716d5 ("usbip: move usbip kernel code out of staging")
by closing the race between readers and writer of ud->tcp_socket on
sock shutdown. To do that, add waitqueue for usbip device and make
writer wait for all readers to go home before releasing the socket.


--- a/drivers/usb/usbip/usbip_common.h
+++ b/drivers/usb/usbip/usbip_common.h
@@ -277,6 +277,9 @@ struct usbip_device {
                void (*reset)(struct usbip_device *);
                void (*unusable)(struct usbip_device *);
        } eh_ops;
+
+       atomic_t sk_shutdown_count;
+       wait_queue_head_t sk_shutdown_waitq;
  };

#define kthread_get_run(threadfn, data, namefmt, ...) \

--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1008,15 +1008,20 @@ static void vhci_shutdown_connection(str

/* kill threads related to this sdev */

        if (vdev->ud.tcp_rx) {
+               atomic_inc(&ud->sk_shutdown_count);
                kthread_stop_put(vdev->ud.tcp_rx);
                vdev->ud.tcp_rx = NULL;
        }
        if (vdev->ud.tcp_tx) {
+               atomic_inc(&ud->sk_shutdown_count);
                kthread_stop_put(vdev->ud.tcp_tx);
                vdev->ud.tcp_tx = NULL;
        }
        pr_info("stop threads\n");

+ wait_event(ud->sk_shutdown_waitq,

+                       !atomic_read(&ud->sk_shutdown_count));
+
        /* active connection is closed */
        if (vdev->ud.tcp_socket) {
                sockfd_put(vdev->ud.tcp_socket);
@@ -1104,6 +1109,8 @@ static void vhci_device_init(struct vhci
        vdev->ud.eh_ops.reset = vhci_device_reset;
        vdev->ud.eh_ops.unusable = vhci_device_unusable;

+ init_waitqueue_head(&vdev->ud.sk_shutdown_waitq);

+
        usbip_start_eh(&vdev->ud);
  }

--- a/drivers/usb/usbip/vhci_rx.c

+++ b/drivers/usb/usbip/vhci_rx.c
@@ -264,5 +264,8 @@ int vhci_rx_loop(void *data)
                vhci_rx_pdu(ud);
        }

+ if (atomic_dec_and_test(&ud->sk_shutdown_count))

+               wake_up(&ud->sk_shutdown_waitq);
+
        return 0;
  }
--- a/drivers/usb/usbip/vhci_tx.c
+++ b/drivers/usb/usbip/vhci_tx.c
@@ -252,5 +252,8 @@ int vhci_tx_loop(void *data)
                usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
        }

+ if (atomic_dec_and_test(&ud->sk_shutdown_count))

+               wake_up(&ud->sk_shutdown_waitq);
+
        return 0;
  }

Without a reproducer, it is hard to verify the fix.

thanks,
-- Shuah