[Qemu-devel] [PATCH] qstring: Fix qstring_from_substr() not to provoke int overflow

liujunjie posted 1 patch 7 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180724134339.17832-1-liujunjie23@huawei.com
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 passed
include/qapi/qmp/qstring.h | 2 +-
qobject/qstring.c          | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] qstring: Fix qstring_from_substr() not to provoke int overflow
Posted by liujunjie 7 years, 3 months ago
qstring_from_substr() parameters @start and @end are of type int.
blkdebug_parse_filename(), blkverify_parse_filename(), nbd_parse_uri(),
and qstring_from_str() pass @end values of type size_t or ptrdiff_t.
Values exceeding INT_MAX get truncated, with possibly disastrous
results.

Such huge substrings seem unlikely, but we found one in a core dump,
where "info tlb" executed via QMP's human-monitor-command apparently
produced 35 GiB of output.

Fix by changing the parameters size_t.

Signed-off-by: liujunjie <liujunjie23@huawei.com>
---
 include/qapi/qmp/qstring.h | 2 +-
 qobject/qstring.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index b3b3d44..3e83e3a 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -24,7 +24,7 @@ struct QString {
 
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
-QString *qstring_from_substr(const char *str, int start, int end);
+QString *qstring_from_substr(const char *str, size_t start, size_t end);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
 const char *qstring_get_try_str(const QString *qstring);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index afca54b..18b8eb8 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -37,7 +37,7 @@ size_t qstring_get_length(const QString *qstring)
  *
  * Return string reference
  */
-QString *qstring_from_substr(const char *str, int start, int end)
+QString *qstring_from_substr(const char *str, size_t start, size_t end)
 {
     QString *qstring;
 
-- 
1.8.3.1



Re: [Qemu-devel] [PATCH] qstring: Fix qstring_from_substr() not to provoke int overflow
Posted by Markus Armbruster 7 years, 3 months ago
liujunjie <liujunjie23@huawei.com> writes:

> qstring_from_substr() parameters @start and @end are of type int.
> blkdebug_parse_filename(), blkverify_parse_filename(), nbd_parse_uri(),
> and qstring_from_str() pass @end values of type size_t or ptrdiff_t.
> Values exceeding INT_MAX get truncated, with possibly disastrous
> results.
>
> Such huge substrings seem unlikely, but we found one in a core dump,
> where "info tlb" executed via QMP's human-monitor-command apparently
> produced 35 GiB of output.
>
> Fix by changing the parameters size_t.
>
> Signed-off-by: liujunjie <liujunjie23@huawei.com>
> ---
>  include/qapi/qmp/qstring.h | 2 +-
>  qobject/qstring.c          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
> index b3b3d44..3e83e3a 100644
> --- a/include/qapi/qmp/qstring.h
> +++ b/include/qapi/qmp/qstring.h
> @@ -24,7 +24,7 @@ struct QString {
>  
>  QString *qstring_new(void);
>  QString *qstring_from_str(const char *str);
> -QString *qstring_from_substr(const char *str, int start, int end);
> +QString *qstring_from_substr(const char *str, size_t start, size_t end);
>  size_t qstring_get_length(const QString *qstring);
>  const char *qstring_get_str(const QString *qstring);
>  const char *qstring_get_try_str(const QString *qstring);
> diff --git a/qobject/qstring.c b/qobject/qstring.c
> index afca54b..18b8eb8 100644
> --- a/qobject/qstring.c
> +++ b/qobject/qstring.c
> @@ -37,7 +37,7 @@ size_t qstring_get_length(const QString *qstring)
>   *
>   * Return string reference
>   */
> -QString *qstring_from_substr(const char *str, int start, int end)
> +QString *qstring_from_substr(const char *str, size_t start, size_t end)
>  {
>      QString *qstring;

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH] qstring: Fix qstring_from_substr() not to provoke int overflow
Posted by Stefan Hajnoczi 7 years, 3 months ago
On Wed, Jul 25, 2018 at 11:21:03AM +0200, Markus Armbruster wrote:
> liujunjie <liujunjie23@huawei.com> writes:
> > diff --git a/qobject/qstring.c b/qobject/qstring.c
> > index afca54b..18b8eb8 100644
> > --- a/qobject/qstring.c
> > +++ b/qobject/qstring.c
> > @@ -37,7 +37,7 @@ size_t qstring_get_length(const QString *qstring)
> >   *
> >   * Return string reference
> >   */
> > -QString *qstring_from_substr(const char *str, int start, int end)
> > +QString *qstring_from_substr(const char *str, size_t start, size_t end)
> >  {
> >      QString *qstring;
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

None more qualified to merge this than you:

$ scripts/get_maintainer.pl -f qobject/qstring.c
Markus Armbruster <armbru@redhat.com> (supporter:QObject)
qemu-devel@nongnu.org (open list:All patches CC here)

:)