From owner-dev-commits-src-branches@freebsd.org Wed Jul 21 16:14:03 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 7FE42666414; Wed, 21 Jul 2021 16:14:03 +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 4GVLHb3Bhmz3LtX; Wed, 21 Jul 2021 16:14:03 +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 559FE20A78; Wed, 21 Jul 2021 16:14:03 +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 16LGE37a080355; Wed, 21 Jul 2021 16:14:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 16LGE3C8080354; Wed, 21 Jul 2021 16:14:03 GMT (envelope-from git) Date: Wed, 21 Jul 2021 16:14:03 GMT Message-Id: <202107211614.16LGE3C8080354@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: c8b24162a4eb - stable/13 - 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/13 X-Git-Reftype: branch X-Git-Commit: c8b24162a4eb20484d4add7710a33ef6387111f7 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:14:03 -0000 The branch stable/13 has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=c8b24162a4eb20484d4add7710a33ef6387111f7 commit c8b24162a4eb20484d4add7710a33ef6387111f7 Author: Young Xiao <92siuyang@gmail.com> AuthorDate: 2019-05-21 07:36:29 +0000 Commit: Warner Losh CommitDate: 2021-07-21 16:13:10 +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 559e9a234b87..a3be7470354f 100644 --- a/sys/cam/ata/ata_xpt.c +++ b/sys/cam/ata/ata_xpt.c @@ -1756,9 +1756,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; @@ -1767,6 +1769,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 126b284936bb..800d6aeb291e 100644 --- a/sys/cam/nvme/nvme_xpt.c +++ b/sys/cam/nvme/nvme_xpt.c @@ -682,9 +682,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; @@ -693,6 +695,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;