Patchwork [2/2] icu 3.6: do_install failed: Segmentation fault

login
register
mail settings
Submitter Robert Yang
Date June 27, 2012, 8:19 a.m.
Message ID <63976f811fe5cb0c5688fe6847975499ac2d9e51.1340773014.git.liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/30693/
State New
Headers show

Comments

Robert Yang - June 27, 2012, 8:19 a.m.
There is a "Segmentation fault" error when the tmpdir is longer than 470
(or less), this is because it used "char cmd[1024]" which is not enough
for the command line. Allocate a larger memory size should fix this problem.

[YOCTO #2664]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 .../icu/files/larger-cmd-size.patch                |   27 ++++++++++++++++++++
 meta/recipes-support/icu/icu-3.6.inc               |    1 +
 meta/recipes-support/icu/icu_3.6.bb                |    2 +-
 3 files changed, 29 insertions(+), 1 deletions(-)
 create mode 100644 meta/recipes-support/icu/files/larger-cmd-size.patch
Robert Yang - June 29, 2012, 1:54 a.m.
On 06/27/2012 04:19 PM, Robert Yang wrote:
> There is a "Segmentation fault" error when the tmpdir is longer than 470
> (or less), this is because it used "char cmd[1024]" which is not enough
> for the command line. Allocate a larger memory size should fix this problem.
>
> [YOCTO #2664]
>
> Signed-off-by: Robert Yang<liezhi.yang@windriver.com>
> ---
>   .../icu/files/larger-cmd-size.patch                |   27 ++++++++++++++++++++
>   meta/recipes-support/icu/icu-3.6.inc               |    1 +
>   meta/recipes-support/icu/icu_3.6.bb                |    2 +-
>   3 files changed, 29 insertions(+), 1 deletions(-)
>   create mode 100644 meta/recipes-support/icu/files/larger-cmd-size.patch
>
> diff --git a/meta/recipes-support/icu/files/larger-cmd-size.patch b/meta/recipes-support/icu/files/larger-cmd-size.patch
> new file mode 100644
> index 0000000..4b5f73d
> --- /dev/null
> +++ b/meta/recipes-support/icu/files/larger-cmd-size.patch
> @@ -0,0 +1,27 @@
> +Allocate a larger memory size for cmd
> +
> +The length of the command line can be longer than 1024 sometimes,
> +which will cause a "Segmentation fault" error.
> +
> +Signed-off-by: Robert Yang<liezhi.yang@windriver.com>
> +
> +Upstream-Status: Pending
> +---
> + tools/pkgdata/pkgdata.c |    2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/tools/pkgdata/pkgdata.c b/tools/pkgdata/pkgdata.c
> +--- a/tools/pkgdata/pkgdata.c
> ++++ b/tools/pkgdata/pkgdata.c
> +@@ -439,7 +439,7 @@ main(int argc, char* argv[]) {
> + /* POSIX - execute makefile */
> + static int executeMakefile(const UPKGOptions *o)
> + {
> +-    char cmd[1024];
> ++    char cmd[BUFSIZ];

I've changed the cmd[BUFSIZ] to cmd[2048], which should be enough for us, and
pushed it to the git repo again:

   git://git.pokylinux.org/poky-contrib robert/icu
   http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=robert/icu

// Robert

> +     /*char pwd[1024];*/
> +     const char *make;
> +     int rc;
> +--
> +1.7.10.2
> +
> diff --git a/meta/recipes-support/icu/icu-3.6.inc b/meta/recipes-support/icu/icu-3.6.inc
> index d3391fe..d969caa 100644
> --- a/meta/recipes-support/icu/icu-3.6.inc
> +++ b/meta/recipes-support/icu/icu-3.6.inc
> @@ -6,6 +6,7 @@ BASE_SRC_URI = "ftp://ftp.software.ibm.com/software/globalization/icu/3.6/icu4c-
>              file://gccfix.patch \
>              file://fix-parallel-build.patch \
>              file://use-g++-for-linking.patch \
> +           file://larger-cmd-size.patch \
>              "
>   SRC_URI = "${BASE_SRC_URI} \
>              file://noldlibpath.patch \
> diff --git a/meta/recipes-support/icu/icu_3.6.bb b/meta/recipes-support/icu/icu_3.6.bb
> index d7be522..f941acf 100644
> --- a/meta/recipes-support/icu/icu_3.6.bb
> +++ b/meta/recipes-support/icu/icu_3.6.bb
> @@ -1,6 +1,6 @@
>   require icu-3.6.inc
>
> -PR = "r8"
> +PR = "r9"
>
>   SRC_URI[md5sum] = "6243f7a19e03e05403ce84e597510d4c"
>   SRC_URI[sha256sum] = "5135e8d69d6206d320515df7aeee7027711ab1aef9d8dbf29571a97a9746b041"
Khem Raj - June 29, 2012, 4:54 a.m.
On Thu, Jun 28, 2012 at 6:54 PM, Robert Yang <liezhi.yang@windriver.com> wrote:
> I've changed the cmd[BUFSIZ] to cmd[2048], which should be enough for us,
> and
> pushed it to the git repo again:

ideally allocating a string on heap and then freeing
it after use would fix this issue once for all.
Robert Yang - June 29, 2012, 5:26 a.m.
On 06/29/2012 12:54 PM, Khem Raj wrote:
> On Thu, Jun 28, 2012 at 6:54 PM, Robert Yang<liezhi.yang@windriver.com>  wrote:
>> I've changed the cmd[BUFSIZ] to cmd[2048], which should be enough for us,
>> and
>> pushed it to the git repo again:
>
> ideally allocating a string on heap and then freeing
> it after use would fix this issue once for all.
>

Thanks Khem, but the problem is that even we use:

void *malloc(size_t size)

We should still use a constant size since the length of the command
is unknown (or not obviously known):

#else
     sprintf(cmd, "%s %s%s -f %s %s %s %s %s",
         make,
         o->install ? "INSTALLTO=" : "",
         o->install ? o->install    : "",
         o->makeFile,
         o->clean   ? "clean"      : "",
         o->rebuild ? "rebuild"    : "",
         o->install ? "install"    : "",
         o->makeArgs);

So I simply increase the size of cmd[] which is a simple fix.

// Robert

> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>
Khem Raj - June 29, 2012, 6:06 a.m.
On Thu, Jun 28, 2012 at 10:26 PM, Robert Yang <liezhi.yang@windriver.com> wrote:
> We should still use a constant size since the length of the command
> is unknown (or not obviously known):

thats the point. Cant you use srtlen(cmd) ?
Robert Yang - June 29, 2012, 6:30 a.m.
On 06/29/2012 02:06 PM, Khem Raj wrote:
> On Thu, Jun 28, 2012 at 10:26 PM, Robert Yang<liezhi.yang@windriver.com>  wrote:
>> We should still use a constant size since the length of the command
>> is unknown (or not obviously known):
>
> thats the point. Cant you use srtlen(cmd) ?
>

I'm afraid not, as the code shows below, we should strlen()
each component of the cmd:

     sprintf(cmd, "%s %s%s -f %s %s %s %s %s",
         make,
         o->install ? "INSTALLTO=" : "",
         o->install ? o->install    : "",
         o->makeFile,
         o->clean   ? "clean"      : "",
         o->rebuild ? "rebuild"    : "",
         o->install ? "install"    : "",
         o->makeArgs);

and this is only the part for linux os, it still has similar parts for
U_WINDOWS and OS400 (the component are different), if we strlen() each
one of them, that would make it complicated. the icu pkg does use many
malloc() in the other part of code, but it uses the char array here,
I think that maybe this is the reason.

// Robert

>
Khem Raj - June 29, 2012, 7:14 a.m.
On Thu, Jun 28, 2012 at 11:30 PM, Robert Yang <liezhi.yang@windriver.com> wrote:
>
>
> On 06/29/2012 02:06 PM, Khem Raj wrote:
>>
>> On Thu, Jun 28, 2012 at 10:26 PM, Robert Yang<liezhi.yang@windriver.com>
>>  wrote:
>>>
>>> We should still use a constant size since the length of the command
>>> is unknown (or not obviously known):
>>
>>
>> thats the point. Cant you use srtlen(cmd) ?
>>
>
> I'm afraid not, as the code shows below, we should strlen()
> each component of the cmd:
>
>
>    sprintf(cmd, "%s %s%s -f %s %s %s %s %s",
>        make,
>        o->install ? "INSTALLTO=" : "",
>        o->install ? o->install    : "",
>        o->makeFile,
>        o->clean   ? "clean"      : "",
>        o->rebuild ? "rebuild"    : "",
>        o->install ? "install"    : "",
>        o->makeArgs);
>
> and this is only the part for linux os, it still has similar parts for
> U_WINDOWS and OS400 (the component are different), if we strlen() each
> one of them, that would make it complicated. the icu pkg does use many
> malloc() in the other part of code, but it uses the char array here,
> I think that maybe this is the reason.
>

OK its fine what you have

> // Robert
>
>>
>

Patch

diff --git a/meta/recipes-support/icu/files/larger-cmd-size.patch b/meta/recipes-support/icu/files/larger-cmd-size.patch
new file mode 100644
index 0000000..4b5f73d
--- /dev/null
+++ b/meta/recipes-support/icu/files/larger-cmd-size.patch
@@ -0,0 +1,27 @@ 
+Allocate a larger memory size for cmd
+
+The length of the command line can be longer than 1024 sometimes,
+which will cause a "Segmentation fault" error.
+
+Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
+
+Upstream-Status: Pending
+---
+ tools/pkgdata/pkgdata.c |    2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/tools/pkgdata/pkgdata.c b/tools/pkgdata/pkgdata.c
+--- a/tools/pkgdata/pkgdata.c
++++ b/tools/pkgdata/pkgdata.c
+@@ -439,7 +439,7 @@ main(int argc, char* argv[]) {
+ /* POSIX - execute makefile */
+ static int executeMakefile(const UPKGOptions *o)
+ {
+-    char cmd[1024];
++    char cmd[BUFSIZ];
+     /*char pwd[1024];*/
+     const char *make;
+     int rc;
+-- 
+1.7.10.2
+
diff --git a/meta/recipes-support/icu/icu-3.6.inc b/meta/recipes-support/icu/icu-3.6.inc
index d3391fe..d969caa 100644
--- a/meta/recipes-support/icu/icu-3.6.inc
+++ b/meta/recipes-support/icu/icu-3.6.inc
@@ -6,6 +6,7 @@  BASE_SRC_URI = "ftp://ftp.software.ibm.com/software/globalization/icu/3.6/icu4c-
            file://gccfix.patch \
            file://fix-parallel-build.patch \
            file://use-g++-for-linking.patch \
+           file://larger-cmd-size.patch \
            "
 SRC_URI = "${BASE_SRC_URI} \
            file://noldlibpath.patch \
diff --git a/meta/recipes-support/icu/icu_3.6.bb b/meta/recipes-support/icu/icu_3.6.bb
index d7be522..f941acf 100644
--- a/meta/recipes-support/icu/icu_3.6.bb
+++ b/meta/recipes-support/icu/icu_3.6.bb
@@ -1,6 +1,6 @@ 
 require icu-3.6.inc
 
-PR = "r8"
+PR = "r9"
 
 SRC_URI[md5sum] = "6243f7a19e03e05403ce84e597510d4c"
 SRC_URI[sha256sum] = "5135e8d69d6206d320515df7aeee7027711ab1aef9d8dbf29571a97a9746b041"