diff mbox series

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

Message ID 20230809133909.45818-5-its@irrelevant.dk
State New
Headers show
Series [PULL,1/2] hw/nvme: fix null pointer access in directive receive | expand

Commit Message

Klaus Jensen Aug. 9, 2023, 1:39 p.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé Aug. 24, 2023, 12:44 p.m. UTC | #1 
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;
Michael Tokarev Aug. 24, 2023, 1:08 p.m. UTC | #2 
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
Klaus Jensen Aug. 24, 2023, 1:27 p.m. UTC | #3 
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!
diff mbox series

Patch

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;