[PULL 0/2] hw/nvme: more fixes

[PULL 0/2] hw/nvme: more fixes

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

Hi,

The following changes since commit a8fc5165aab02f328ccd148aafec1e59fd1426eb:

  Merge tag 'nvme-next-pull-request' of https://gitlab.com/birkelund/qemu into staging (2023-08-08 16:39:20 -0700)

are available in the Git repository at:

  https://gitlab.com/birkelund/qemu.git tags/nvme-fixes-pull-request

for you to fetch changes up to 3439ba9c5da943d96f7a3c86e0a7eb2ff48de41c:

  hw/nvme: fix null pointer access in ruh update (2023-08-09 15:32:32 +0200)

----------------------------------------------------------------
hw/nvme: fixes
-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEUigzqnXi3OaiR2bATeGvMW1PDekFAmTTlmcACgkQTeGvMW1P
DemjjggAnhEvaJ4fgS9rsvtxCwtzLNc405xMpNxh6rPaxa+sL3RXPIrW6vWG13+W
VcHw8DI8EV4DzAFP919ZmTUq9/boRbgxx84bStlILUPHWol8+eGYVVfT75wFKszx
d4Vi3nyPSGlrxieSrosARqimcUDtFtDGGAxjvEcKgzhkcU3a8DVYAOmx/hdlWJJQ
KSk4h/E1pKItFbvv+w9yszsbToeZN65oIy7kQtFgx0JOULyWvEYSVygotw/AruF3
FPQ0nrJuZ115w3cJWDszznVJ6+3EcWbD3luQc3zE1FOPp76EkAOkcnPh1XbBJrE2
2BsCX/XnXcZT7BWSJbEzGXLsHjqsPg==
=Zy0+
-----END PGP SIGNATURE-----

----------------------------------------------------------------

Klaus Jensen (2):
  hw/nvme: fix null pointer access in directive receive
  hw/nvme: fix null pointer access in ruh update

 hw/nvme/ctrl.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.41.0

[PULL 1/2] hw/nvme: fix null pointer access in directive receive

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

nvme_directive_receive() does not check if an endurance group has been
configured (set) prior to testing if flexible data placement is enabled
or not.

Fix this.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1815
Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Reviewed-by: Jesper Wendel Devantier <j.devantier@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index d217ae91b506..e5b5c7034d2b 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6900,7 +6900,7 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n, NvmeRequest *req)
     case NVME_DIRECTIVE_IDENTIFY:
         switch (doper) {
         case NVME_DIRECTIVE_RETURN_PARAMS:
-            if (ns->endgrp->fdp.enabled) {
+            if (ns->endgrp && ns->endgrp->fdp.enabled) {
                 id.supported |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
                 id.enabled |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
                 id.persistent |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
-- 
2.41.0

[PULL 2/2] hw/nvme: fix null pointer access in ruh update

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

The Reclaim Unit Update operation in I/O Management Receive does not
verify the presence of a configured endurance group prior to accessing
it.

Fix this.

Cc: qemu-stable@nongnu.org
Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Reviewed-by: Jesper Wendel Devantier <j.devantier@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e5b5c7034d2b..539d27355313 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4361,7 +4361,13 @@ static uint16_t nvme_io_mgmt_send_ruh_update(NvmeCtrl *n, NvmeRequest *req)
     uint32_t npid = (cdw10 >> 1) + 1;
     unsigned int i = 0;
     g_autofree uint16_t *pids = NULL;
-    uint32_t maxnpid = n->subsys->endgrp.fdp.nrg * n->subsys->endgrp.fdp.nruh;
+    uint32_t maxnpid;
+
+    if (!ns->endgrp || !ns->endgrp->fdp.enabled) {
+        return NVME_FDP_DISABLED | NVME_DNR;
+    }
+
+    maxnpid = n->subsys->endgrp.fdp.nrg * n->subsys->endgrp.fdp.nruh;
 
     if (unlikely(npid >= MIN(NVME_FDP_MAXPIDS, maxnpid))) {
         return NVME_INVALID_FIELD | NVME_DNR;
-- 
2.41.0

Re: [PULL 0/2] hw/nvme: more fixes

[Richard Henderson]

On 8/9/23 06:39, Klaus Jensen wrote:
> From: Klaus Jensen<k.jensen@samsung.com>
> 
> Hi,
> 
> The following changes since commit a8fc5165aab02f328ccd148aafec1e59fd1426eb:
> 
>    Merge tag 'nvme-next-pull-request' ofhttps://gitlab.com/birkelund/qemu  into staging (2023-08-08 16:39:20 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/birkelund/qemu.git  tags/nvme-fixes-pull-request
> 
> for you to fetch changes up to 3439ba9c5da943d96f7a3c86e0a7eb2ff48de41c:
> 
>    hw/nvme: fix null pointer access in ruh update (2023-08-09 15:32:32 +0200)
> 
> ----------------------------------------------------------------
> hw/nvme: fixes

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~

Re: [PULL 1/2] hw/nvme: fix null pointer access in directive receive

[Philippe Mathieu-Daudé]

On 9/8/23 15:39, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> nvme_directive_receive() does not check if an endurance group has been
> configured (set) prior to testing if flexible data placement is enabled
> or not.
> 
> Fix this.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1815
> Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
> Reviewed-by: Jesper Wendel Devantier <j.devantier@samsung.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>   hw/nvme/ctrl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index d217ae91b506..e5b5c7034d2b 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -6900,7 +6900,7 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n, NvmeRequest *req)
>       case NVME_DIRECTIVE_IDENTIFY:
>           switch (doper) {
>           case NVME_DIRECTIVE_RETURN_PARAMS:
> -            if (ns->endgrp->fdp.enabled) {
> +            if (ns->endgrp && ns->endgrp->fdp.enabled) {

This patch fixes CVE-2023-40360 ("QEMU: NVMe: NULL pointer
dereference in nvme_directive_receive"). Were you aware of
the security implications?

Too bad we hadn't committed "Fixes: CVE-2023-40360" as that
would have helped downstream distributions cherry-picking
security fixes ASAP, since our stable is not that frequent.
At least the commit has the 'qemu-stable@nongnu.org' tag.

>                   id.supported |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
>                   id.enabled |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
>                   id.persistent |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;

Re: [PULL 1/2] hw/nvme: fix null pointer access in directive receive

[Michael Tokarev]

24.08.2023 15:44, Philippe Mathieu-Daudé wrote:
..
> This patch fixes CVE-2023-40360 ("QEMU: NVMe: NULL pointer
> dereference in nvme_directive_receive"). Were you aware of
> the security implications?
> 
> Too bad we hadn't committed "Fixes: CVE-2023-40360" as that
> would have helped downstream distributions cherry-picking
> security fixes ASAP, since our stable is not that frequent.

https://tracker.debian.org/news/1455443/accepted-qemu-1804dfsg-2-source-into-unstable/

FWIW.

/mjt

Re: [PULL 1/2] hw/nvme: fix null pointer access in directive receive

[Klaus Jensen]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Aug 24 14:44, Philippe Mathieu-Daudé wrote:
> On 9/8/23 15:39, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > nvme_directive_receive() does not check if an endurance group has been
> > configured (set) prior to testing if flexible data placement is enabled
> > or not.
> > 
> > Fix this.
> > 
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1815
> > Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
> > Reviewed-by: Jesper Wendel Devantier <j.devantier@samsung.com>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> >   hw/nvme/ctrl.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > index d217ae91b506..e5b5c7034d2b 100644
> > --- a/hw/nvme/ctrl.c
> > +++ b/hw/nvme/ctrl.c
> > @@ -6900,7 +6900,7 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n, NvmeRequest *req)
> >       case NVME_DIRECTIVE_IDENTIFY:
> >           switch (doper) {
> >           case NVME_DIRECTIVE_RETURN_PARAMS:
> > -            if (ns->endgrp->fdp.enabled) {
> > +            if (ns->endgrp && ns->endgrp->fdp.enabled) {
> 
> This patch fixes CVE-2023-40360 ("QEMU: NVMe: NULL pointer
> dereference in nvme_directive_receive"). Were you aware of
> the security implications?
> 

Yes, but I was not aware of the CVE being assigned at the time. I don't
think it was?

But if what you are saying is that it was my responsibility as
maintainer, to get that reported and assigned, then I apologies and will
of course keep that in mind going forward!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]