The declaration of local variable 'bytes' in 'hvmemul_rep_stos' causes
the shadowing of the same variable defined in the enclosing scope,
hence the declaration has been moved inside the scope where it's used,
with a different name.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
xen/arch/x86/hvm/emulate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 75ee98a73b..0d41928ff3 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -2024,15 +2024,15 @@ static int cf_check hvmemul_rep_stos(
switch ( p2mt )
{
- unsigned long bytes;
char *buf;
default:
/* Allocate temporary buffer. */
for ( ; ; )
{
- bytes = *reps * bytes_per_rep;
- buf = xmalloc_bytes(bytes);
+ unsigned long bytes_tmp;
+ bytes_tmp = *reps * bytes_per_rep;
+ buf = xmalloc_bytes(bytes_tmp);
if ( buf || *reps <= 1 )
break;
*reps >>= 1;
--
2.34.1On 27.07.2023 12:48, Nicola Vetrini wrote:
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2024,15 +2024,15 @@ static int cf_check hvmemul_rep_stos(
>
> switch ( p2mt )
> {
> - unsigned long bytes;
> char *buf;
>
> default:
> /* Allocate temporary buffer. */
> for ( ; ; )
> {
> - bytes = *reps * bytes_per_rep;
> - buf = xmalloc_bytes(bytes);
> + unsigned long bytes_tmp;
> + bytes_tmp = *reps * bytes_per_rep;
> + buf = xmalloc_bytes(bytes_tmp);
> if ( buf || *reps <= 1 )
> break;
> *reps >>= 1;
This wants dealing with differently - the outer scope variable is unused
(only written to) afaics. Eliminating it will, aiui, address another
violation at the same time. And then the same in hvmemul_rep_movs(), just
that there the variable itself needs to survive. I guess I'll make a
patch ...
Jan
On 27/07/23 17:06, Jan Beulich wrote:
> On 27.07.2023 12:48, Nicola Vetrini wrote:
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -2024,15 +2024,15 @@ static int cf_check hvmemul_rep_stos(
>>
>> switch ( p2mt )
>> {
>> - unsigned long bytes;
>> char *buf;
>>
>> default:
>> /* Allocate temporary buffer. */
>> for ( ; ; )
>> {
>> - bytes = *reps * bytes_per_rep;
>> - buf = xmalloc_bytes(bytes);
>> + unsigned long bytes_tmp;
>> + bytes_tmp = *reps * bytes_per_rep;
>> + buf = xmalloc_bytes(bytes_tmp);
>> if ( buf || *reps <= 1 )
>> break;
>> *reps >>= 1;
>
> This wants dealing with differently - the outer scope variable is unused
> (only written to) afaics. Eliminating it will, aiui, address another
> violation at the same time. And then the same in hvmemul_rep_movs(), just
> that there the variable itself needs to survive. I guess I'll make a
> patch ...
>
> Jan
Wouldn't this code at line ~2068 be possibly affected by writing to
bytes, if the outer variable is used?
/* Adjust address for reverse store. */
if ( df )
gpa -= bytes - bytes_per_rep;
rc = hvm_copy_to_guest_phys(gpa, buf, bytes, curr);
You're right about the other violation (R2.1)
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
On 27.07.2023 17:22, Nicola Vetrini wrote:
>
>
> On 27/07/23 17:06, Jan Beulich wrote:
>> On 27.07.2023 12:48, Nicola Vetrini wrote:
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -2024,15 +2024,15 @@ static int cf_check hvmemul_rep_stos(
>>>
>>> switch ( p2mt )
>>> {
>>> - unsigned long bytes;
>>> char *buf;
>>>
>>> default:
>>> /* Allocate temporary buffer. */
>>> for ( ; ; )
>>> {
>>> - bytes = *reps * bytes_per_rep;
>>> - buf = xmalloc_bytes(bytes);
>>> + unsigned long bytes_tmp;
>>> + bytes_tmp = *reps * bytes_per_rep;
>>> + buf = xmalloc_bytes(bytes_tmp);
>>> if ( buf || *reps <= 1 )
>>> break;
>>> *reps >>= 1;
>>
>> This wants dealing with differently - the outer scope variable is unused
>> (only written to) afaics. Eliminating it will, aiui, address another
>> violation at the same time. And then the same in hvmemul_rep_movs(), just
>> that there the variable itself needs to survive. I guess I'll make a
>> patch ...
>
> Wouldn't this code at line ~2068 be possibly affected by writing to
> bytes, if the outer variable is used?
Which outer variable? I'm suggesting to drop that (see the patch that
I've sent already).
Jan
> /* Adjust address for reverse store. */
> if ( df )
> gpa -= bytes - bytes_per_rep;
>
> rc = hvm_copy_to_guest_phys(gpa, buf, bytes, curr);
>
> You're right about the other violation (R2.1)
>
On 27/07/23 17:31, Jan Beulich wrote:
> On 27.07.2023 17:22, Nicola Vetrini wrote:
>>
>>
>> On 27/07/23 17:06, Jan Beulich wrote:
>>> On 27.07.2023 12:48, Nicola Vetrini wrote:
>>>> --- a/xen/arch/x86/hvm/emulate.c
>>>> +++ b/xen/arch/x86/hvm/emulate.c
>>>> @@ -2024,15 +2024,15 @@ static int cf_check hvmemul_rep_stos(
>>>>
>>>> switch ( p2mt )
>>>> {
>>>> - unsigned long bytes;
>>>> char *buf;
>>>>
>>>> default:
>>>> /* Allocate temporary buffer. */
>>>> for ( ; ; )
>>>> {
>>>> - bytes = *reps * bytes_per_rep;
>>>> - buf = xmalloc_bytes(bytes);
>>>> + unsigned long bytes_tmp;
>>>> + bytes_tmp = *reps * bytes_per_rep;
>>>> + buf = xmalloc_bytes(bytes_tmp);
>>>> if ( buf || *reps <= 1 )
>>>> break;
>>>> *reps >>= 1;
>>>
>>> This wants dealing with differently - the outer scope variable is unused
>>> (only written to) afaics. Eliminating it will, aiui, address another
>>> violation at the same time. And then the same in hvmemul_rep_movs(), just
>>> that there the variable itself needs to survive. I guess I'll make a
>>> patch ...
>>
>> Wouldn't this code at line ~2068 be possibly affected by writing to
>> bytes, if the outer variable is used?
>
> Which outer variable? I'm suggesting to drop that (see the patch that
> I've sent already).
>
> Jan
>
>> /* Adjust address for reverse store. */
>> if ( df )
>> gpa -= bytes - bytes_per_rep;
>>
>> rc = hvm_copy_to_guest_phys(gpa, buf, bytes, curr);
>>
>> You're right about the other violation (R2.1)
>>
>
I see, sorry for the noise.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
© 2016 - 2024 Red Hat, Inc.