thr3ads.net - Xen devel - [PATCH] Fix stubdom build failure for RELEASE-4.3.1 [Oct 2013]

If this information is useful, please help other people find it:
Share via:

Alex Sharp

2013-Oct-31 14:44 UTC

[PATCH] Fix stubdom build failure for RELEASE-4.3.1

Hey,

I noticed that RELEASE-4.3.1 (commit 4e8e0bdef96859e6f428af74be6f4a3bc7fd11f3)
was failing to make stubdom, complaining of various forms of format string
mismatch. The following patch at least gets things to compile, but I''m
not sure if there''s a deeper underlying issue. I''m using gcc
version 4.8.1 (Ubuntu/Linaro 4.8.1-10ubuntu8) on x86_64. My .config contains the
line "PYTHON_PREFIX_ARG=--install-layout=deb" and I''m
./configuring with --prefix
Hope this helps.

diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
index 54a5e67..b6aaa6b 100644
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -105,7 +105,7 @@ again:
         free(err);
     }

-    err = xenbus_printf(xbt, nodename, "page-ref","%u",
virt_to_mfn(s));
+    err = xenbus_printf(xbt, nodename, "page-ref","%lu",
virt_to_mfn(s));
     if (err) {
         message = "writing page-ref";
         goto abort_transaction;
@@ -463,7 +463,7 @@ again:
         free(err);
     }

