| Message ID | 20181206084816.30485-1-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i2c: pm_smbus: check smb_index before block transfer write | expand |
在 2018/12/6 16:48, P J P 写道: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While performing block transfer write in smb_ioport_writeb(), > 'smb_index' is incremented and used to index smb_data[] array. > Check 'smb_index' value to avoid OOB access. > > Reported-by: Michael Hanselmann <public@hansmi.ch> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/i2c/pm_smbus.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c > index 685a2378ed..03062740cc 100644 > --- a/hw/i2c/pm_smbus.c > +++ b/hw/i2c/pm_smbus.c > @@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, > uint8_t read = s->smb_addr & 0x01; > > s->smb_index++; > + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { > + s->smb_index = 0; > + } > if (!read && s->smb_index == s->smb_data0) { > uint8_t prot = (s->smb_ctl >> 2) & 0x07; > uint8_t cmd = s->smb_cmd; Oh... Finally another one find this..... I've already found this. This is very a serious security issue. I have wrote a full exploit to make a VM escape using this vulnerability. This guest can read/write a 4G memory of qemu process by default configuration. As far as I know, this vulnerability may be the most serious vulnerability of the qemu history. Please pay a lot of attention for this issue. Later I will release the full paper and exploit. It's not harm as this is introduced in 3.1 and no one use it now. Thanks, Li Qiang
On Thu, 6 Dec 2018 14:18:16 +0530 P J P <ppandit@redhat.com> wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While performing block transfer write in smb_ioport_writeb(), > 'smb_index' is incremented and used to index smb_data[] array. > Check 'smb_index' value to avoid OOB access. > > Reported-by: Michael Hanselmann <public@hansmi.ch> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Peter, is it possible for fix to make into 3.1? > --- > hw/i2c/pm_smbus.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c > index 685a2378ed..03062740cc 100644 > --- a/hw/i2c/pm_smbus.c > +++ b/hw/i2c/pm_smbus.c > @@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, > uint8_t read = s->smb_addr & 0x01; > > s->smb_index++; > + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { > + s->smb_index = 0; > + } > if (!read && s->smb_index == s->smb_data0) { > uint8_t prot = (s->smb_ctl >> 2) & 0x07; > uint8_t cmd = s->smb_cmd;
On Thu, 6 Dec 2018 14:18:16 +0530 P J P <ppandit@redhat.com> wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While performing block transfer write in smb_ioport_writeb(), > 'smb_index' is incremented and used to index smb_data[] array. > Check 'smb_index' value to avoid OOB access. > > Reported-by: Michael Hanselmann <public@hansmi.ch> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> pls squash in: Fixes: 38ad4fae43 i2c: pm_smbus: Add block transfer capability > --- > hw/i2c/pm_smbus.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c > index 685a2378ed..03062740cc 100644 > --- a/hw/i2c/pm_smbus.c > +++ b/hw/i2c/pm_smbus.c > @@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, > uint8_t read = s->smb_addr & 0x01; > > s->smb_index++; > + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { > + s->smb_index = 0; > + } > if (!read && s->smb_index == s->smb_data0) { > uint8_t prot = (s->smb_ctl >> 2) & 0x07; > uint8_t cmd = s->smb_cmd;
On Thu, 6 Dec 2018 at 09:48, Igor Mammedov <imammedo@redhat.com> wrote: > > On Thu, 6 Dec 2018 14:18:16 +0530 > P J P <ppandit@redhat.com> wrote: > > > From: Prasad J Pandit <pjp@fedoraproject.org> > > > > While performing block transfer write in smb_ioport_writeb(), > > 'smb_index' is incremented and used to index smb_data[] array. > > Check 'smb_index' value to avoid OOB access. > > > > Reported-by: Michael Hanselmann <public@hansmi.ch> > > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > Peter, > is it possible for fix to make into 3.1? This question would have been much better asked on Tuesday... thanks -- PMM
FYI: http://terenceli.github.io/%E6%8A%80%E6%9C%AF/2018/12/06/qemu-escape 在 2018/12/6 17:02, li qiang 写道: > 在 2018/12/6 16:48, P J P 写道: >> From: Prasad J Pandit <pjp@fedoraproject.org> >> >> While performing block transfer write in smb_ioport_writeb(), >> 'smb_index' is incremented and used to index smb_data[] array. >> Check 'smb_index' value to avoid OOB access. >> >> Reported-by: Michael Hanselmann <public@hansmi.ch> >> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> >> --- >> hw/i2c/pm_smbus.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c >> index 685a2378ed..03062740cc 100644 >> --- a/hw/i2c/pm_smbus.c >> +++ b/hw/i2c/pm_smbus.c >> @@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, >> uint8_t read = s->smb_addr & 0x01; >> >> s->smb_index++; >> + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { >> + s->smb_index = 0; >> + } >> if (!read && s->smb_index == s->smb_data0) { >> uint8_t prot = (s->smb_ctl >> 2) & 0x07; >> uint8_t cmd = s->smb_cmd; > Oh... Finally another one find this..... > > I've already found this. This is very a serious security issue. > > I have wrote a full exploit to make a VM escape using this vulnerability. > > This guest can read/write a 4G memory of qemu process by default > configuration. > > As far as I know, this vulnerability may be the most serious > vulnerability of the qemu history. > > Please pay a lot of attention for this issue. > > Later I will release the full paper and exploit. It's not harm as this > is introduced in 3.1 > > and no one use it now. > > > Thanks, > > Li Qiang > > > >
On Thu, 6 Dec 2018 at 09:10, li qiang <liq3ea@outlook.com> wrote: > Oh... Finally another one find this..... > > I've already found this. This is very a serious security issue. If you find a security issue, we would appreciate it if you let us know, rather than just waiting to see if anybody else notices it... thanks -- PMM
在 2018/12/6 18:16, Peter Maydell 写道: > On Thu, 6 Dec 2018 at 09:10, li qiang<liq3ea@outlook.com> wrote: >> Oh... Finally another one find this..... >> >> I've already found this. This is very a serious security issue. > If you find a security issue, we would appreciate it if > you let us know, rather than just waiting to see if > anybody else notices it... I'm sorry for that. To be perfect, I'm currently trying to write an exp for i440fx. I intend report it after completing it. Thanks, Li Qiang > thanks > -- PMM
On Thu, 6 Dec 2018 at 10:34, li qiang <liq3ea@outlook.com> wrote: > > > 在 2018/12/6 18:16, Peter Maydell 写道: > > On Thu, 6 Dec 2018 at 09:10, li qiang<liq3ea@outlook.com> wrote: > >> Oh... Finally another one find this..... > >> > >> I've already found this. This is very a serious security issue. > > If you find a security issue, we would appreciate it if > > you let us know, rather than just waiting to see if > > anybody else notices it... > > I'm sorry for that. > > To be perfect, I'm currently trying to write an exp for i440fx. > > I intend report it after completing it. Is that an exploit for this bug, or for some other bug? It's OK (and we recommend) reporting security issues as "I think this is probably exploitable"; we don't require a proof-of-concept for security bugs. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> 于2018年12月6日周四 下午6:46写道： > On Thu, 6 Dec 2018 at 10:34, li qiang <liq3ea@outlook.com> wrote: > > > > > > 在 2018/12/6 18:16, Peter Maydell 写道: > > > On Thu, 6 Dec 2018 at 09:10, li qiang<liq3ea@outlook.com> wrote: > > >> Oh... Finally another one find this..... > > >> > > >> I've already found this. This is very a serious security issue. > > > If you find a security issue, we would appreciate it if > > > you let us know, rather than just waiting to see if > > > anybody else notices it... > > > > I'm sorry for that. > > > > To be perfect, I'm currently trying to write an exp for i440fx. > > > > I intend report it after completing it. > > Is that an exploit for this bug, or for some other bug? Just for this. But for now still no exp for i440fx. > It's OK (and we recommend) reporting security issues as > "I think this is probably exploitable"; we don't require > a proof-of-concept for security bugs. > Yes, I know that, but as this issue is so good to write a perfect exploit so I want to do more. I know the qemu planing and know this issue doesn't affect anyone. I want to do a perfect work. Thanks, Li Qiang > > thanks > -- PMM >
On Thu, 6 Dec 2018 at 11:00, Li Qiang <liq3ea@gmail.com> wrote: > Yes, I know that, but as this issue is so good to write a perfect exploit > so I want to do more. > > I know the qemu planing and know this issue doesn't affect anyone. > I want to do a perfect work. The problem is that it does affect other people, because if Michael hadn't happened to also notice this bug then we would have released 3.1 next Tuesday with the bug still present. As it is we'll need to do an extra rc5, which we could have avoided if we'd known about this bug earlier, on Monday or Tuesday. thanks -- PMM
+-- On Thu, 6 Dec 2018, Igor Mammedov wrote --+ | > From: Prasad J Pandit <pjp@fedoraproject.org> | > | > While performing block transfer write in smb_ioport_writeb(), | > 'smb_index' is incremented and used to index smb_data[] array. | > Check 'smb_index' value to avoid OOB access. | > | > Reported-by: Michael Hanselmann <public@hansmi.ch> | > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> | pls squash in: | | Fixes: 38ad4fae43 i2c: pm_smbus: Add block transfer capability Do we need patch v2, or it can be done while merging it? Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Peter Maydell <peter.maydell@linaro.org> 于2018年12月6日周四 下午7:05写道： > On Thu, 6 Dec 2018 at 11:00, Li Qiang <liq3ea@gmail.com> wrote: > > Yes, I know that, but as this issue is so good to write a perfect exploit > > so I want to do more. > > > > I know the qemu planing and know this issue doesn't affect anyone. > > I want to do a perfect work. > > The problem is that it does affect other people, because if > Michael hadn't happened to also notice this bug then we would > have released 3.1 next Tuesday with the bug still present. > As it is we'll need to do an extra rc5, which we could have > avoided if we'd known about this bug earlier, on Monday or Tuesday. > OK, next time I will report it directly like what I did before. Thanks, Li Qiang > > thanks > -- PMM >
On Thu, 6 Dec 2018 at 11:12, Li Qiang <liq3ea@gmail.com> wrote:
> OK, next time I will report it directly like what I did before.
Thank you -- I appreciate that.
-- PMM
On Thu, 6 Dec 2018 at 11:10, P J P <ppandit@redhat.com> wrote: > > +-- On Thu, 6 Dec 2018, Igor Mammedov wrote --+ > | > From: Prasad J Pandit <pjp@fedoraproject.org> > | > > | > While performing block transfer write in smb_ioport_writeb(), > | > 'smb_index' is incremented and used to index smb_data[] array. > | > Check 'smb_index' value to avoid OOB access. > | > > | > Reported-by: Michael Hanselmann <public@hansmi.ch> > | > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > | pls squash in: > | > | Fixes: 38ad4fae43 i2c: pm_smbus: Add block transfer capability > > Do we need patch v2, or it can be done while merging it? I can add in the Fixes line when I apply the patch to master. It would be good if we could get some Reviewed-by: tags as well. I think my current view is that we should apply this today, and tag rc5 this afternoon (or maybe tomorrow?), and do the final release on schedule on Tuesday. Is there anything else in the pipeline that should go in rc5? thanks -- PMM
On Thu, 6 Dec 2018 at 11:19, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 6 Dec 2018 at 11:10, P J P <ppandit@redhat.com> wrote: > > > > +-- On Thu, 6 Dec 2018, Igor Mammedov wrote --+ > > | > From: Prasad J Pandit <pjp@fedoraproject.org> > > | > > > | > While performing block transfer write in smb_ioport_writeb(), > > | > 'smb_index' is incremented and used to index smb_data[] array. > > | > Check 'smb_index' value to avoid OOB access. > > | > > > | > Reported-by: Michael Hanselmann <public@hansmi.ch> > > | > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > > | pls squash in: > > | > > | Fixes: 38ad4fae43 i2c: pm_smbus: Add block transfer capability > > > > Do we need patch v2, or it can be done while merging it? > > I can add in the Fixes line when I apply the patch to master. Oh, I think we should also add to the commit message something along the lines of: "Note that this bug is exploitable by a guest to escape from the virtual machine. However the commit which introduced the bug was only made after the 3.0 release, and so it is not present in any released QEMU version." to clarify that this is a serious bug but also that it's not one that will be affecting anybody's production systems. thanks -- PMM
在 2018/12/6 16:48, P J P 写道: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While performing block transfer write in smb_ioport_writeb(), > 'smb_index' is incremented and used to index smb_data[] array. > Check 'smb_index' value to avoid OOB access. > > Reported-by: Michael Hanselmann <public@hansmi.ch> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> Reviewed-by: Li Qiang <liq3ea@gmail.com> > --- > hw/i2c/pm_smbus.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c > index 685a2378ed..03062740cc 100644 > --- a/hw/i2c/pm_smbus.c > +++ b/hw/i2c/pm_smbus.c > @@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, > uint8_t read = s->smb_addr & 0x01; > > s->smb_index++; > + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { > + s->smb_index = 0; > + } > if (!read && s->smb_index == s->smb_data0) { > uint8_t prot = (s->smb_ctl >> 2) & 0x07; > uint8_t cmd = s->smb_cmd;
On 06.12.18 09:48, P J P wrote: > Reported-by: Michael Hanselmann <public@hansmi.ch> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> Reviewed-by: Michael Hanselmann <public@hansmi.ch> Best regards, Michael
+-- On Thu, 6 Dec 2018, Peter Maydell wrote --+ | > > Do we need patch v2, or it can be done while merging it? | > | > I can add in the Fixes line when I apply the patch to master. | | Oh, I think we should also add to the commit message something | along the lines of: | | "Note that this bug is exploitable by a guest to escape | from the virtual machine. However the commit which | introduced the bug was only made after the 3.0 release, | and so it is not present in any released QEMU version." | | to clarify that this is a serious bug but also that it's | not one that will be affecting anybody's production systems. Okay, preparing patch v2... -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
+-- On Thu, 6 Dec 2018, P J P wrote --+ | | to clarify that this is a serious bug but also that it's | | not one that will be affecting anybody's production systems. | | Okay, preparing patch v2... Sent revised patch [PATCH v1] i2c: pm_smbus: check smb_index before block transfer write Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
On 06.12.18 09:48, P J P wrote: > While performing block transfer write in smb_ioport_writeb(), > 'smb_index' is incremented and used to index smb_data[] array. > Check 'smb_index' value to avoid OOB access. > > Reported-by: Michael Hanselmann <public@hansmi.ch> Considering that Li Qiang had already published his exploit for a couple of hours (at the time of writing the URL is returning an HTTP 404 though I'd seen it earlier) and with the patch being public I decided to also publish my report: https://hansmi.ch/articles/2018-12-qemu-pm-smbus-oob I'd like to thank Prasad and his colleagues at Red Hat for the quick response to my report (patch committed within less than 18 hours). Best regards, Michael
diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c index 685a2378ed..03062740cc 100644 --- a/hw/i2c/pm_smbus.c +++ b/hw/i2c/pm_smbus.c @@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, uint8_t read = s->smb_addr & 0x01; s->smb_index++; + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { + s->smb_index = 0; + } if (!read && s->smb_index == s->smb_data0) { uint8_t prot = (s->smb_ctl >> 2) & 0x07; uint8_t cmd = s->smb_cmd;