| Message ID | 20211117123534.2900334-2-philmd@redhat.com |
|---|---|
| State | New |
| 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 > Reported-by: Qiuhao Li <Qiuhao.Li@outlook.com> > Fixes: f432fdfa121 ("support changed namespace asynchronous event") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/nvme/ctrl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index 6a571d18cfa..93a24464647 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -4168,8 +4168,11 @@ static uint16_t nvme_changed_nslist(NvmeCtrl *n, uint8_t rae, uint32_t buf_len, > int i = 0; > uint32_t nsid; > > - memset(nslist, 0x0, sizeof(nslist)); > trans_len = MIN(sizeof(nslist) - off, buf_len); > + if (trans_len >= sizeof(nslist)) { > + return NVME_INVALID_FIELD | NVME_DNR; > + } > + memset(nslist, 0x0, sizeof(nslist)); > > while ((nsid = find_first_bit(n->changed_nsids, NVME_CHANGED_NSID_SIZE)) != > NVME_CHANGED_NSID_SIZE) { The issue I mentioned with off > sizeof(nslist). I'll send a fix that just deals with it like all the other log pages. There is probably a better way to do these checks, but I'm not sure how right now.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 6a571d18cfa..93a24464647 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -4168,8 +4168,11 @@ static uint16_t nvme_changed_nslist(NvmeCtrl *n, uint8_t rae, uint32_t buf_len, int i = 0; uint32_t nsid; - memset(nslist, 0x0, sizeof(nslist)); trans_len = MIN(sizeof(nslist) - off, buf_len); + if (trans_len >= sizeof(nslist)) { + return NVME_INVALID_FIELD | NVME_DNR; + } + memset(nslist, 0x0, sizeof(nslist)); while ((nsid = find_first_bit(n->changed_nsids, NVME_CHANGED_NSID_SIZE)) != NVME_CHANGED_NSID_SIZE) {
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 Reported-by: Qiuhao Li <Qiuhao.Li@outlook.com> Fixes: f432fdfa121 ("support changed namespace asynchronous event") Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/nvme/ctrl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)