[PATCH v1] x86/domctl: remove impossible condition in XEN_DOMCTL_getpageframeinfo3

[PATCH v1] x86/domctl: remove impossible condition in XEN_DOMCTL_getpageframeinfo3

[Olaf Hering]

The value of num is always the same as domctl->u.getpageframeinfo3.num,
it was assigned just a few lines before.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 xen/arch/x86/domctl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 8fbbf3aeb3..46d288a490 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -421,8 +421,7 @@ long arch_do_domctl(
         /* Games to allow this code block to handle a compat guest. */
         void __user *guest_handle = domctl->u.getpageframeinfo3.array.p;
 
-        if ( unlikely(num > 1024) ||
-             unlikely(num != domctl->u.getpageframeinfo3.num) )
+        if ( unlikely(num > 1024) )
         {
             ret = -E2BIG;
             break;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1] x86/domctl: remove impossible condition in XEN_DOMCTL_getpageframeinfo3

[Andrew Cooper]

On 09/03/18 16:01, Olaf Hering wrote:
> The value of num is always the same as domctl->u.getpageframeinfo3.num,
> it was assigned just a few lines before.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

This isn't dead code.  It is a truncation check.

~Andrew

> ---
>  xen/arch/x86/domctl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 8fbbf3aeb3..46d288a490 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -421,8 +421,7 @@ long arch_do_domctl(
>          /* Games to allow this code block to handle a compat guest. */
>          void __user *guest_handle = domctl->u.getpageframeinfo3.array.p;
>  
> -        if ( unlikely(num > 1024) ||
> -             unlikely(num != domctl->u.getpageframeinfo3.num) )
> +        if ( unlikely(num > 1024) )
>          {
>              ret = -E2BIG;
>              break;


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1] x86/domctl: remove impossible condition in XEN_DOMCTL_getpageframeinfo3

[Wei Liu]

On Fri, Mar 09, 2018 at 05:01:16PM +0100, Olaf Hering wrote:
> The value of num is always the same as domctl->u.getpageframeinfo3.num,
> it was assigned just a few lines before.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

It is still useful.  The field in getpageframeinfo3 is uint64_aligned_t
while here num is just unsigned int. The check makes sure no truncation
happens.

> ---
>  xen/arch/x86/domctl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 8fbbf3aeb3..46d288a490 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -421,8 +421,7 @@ long arch_do_domctl(
>          /* Games to allow this code block to handle a compat guest. */
>          void __user *guest_handle = domctl->u.getpageframeinfo3.array.p;
>  
> -        if ( unlikely(num > 1024) ||
> -             unlikely(num != domctl->u.getpageframeinfo3.num) )
> +        if ( unlikely(num > 1024) )
>          {
>              ret = -E2BIG;
>              break;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1] x86/domctl: remove impossible condition in XEN_DOMCTL_getpageframeinfo3

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 435 bytes --]

On Fri, Mar 09, Andrew Cooper wrote:

> On 09/03/18 16:01, Olaf Hering wrote:
> > The value of num is always the same as domctl->u.getpageframeinfo3.num,
> > it was assigned just a few lines before.
> >
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> This isn't dead code.  It is a truncation check.

How can this happen, other than num being a 32bit type?
Perhaps the type of num should match the type of i?

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel