From owner-dev-commits-src-branches@freebsd.org Wed Jul 21 16:18:49 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id CCA66666D8A; Wed, 21 Jul 2021 16:18:49 +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 4GVLP54zzxz3Mhd; Wed, 21 Jul 2021 16:18:49 +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 861622123C; Wed, 21 Jul 2021 16:18:49 +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 16LGInmi081353; Wed, 21 Jul 2021 16:18:49 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 16LGIngP081352; Wed, 21 Jul 2021 16:18:49 GMT (envelope-from git) Date: Wed, 21 Jul 2021 16:18:49 GMT Message-Id: <202107211618.16LGIngP081352@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Warner Losh Subject: git: 2f5778abbe18 - stable/12 - Fix potential NULL pointer dereference of device physical path MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 2f5778abbe180dcb44eaf17db1fb915204249615 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jul 2021 16:18:50 -0000 The branch stable/12 has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=2f5778abbe180dcb44eaf17db1fb915204249615 commit 2f5778abbe180dcb44eaf17db1fb915204249615 Author: Young Xiao <92siuyang@gmail.com> AuthorDate: 2019-05-21 07:36:29 +0000 Commit: Warner Losh CommitDate: 2021-07-21 16:16:31 +0000 Fix potential NULL pointer dereference of device physical path In ata_dev_advinfo() and nvme_dev_advinfo(), if the physical path is being stored and there is a malloc failure (malloc(9) is called with M_NOWAIT), we could wind up in a situation where the device's physpath_len is set to the length the user provided, but the physpath itself is NULL. If another context then comes in to fetch the physical path value, we would wind up trying to memcpy a NULL pointer into the caller's buffer. So, set the physpath_len to 0 when we free the physpath on entry into the store case for the physical path. Reset the length to a non-zero value only after we've successfully malloced a buffer to hold it. This code mirrors scsi_xpt.c does already as well. Signed-off-by: Young Xiao <92siuyang@gmail.com> Reviewed by: imp PR: 238014 (cherry picked from commit 431ddd94360a9e86c91294eaa2c7b859911984b7) --- sys/cam/ata/ata_xpt.c | 7 +++++-- sys/cam/nvme/nvme_xpt.c | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/sys/cam/ata/ata_xpt.c b/sys/cam/ata/ata_xpt.c index 5b789559468f..34a70a951a39 100644 --- a/sys/cam/ata/ata_xpt.c +++ b/sys/cam/ata/ata_xpt.c @@ -1757,9 +1757,11 @@ ata_dev_advinfo(union ccb *start_ccb) break; case CDAI_TYPE_PHYS_PATH: if (cdai->flags & CDAI_FLAG_STORE) { - if (device->physpath != NULL) + if (device->physpath != NULL) { free(device->physpath, M_CAMXPT); - device->physpath_len = cdai->bufsiz; + device->physpath = NULL; + device->physpath_len = 0; + } /* Clear existing buffer if zero length */ if (cdai->bufsiz == 0) break; @@ -1768,6 +1770,7 @@ ata_dev_advinfo(union ccb *start_ccb) start_ccb->ccb_h.status = CAM_REQ_ABORTED; return; } + device->physpath_len = cdai->bufsiz; memcpy(device->physpath, cdai->buf, cdai->bufsiz); } else { cdai->provsiz = device->physpath_len; diff --git a/sys/cam/nvme/nvme_xpt.c b/sys/cam/nvme/nvme_xpt.c index 68097a5e4e74..f3cf66f1ca19 100644 --- a/sys/cam/nvme/nvme_xpt.c +++ b/sys/cam/nvme/nvme_xpt.c @@ -683,9 +683,11 @@ nvme_dev_advinfo(union ccb *start_ccb) break; case CDAI_TYPE_PHYS_PATH: if (cdai->flags & CDAI_FLAG_STORE) { - if (device->physpath != NULL) + if (device->physpath != NULL) { free(device->physpath, M_CAMXPT); - device->physpath_len = cdai->bufsiz; + device->physpath = NULL; + device->physpath_len = 0; + } /* Clear existing buffer if zero length */ if (cdai->bufsiz == 0) break; @@ -694,6 +696,7 @@ nvme_dev_advinfo(union ccb *start_ccb) start_ccb->ccb_h.status = CAM_REQ_ABORTED; return; } + device->physpath_len = cdai->bufsiz; memcpy(device->physpath, cdai->buf, cdai->bufsiz); } else { cdai->provsiz = device->physpath_len;