Re: [Xen-devel] [PATCH 1/2] build-id: fix minor quirks

2016-08-12 Thread Konrad Rzeszutek Wilk
On Fri, Aug 12, 2016 at 08:45:02AM -0600, Jan Beulich wrote:
> The initial size check in xen_build_id_check() came too late (after the
> first access to the structure), but was mostly redundant with checks
> done in all callers; convert it to a properly placed ASSERT(). The
> "mostly" part being addressed too: xen_build_init() was off by one.
> 
> And then there was a stray semicolon.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Konrad Rzeszutek Wilk 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


[Xen-devel] [PATCH 1/2] build-id: fix minor quirks

2016-08-12 Thread Jan Beulich
The initial size check in xen_build_id_check() came too late (after the
first access to the structure), but was mostly redundant with checks
done in all callers; convert it to a properly placed ASSERT(). The
"mostly" part being addressed too: xen_build_init() was off by one.

And then there was a stray semicolon.

Signed-off-by: Jan Beulich 

--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -1,6 +1,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -90,12 +91,11 @@ int xen_build_id_check(const Elf_Note *n
const void **p, unsigned int *len)
 {
 /* Check if we really have a build-id. */
+ASSERT(n_sz > sizeof(*n));
+
 if ( NT_GNU_BUILD_ID != n->type )
 return -ENODATA;
 
-if ( n_sz <= sizeof(*n) )
-return -EINVAL;
-
 if ( n->namesz + n->descsz < n->namesz )
 return -EINVAL;
 
@@ -127,8 +127,8 @@ static int __init xen_build_init(void)
 return -ENODATA;
 
 /* Check for full Note header. */
-if ( &n[1] > __note_gnu_build_id_end )
-return -ENODATA;;
+if ( &n[1] >= __note_gnu_build_id_end )
+return -ENODATA;
 
 sz = (void *)__note_gnu_build_id_end - (void *)n;
 



build-id: fix minor quirks

The initial size check in xen_build_id_check() came too late (after the
first access to the structure), but was mostly redundant with checks
done in all callers; convert it to a properly placed ASSERT(). The
"mostly" part being addressed too: xen_build_init() was off by one.

And then there was a stray semicolon.

Signed-off-by: Jan Beulich 

--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -1,6 +1,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -90,12 +91,11 @@ int xen_build_id_check(const Elf_Note *n
const void **p, unsigned int *len)
 {
 /* Check if we really have a build-id. */
+ASSERT(n_sz > sizeof(*n));
+
 if ( NT_GNU_BUILD_ID != n->type )
 return -ENODATA;
 
-if ( n_sz <= sizeof(*n) )
-return -EINVAL;
-
 if ( n->namesz + n->descsz < n->namesz )
 return -EINVAL;
 
@@ -127,8 +127,8 @@ static int __init xen_build_init(void)
 return -ENODATA;
 
 /* Check for full Note header. */
-if ( &n[1] > __note_gnu_build_id_end )
-return -ENODATA;;
+if ( &n[1] >= __note_gnu_build_id_end )
+return -ENODATA;
 
 sz = (void *)__note_gnu_build_id_end - (void *)n;
 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel