From nobody Wed Nov 03 17:40:45 2021 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id AFCBB184D3C4; Wed, 3 Nov 2021 17:40:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HkvF94cxdz3lDl; Wed, 3 Nov 2021 17:40:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 7EF0B1CFAA; Wed, 3 Nov 2021 17:40:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1A3HejOD011969; Wed, 3 Nov 2021 17:40:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1A3HejcS011968; Wed, 3 Nov 2021 17:40:45 GMT (envelope-from git) Date: Wed, 3 Nov 2021 17:40:45 GMT Message-Id: <202111031740.1A3HejcS011968@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Hans Petter Selasky Subject: git: 791f88fff31d - releng/12.3 - usb(4): Fix for use after free in combination with EVDEV_SUPPORT. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: hselasky X-Git-Repository: src X-Git-Refname: refs/heads/releng/12.3 X-Git-Reftype: branch X-Git-Commit: 791f88fff31d8067ffbfc4f0670aff84f094549d Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch releng/12.3 has been updated by hselasky: URL: https://cgit.FreeBSD.org/src/commit/?id=791f88fff31d8067ffbfc4f0670aff84f094549d commit 791f88fff31d8067ffbfc4f0670aff84f094549d Author: Hans Petter Selasky AuthorDate: 2021-10-24 11:38:04 +0000 Commit: Hans Petter Selasky CommitDate: 2021-11-03 17:39:23 +0000 usb(4): Fix for use after free in combination with EVDEV_SUPPORT. When EVDEV_SUPPORT was introduced, the USB transfers may be running after the main FIFO is closed. In connection to this a race may appear which can lead to use-after-free scenarios. Fix this for all FIFO consumers by initializing and resetting the FIFO queues under the lock used by the client. Then the client driver will see an empty queue in all cases a race may appear. Approved by: re@ (gjb) Found by: pho@ Sponsored by: NVIDIA Networking (cherry picked from commit aad0c65d6b37364d8ba92ecb8c85e004398a5194) (cherry picked from commit bb9bee1ffbb27f903bfd2c11d681d331bea727ea) --- sys/dev/usb/usb_dev.c | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c index 9cd54bf0032c..d1a65bf8561b 100644 --- a/sys/dev/usb/usb_dev.c +++ b/sys/dev/usb/usb_dev.c @@ -1951,18 +1951,30 @@ int usb_fifo_alloc_buffer(struct usb_fifo *f, usb_size_t bufsize, uint16_t nbuf) { + struct usb_ifqueue temp_q = {}; + void *queue_data; + usb_fifo_free_buffer(f); - /* allocate an endpoint */ - f->free_q.ifq_maxlen = nbuf; - f->used_q.ifq_maxlen = nbuf; + temp_q.ifq_maxlen = nbuf; - f->queue_data = usb_alloc_mbufs( - M_USBDEV, &f->free_q, bufsize, nbuf); + queue_data = usb_alloc_mbufs( + M_USBDEV, &temp_q, bufsize, nbuf); - if ((f->queue_data == NULL) && bufsize && nbuf) { + if (queue_data == NULL && bufsize != 0 && nbuf != 0) return (ENOMEM); - } + + mtx_lock(f->priv_mtx); + + /* + * Setup queues and sizes under lock to avoid early use by + * concurrent FIFO access: + */ + f->free_q = temp_q; + f->used_q.ifq_maxlen = nbuf; + f->queue_data = queue_data; + mtx_unlock(f->priv_mtx); + return (0); /* success */ } @@ -1975,15 +1987,24 @@ usb_fifo_alloc_buffer(struct usb_fifo *f, usb_size_t bufsize, void usb_fifo_free_buffer(struct usb_fifo *f) { - if (f->queue_data) { - /* free old buffer */ - free(f->queue_data, M_USBDEV); - f->queue_data = NULL; - } - /* reset queues */ + void *queue_data; + + mtx_lock(f->priv_mtx); + + /* Get and clear pointer to free, if any. */ + queue_data = f->queue_data; + f->queue_data = NULL; + /* + * Reset queues under lock to avoid use of freed buffers by + * concurrent FIFO activity: + */ memset(&f->free_q, 0, sizeof(f->free_q)); memset(&f->used_q, 0, sizeof(f->used_q)); + mtx_unlock(f->priv_mtx); + + /* Free old buffer, if any. */ + free(queue_data, M_USBDEV); } void