Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Ping? 26.01.2016 12:36, Michael Tokarev wrote: > 18.01.2016 17:23, Markus Armbruster wrote: > [...] >> Applied to my monitor-next with these tweaks: >> >> diff --git a/hmp.c b/hmp.c >> index 8be03df..9c571f5 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -1739,7 +1739,7 @@ void hmp_sendkey(Monitor

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Michael Tokarev writes: > Ping? Sorry for the delay, I've been focusing on the QAPI queue to the exclusion of pretty much everything else. > 26.01.2016 12:36, Michael Tokarev wrote: >> 18.01.2016 17:23, Markus Armbruster wrote: >> [...] >>> Applied to my monitor-next with

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

18.01.2016 17:23, Markus Armbruster wrote: [...] > Applied to my monitor-next with these tweaks: > > diff --git a/hmp.c b/hmp.c > index 8be03df..9c571f5 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1739,7 +1739,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict) > keyname_len = separator

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Wolfgang Bumiller writes: > On Mon, Jan 18, 2016 at 02:02:07PM +0100, Markus Armbruster wrote: >> Wolfgang Bumiller writes: >> >> > On Tue, Jan 12, 2016 at 05:52:38PM +0100, Markus Armbruster wrote: >> >> Wolfgang Bumiller

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Mon, Jan 18, 2016 at 02:02:07PM +0100, Markus Armbruster wrote: > Wolfgang Bumiller writes: > > > On Tue, Jan 12, 2016 at 05:52:38PM +0100, Markus Armbruster wrote: > >> Wolfgang Bumiller writes: > >> > > while (1) { > >

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Wolfgang Bumiller writes: > On Tue, Jan 12, 2016 at 05:52:38PM +0100, Markus Armbruster wrote: >> Wolfgang Bumiller writes: >> >> >> On January 12, 2016 at 5:00 PM Markus Armbruster >> >> wrote: >> >> Will you prepare a

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Tue, Jan 12, 2016 at 05:52:38PM +0100, Markus Armbruster wrote: > Wolfgang Bumiller writes: > > >> On January 12, 2016 at 5:00 PM Markus Armbruster wrote: > >> Will you prepare a revised patch? > > > > Can do that tomorrow, but which option is the

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Wolfgang Bumiller writes: > On Sun, Jan 10, 2016 at 10:56:55AM +0300, Michael Tokarev wrote: >> So, what's the status of this issue now? >> (it is CVE-2015-8619 btw, maybe worth to mention this in the commit message) > > Seems we concluded it's best to keep keyname_len

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

> On January 12, 2016 at 9:45 AM Markus Armbruster wrote: > > Wolfgang Bumiller writes: > > > When processing 'sendkey' command, hmp_sendkey routine null > > terminates the 'keyname_buf' array. This results in an OOB > > Well, it technically doesn't

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Luiz Capitulino writes: > On Thu, 17 Dec 2015 18:10:59 +0530 (IST) > P J P wrote: > >>Hello, >> >> An OOB write issue was reported by Mr Ling Liu, CC'd here. It occurs while >> processing the 'sendkey' command, if the command argument was longer

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Wolfgang Bumiller writes: >> On January 12, 2016 at 9:45 AM Markus Armbruster wrote: >> >> Wolfgang Bumiller writes: >> >> > When processing 'sendkey' command, hmp_sendkey routine null >> > terminates the 'keyname_buf' array.

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Wolfgang Bumiller writes: >> On January 12, 2016 at 5:00 PM Markus Armbruster wrote: >> separator = strchr(keys, '-'); >> keyname_len = separator ? separator - keys : strlen(keys); >> pstrcpy(keyname_buf, sizeof(keyname_buf),

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

> On January 12, 2016 at 5:00 PM Markus Armbruster wrote: > separator = strchr(keys, '-'); > keyname_len = separator ? separator - keys : strlen(keys); > pstrcpy(keyname_buf, sizeof(keyname_buf), keys); > [...] > keyname_buf[keyname_len]

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Sun, Jan 10, 2016 at 10:56:55AM +0300, Michael Tokarev wrote: > So, what's the status of this issue now? > (it is CVE-2015-8619 btw, maybe worth to mention this in the commit message) Seems we concluded it's best to keep keyname_len around and simply check it against the sizeof(keyname_buf).

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

+-- On Mon, 11 Jan 2016, Wolfgang Bumiller wrote --+ | Seems we concluded it's best to keep keyname_len around and simply check it | against the sizeof(keyname_buf). | | Here's a full new version as I haven't seen one yet. (With an adapted commit | message and the CVE id added.) Sorry, i

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Hello, +-- On Sun, 10 Jan 2016, Michael Tokarev wrote --+ | So, what's the status of this issue now? | (it is CVE-2015-8619 btw, maybe worth to mention this in the commit message) Yes, if the patch is not yet merged upstream, it'd be good to include this CVE in the commit message. --

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

+-- On Sat, 9 Jan 2016, Wolfgang Bumiller wrote --+ | > could say: if (!strcmp(keyname_buf, "<")). | | keyname_len+1 (size instead of length) to include the \0, then yes I think | strcmp can be used this way. The +1 should be fine there (since >= covers | it). Yes, right. -- - P J P 47AF CE69

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

> On January 8, 2016 at 6:32 PM P J P wrote: > > > +-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+ > | On Fri, Jan 08, 2016 at 07:29:31PM +0530, P J P wrote: > | > + if (!strncmp(keyname_buf, "<-", 2)) > | > and remove the 'keyname_len' altogether. > | > | This

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

So, what's the status of this issue now? (it is CVE-2015-8619 btw, maybe worth to mention this in the commit message) Thanks, /mjt

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Thu, Dec 17, 2015 at 06:10:59PM +0530, P J P wrote: > Hello, > > An OOB write issue was reported by Mr Ling Liu, CC'd here. It occurs while > processing the 'sendkey' command, if the command argument was longer than > the 'keyname_buf[16]' buffer. > > === > From

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Fri, Jan 08, 2016 at 05:49:51PM +0530, P J P wrote: >Hello, > > +-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+ > | > if (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) { > | > pstrcpy(keyname_buf, sizeof(keyname_buf), "less"); > | > -keyname_len =

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

+-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+ | Ah yes, how could I miss that. Maybe just add a min() around the | keyname_len computation? | | - keyname_len = separator ? separator - keys : strlen(keys); | + keyname_len = MIN(sizeof(keyname_buf), separator ? separator - keys :

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Fri, Jan 08, 2016 at 07:29:31PM +0530, P J P wrote: > +-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+ > | Ah yes, how could I miss that. Maybe just add a min() around the > | keyname_len computation? > | > | - keyname_len = separator ? separator - keys : strlen(keys); > | + keyname_len =

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Hello, +-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+ | > if (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) { | > pstrcpy(keyname_buf, sizeof(keyname_buf), "less"); | > -keyname_len = 4; | | keyname_buf is a char[16] so 4 will not overflow it. | |

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

+-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+ | On Fri, Jan 08, 2016 at 07:29:31PM +0530, P J P wrote: | > + if (!strncmp(keyname_buf, "<-", 2)) | > and remove the 'keyname_len' altogether. | | This wouldn't catch '<' without '-'. (`sendkey <`) | Also, strncmp with a length of 1 (in the

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

On Thu, 17 Dec 2015 18:10:59 +0530 (IST) P J P wrote: >Hello, > > An OOB write issue was reported by Mr Ling Liu, CC'd here. It occurs while > processing the 'sendkey' command, if the command argument was longer than > the 'keyname_buf[16]' buffer. Markus, are you

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Hello Prasad, Can you give this a cve id? Thank you. -Original Message- From: P J P [mailto:ppan...@redhat.com] Sent: Thursday, December 17, 2015 8:41 PM To: qemu-devel@nongnu.org Cc: 刘令 Subject: [PATCH] hmp: avoid redundant null termination of buffer Hello, An OOB write issue was

[Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Hello, An OOB write issue was reported by Mr Ling Liu, CC'd here. It occurs while processing the 'sendkey' command, if the command argument was longer than the 'keyname_buf[16]' buffer. === From b0363f4c0e91671064dd7ffece8a6923c8dcaf20 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit

Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

Hello Ling, +-- On Fri, 18 Dec 2015, 刘令 wrote --+ | Can you give this a cve id? Yes, I'll request one once it is accepted upstream. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F