-    err = xenbus_printf(xbt, nodename, "page-ref","%u",
virt_to_mfn(s));
+    err = xenbus_printf(xbt, nodename, "page-ref","%lu",
virt_to_mfn(s));
     if (err) {
         message = "writing page-ref";
         goto abort_transaction;
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
index bbe21e0..9d0cb17 100644
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -413,7 +413,7 @@ int pcifront_physical_to_virtual (struct pcifront_dev *dev,
                 continue;
             }

-            if (sscanf(s, "%x:%x:%x.%x", dom, bus, slot, fun) != 4) {
+            if (sscanf(s, "%x:%x:%x.%lx", dom, bus, slot, fun) != 4)
{
                 printk("\"%s\" does not look like a PCI device
address\n", s);
                 free(s);
                 continue;
diff --git a/extras/mini-os/xenbus/xenbus.c b/extras/mini-os/xenbus/xenbus.c
index ee1691b..fa2e872 100644
--- a/extras/mini-os/xenbus/xenbus.c
+++ b/extras/mini-os/xenbus/xenbus.c
@@ -672,7 +672,7 @@ char *xenbus_transaction_start(xenbus_transaction_t *xbt)
     err = errmsg(rep);
     if (err)
        return err;
-    sscanf((char *)(rep + 1), "%u", xbt);
+    sscanf((char *)(rep + 1), "%lu", xbt);
     free(rep);
     return NULL;
 }
@@ -769,7 +769,7 @@ domid_t xenbus_get_self_id(void)
     domid_t ret;

     BUG_ON(xenbus_read(XBT_NIL, "domid", &dom_id));
-    sscanf(dom_id, "%d", &ret);
+    sscanf(dom_id, "%u", (unsigned int*)&ret);

     return ret;
 }



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Andrew Cooper

2013-Oct-31 14:57 UTC

head link

Re: [PATCH] Fix stubdom build failure for RELEASE-4.3.1

On 31/10/13 14:44, Alex Sharp wrote:> Hey,
>
> I noticed that RELEASE-4.3.1 (commit
> 4e8e0bdef96859e6f428af74be6f4a3bc7fd11f3) was failing to make stubdom,
> complaining of various forms of format string mismatch. The following
> patch at least gets things to compile, but I''m not sure if
there''s a
> deeper underlying issue. I''m using gcc version 4.8.1
(Ubuntu/Linaro
> 4.8.1-10ubuntu8) on x86_64. My .config contains the line
> "PYTHON_PREFIX_ARG=--install-layout=deb" and I''m
./configuring with
> --prefix>
> Hope this helps.
>
> diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
> index 54a5e67..b6aaa6b 100644
> --- a/extras/mini-os/fbfront.c
> +++ b/extras/mini-os/fbfront.c
> @@ -105,7 +105,7 @@ again:
>          free(err);
>      }
>  
> -    err = xenbus_printf(xbt, nodename,
"page-ref","%u", virt_to_mfn(s));
> +    err = xenbus_printf(xbt, nodename,
"page-ref","%lu", virt_to_mfn(s));
>      if (err) {
>          message = "writing page-ref";
>          goto abort_transaction;
> @@ -463,7 +463,7 @@ again:
>          free(err);
>      }
>  
> -    err = xenbus_printf(xbt, nodename,
"page-ref","%u", virt_to_mfn(s));
> +    err = xenbus_printf(xbt, nodename,
"page-ref","%lu", virt_to_mfn(s));
The two above here are certainly valid.  virt_to_mfn() goes through many
macros and ends up reading out of "unsigned long
*phys_to_machine_mapping".
>      if (err) {
>          message = "writing page-ref";
>          goto abort_transaction;
> diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
> index bbe21e0..9d0cb17 100644
> --- a/extras/mini-os/pcifront.c
> +++ b/extras/mini-os/pcifront.c
> @@ -413,7 +413,7 @@ int pcifront_physical_to_virtual (struct
> pcifront_dev *dev,
>                  continue;
>              }
>  
> -            if (sscanf(s, "%x:%x:%x.%x", dom, bus, slot, fun) !=
4) {
> +            if (sscanf(s, "%x:%x:%x.%lx", dom, bus, slot, fun)
!= 4) {
This is a little awkward.  It has been changed in unstable and is now
correct.  Perhaps a subset of the change needs backporting
>                  printk("\"%s\" does not look like a PCI
device
> address\n", s);
>                  free(s);
>                  continue;
> diff --git a/extras/mini-os/xenbus/xenbus.c
> b/extras/mini-os/xenbus/xenbus.c
> index ee1691b..fa2e872 100644
> --- a/extras/mini-os/xenbus/xenbus.c
> +++ b/extras/mini-os/xenbus/xenbus.c
> @@ -672,7 +672,7 @@ char
> *xenbus_transaction_start(xenbus_transaction_t *xbt)
>      err = errmsg(rep);
>      if (err)
>         return err;
> -    sscanf((char *)(rep + 1), "%u", xbt);
> +    sscanf((char *)(rep + 1), "%lu", xbt);
This is good
>      free(rep);
>      return NULL;
>  }
> @@ -769,7 +769,7 @@ domid_t xenbus_get_self_id(void)
>      domid_t ret;
>  
>      BUG_ON(xenbus_read(XBT_NIL, "domid", &dom_id));
> -    sscanf(dom_id, "%d", &ret);
> +    sscanf(dom_id, "%u", (unsigned int*)&ret);
This needs to be

sscanf(dom_id, "%"SCNd16, &ret);

That pointer casting there will result in stack corruption.

Could you please follow the instructions
http://wiki.xen.org/wiki/Submitting_Xen_Patches (specifically the
Signed-off-by line) and submit a patch against xen-unstable, which can
then be backported appropriately.

~Andrew
>  
>      return ret;
>  }
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Samuel Thibault

2013-Oct-31 18:00 UTC

head link

Re: [PATCH] Fix stubdom build failure for RELEASE-4.3.1

Andrew Cooper, le Thu 31 Oct 2013 14:57:24 +0000, a écrit
:>     diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
>     index bbe21e0..9d0cb17 100644
>     --- a/extras/mini-os/pcifront.c
>     +++ b/extras/mini-os/pcifront.c
>     @@ -413,7 +413,7 @@ int pcifront_physical_to_virtual (struct
pcifront_dev
>     *dev,
>                      continue;
>                  }
>      
>     -            if (sscanf(s, "%x:%x:%x.%x", dom, bus, slot,
fun) != 4) {
>     +            if (sscanf(s, "%x:%x:%x.%lx", dom, bus, slot,
fun) != 4) {
> 
> 
> This is a little awkward.  It has been changed in unstable and is now
correct.
Mmm, I don''t see it fixed in unstable.  I wonder why fun is an
unsigned long at all, it is even passed unsigned ints in callers.
pcifront_physical_to_virtual and pcifront_conf_read should just use
unsigned int.
>     @@ -769,7 +769,7 @@ domid_t xenbus_get_self_id(void)
>          domid_t ret;
>      
>          BUG_ON(xenbus_read(XBT_NIL, "domid", &dom_id));
>     -    sscanf(dom_id, "%d", &ret);
>     +    sscanf(dom_id, "%u", (unsigned int*)&ret);
> 
> 
> This needs to be
> 
> sscanf(dom_id, "%"SCNd16, &ret);
Mmm, perhaps rather make ret itself an unsigned int? I''d rather not
hardcode here what a domid_t is.

I agree on the rest.

Samuel

Xen devel - Oct 2013 - [PATCH] Fix stubdom build failure for RELEASE-4.3.1

[PATCH] Fix stubdom build failure for RELEASE-4.3.1

Re: [PATCH] Fix stubdom build failure for RELEASE-4.3.1

Re: [PATCH] Fix stubdom build failure for RELEASE-4.3.1