Patchwork [23/29] monitor: fail when 'i' type is greater than 32-bit

login
register
mail settings
Submitter Luiz Capitulino
Date Aug. 13, 2009, 1:50 p.m.
Message ID <1250171428-29308-24-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/31326/
State Superseded
Headers show

Comments

Luiz Capitulino - Aug. 13, 2009, 1:50 p.m.
The 'i' argument type is for 32-bit only and most handlers
will use an 'int' to store its value.

It's better to fail gracefully when the user enters a value
greater than 32-bit than to get subtle casting bugs.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Avi Kivity - Aug. 13, 2009, 1:59 p.m.
On 08/13/2009 04:50 PM, Luiz Capitulino wrote:
> The 'i' argument type is for 32-bit only and most handlers
> will use an 'int' to store its value.
>
> It's better to fail gracefully when the user enters a value
> greater than 32-bit than to get subtle casting bugs.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   monitor.c |    6 ++++++
>   1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index e736de4..2052c00 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>                   }
>                   if (get_expr(mon,&val,&p))
>                       goto fail;
> +                /* Check if 'i' is greater than 32-bit */
> +                if ((c == 'i')&&  ((val>>  32)&  0xffffffff)) {
> +                    monitor_printf(mon, "\'%s\' has failed: ", cmdname);
> +                    monitor_printf(mon, "integer is for 32-bit values\n");
> +                    goto fail;
> +                }
>                   qdict_add_qint(qdict, key, qint_from_int64(val));
>               }
>    

What about the commands that do allow 64-bit values, like memory inspection?
Luiz Capitulino - Aug. 13, 2009, 2:18 p.m.
On Thu, 13 Aug 2009 16:59:46 +0300
Avi Kivity <avi@redhat.com> wrote:

> On 08/13/2009 04:50 PM, Luiz Capitulino wrote:
> > The 'i' argument type is for 32-bit only and most handlers
> > will use an 'int' to store its value.
> >
> > It's better to fail gracefully when the user enters a value
> > greater than 32-bit than to get subtle casting bugs.
> >
> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > ---
> >   monitor.c |    6 ++++++
> >   1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index e736de4..2052c00 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >                   }
> >                   if (get_expr(mon,&val,&p))
> >                       goto fail;
> > +                /* Check if 'i' is greater than 32-bit */
> > +                if ((c == 'i')&&  ((val>>  32)&  0xffffffff)) {
> > +                    monitor_printf(mon, "\'%s\' has failed: ", cmdname);
> > +                    monitor_printf(mon, "integer is for 32-bit values\n");
> > +                    goto fail;
> > +                }
> >                   qdict_add_qint(qdict, key, qint_from_int64(val));
> >               }
> >    
> 
> What about the commands that do allow 64-bit values, like memory inspection?

 They work with the long type ('l'), so the check will be false
as it checks for 'i'.
Avi Kivity - Aug. 13, 2009, 2:43 p.m.
On 08/13/2009 05:18 PM, Luiz Capitulino wrote:
> On Thu, 13 Aug 2009 16:59:46 +0300
> Avi Kivity<avi@redhat.com>  wrote:
>
>    
>> On 08/13/2009 04:50 PM, Luiz Capitulino wrote:
>>      
>>> The 'i' argument type is for 32-bit only and most handlers
>>> will use an 'int' to store its value.
>>>
>>> It's better to fail gracefully when the user enters a value
>>> greater than 32-bit than to get subtle casting bugs.
>>>
>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>> ---
>>>    monitor.c |    6 ++++++
>>>    1 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index e736de4..2052c00 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>>                    }
>>>                    if (get_expr(mon,&val,&p))
>>>                        goto fail;
>>> +                /* Check if 'i' is greater than 32-bit */
>>> +                if ((c == 'i')&&   ((val>>   32)&   0xffffffff)) {
>>> +                    monitor_printf(mon, "\'%s\' has failed: ", cmdname);
>>> +                    monitor_printf(mon, "integer is for 32-bit values\n");
>>> +                    goto fail;
>>> +                }
>>>                    qdict_add_qint(qdict, key, qint_from_int64(val));
>>>                }
>>>
>>>        
>> What about the commands that do allow 64-bit values, like memory inspection?
>>      
>
>   They work with the long type ('l'), so the check will be false
> as it checks for 'i'.
>    

Ah, ok.

Patch

diff --git a/monitor.c b/monitor.c
index e736de4..2052c00 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2820,6 +2820,12 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 }
                 if (get_expr(mon, &val, &p))
                     goto fail;
+                /* Check if 'i' is greater than 32-bit */
+                if ((c == 'i') && ((val >> 32) & 0xffffffff)) {
+                    monitor_printf(mon, "\'%s\' has failed: ", cmdname);
+                    monitor_printf(mon, "integer is for 32-bit values\n");
+                    goto fail;
+                }
                 qdict_add_qint(qdict, key, qint_from_int64(val));
             }
             break;