| Message ID | 20211117123534.2900334-3-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/nvme/ctrl: Fix buffer overrun (CVE-2021-3947) | expand |
On Nov 17 13:35, Philippe Mathieu-Daudé wrote: > Both 'buf_len' and 'off' arguments are under guest control. > Since nvme_c2h() doesn't check out of boundary access, the > caller must check for eventual buffer overrun on 'trans_len'. > > Cc: qemu-stable@nongnu.org > Fixes: 94a7897c41d ("add support for the get log page command") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/nvme/ctrl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index 93a24464647..7414f3b4dd1 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -4146,7 +4146,8 @@ static uint16_t nvme_error_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len, > uint32_t trans_len; > NvmeErrorLog errlog; > > - if (off >= sizeof(errlog)) { > + trans_len = MIN(sizeof(errlog) - off, buf_len); > + if (trans_len >= sizeof(errlog)) { > return NVME_INVALID_FIELD | NVME_DNR; > } > > @@ -4155,7 +4156,6 @@ static uint16_t nvme_error_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len, > } > > memset(&errlog, 0x0, sizeof(errlog)); > - trans_len = MIN(sizeof(errlog) - off, buf_len); > > return nvme_c2h(n, (uint8_t *)&errlog, trans_len, req); > } > -- > 2.31.1 > I don't see any buffer overrun issue prior to this patch. However, there is a functional bug since the offset is not added in the c2h.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 93a24464647..7414f3b4dd1 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -4146,7 +4146,8 @@ static uint16_t nvme_error_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len, uint32_t trans_len; NvmeErrorLog errlog; - if (off >= sizeof(errlog)) { + trans_len = MIN(sizeof(errlog) - off, buf_len); + if (trans_len >= sizeof(errlog)) { return NVME_INVALID_FIELD | NVME_DNR; } @@ -4155,7 +4156,6 @@ static uint16_t nvme_error_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len, } memset(&errlog, 0x0, sizeof(errlog)); - trans_len = MIN(sizeof(errlog) - off, buf_len); return nvme_c2h(n, (uint8_t *)&errlog, trans_len, req); }
Both 'buf_len' and 'off' arguments are under guest control. Since nvme_c2h() doesn't check out of boundary access, the caller must check for eventual buffer overrun on 'trans_len'. Cc: qemu-stable@nongnu.org Fixes: 94a7897c41d ("add support for the get log page command") Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/nvme/ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)