diff mbox series

fakeroot: Fix segfault on ppc64le

Message ID 20220315130737.428449-1-joel@jms.id.au
State Accepted
Headers show
Series fakeroot: Fix segfault on ppc64le | expand

Commit Message

Joel Stanley March 15, 2022, 1:07 p.m. UTC
When generating a filesystem image on a power10 build machine running
Ubuntu, we see a segfault when fakeroot is running chmod.

This has been reported and fixed upstream in Debian in version 1.26-1.2:

 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995393#53

Add the samae patch to resolve the segfault.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
This isn't a commit in the fakeroot tree, rather a patch from the Debian
packaging.

I would appreciate if this was added to the 2022.02 stable tree.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../fakeroot/fix-prototype-generation.patch   | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 package/fakeroot/fix-prototype-generation.patch

Comments

Joel Stanley March 23, 2022, 2:32 a.m. UTC | #1
On Tue, 15 Mar 2022 at 13:07, Joel Stanley <joel@jms.id.au> wrote:
>
> When generating a filesystem image on a power10 build machine running
> Ubuntu, we see a segfault when fakeroot is running chmod.
>
> This has been reported and fixed upstream in Debian in version 1.26-1.2:
>
>  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995393#53
>
> Add the samae patch to resolve the segfault.

ping

>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> This isn't a commit in the fakeroot tree, rather a patch from the Debian
> packaging.
>
> I would appreciate if this was added to the 2022.02 stable tree.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  .../fakeroot/fix-prototype-generation.patch   | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 package/fakeroot/fix-prototype-generation.patch
>
> diff --git a/package/fakeroot/fix-prototype-generation.patch b/package/fakeroot/fix-prototype-generation.patch
> new file mode 100644
> index 000000000000..38d32ff3b22c
> --- /dev/null
> +++ b/package/fakeroot/fix-prototype-generation.patch
> @@ -0,0 +1,59 @@
> +Subject: Fix prototype generation for openat
> +Author: Christoph Biedl <debian.axhn@manchmal.in-ulm.de>
> +Date: 2021-12-30
> +Bug-Debian: https://bugs.debian.org/995393
> +Forwarded: Yes (implicitely)
> +
> +    As jrtc27 pointed out in IRC, ppc64el is more strict than other
> +    architectures when it comes to va_arg handling:
> +
> +        it's that ppc64le uses the elfv2 abi, and for variadic calls you
> +        must reserve space for a parameter save area
> +
> +    So enhance wrapawk to create a proper prototype and argument
> +    handling although it's specific to the openat call. Also add the
> +    missing documentation for the sixth column to wrapfunc.inp.
> +
> +--- a/wrapawk
> ++++ b/wrapawk
> +@@ -37,7 +37,25 @@
> +   argtype=$3;
> +   argname=$4;
> +   MACRO=$5;
> +-  if(MACRO){
> ++  openat_extra=$6;
> ++  if(openat_extra){
> ++    print "  {(void(*))&next_" name ", \"" name "\"},"  > structfile;
> ++    print "extern " ret " (*next_" name ")" openat_extra ";" > headerfile;
> ++    print ret " (*next_" name ")" openat_extra "=tmp_" name ";"> deffile;
> ++
> ++    print ret " tmp_" name,  openat_extra "{"           > tmpffile;
> ++    print "  mode_t mode = 0;"                          > tmpffile;
> ++    print "  if (flags & O_CREAT) {"                    > tmpffile;
> ++    print "    va_list args;"                           > tmpffile;
> ++    print "    va_start(args, flags);"                  > tmpffile;
> ++    print "    mode = va_arg(args, int);"               > tmpffile;
> ++    print "    va_end(args);"                           > tmpffile;
> ++    print "  }"                                         > tmpffile;
> ++    print "  load_library_symbols();"                   > tmpffile;
> ++    print "  return  next_" name,  argname ";"          > tmpffile;
> ++    print "}"                                           > tmpffile;
> ++    print ""                                            > tmpffile;
> ++  } else if(MACRO){
> +     print "  {(void(*))&NEXT_" MACRO "_NOARG, " name "_QUOTE},"  > structfile;
> +     print "extern " ret " (*NEXT_" MACRO "_NOARG)" argtype ";" > headerfile;
> +     print ret " (*NEXT_" MACRO "_NOARG)" argtype "=TMP_" MACRO ";"> deffile;
> +--- a/wrapfunc.inp
> ++++ b/wrapfunc.inp
> +@@ -9,8 +9,10 @@
> + /**/                                                                    */
> + /* each line of this file lists 4 fields, seperated by a ";".           */
> + /* The first field is the name of the wrapped function, then it's return  */
> +-/* value. After that come the function arguments with types, and the last */
> ++/* value. After that come the function arguments with types, and the fifth */
> + /* field contains the function arguments without types.                   */
> ++/* A sixth field is a special needed when wrapping the openat syscall.    */
> ++/* Otherwise it's like the third (function arguments with types).         */
> + /**/
> +
> + /* __*xstat are used on glibc systems instead of just *xstat. */
> --
> 2.35.1
>
Arnout Vandecappelle March 24, 2022, 8:02 p.m. UTC | #2
On 15/03/2022 14:07, Joel Stanley wrote:
> When generating a filesystem image on a power10 build machine running
> Ubuntu, we see a segfault when fakeroot is running chmod.
> 
> This has been reported and fixed upstream in Debian in version 1.26-1.2:
> 
>   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995393#53
> 
> Add the samae patch to resolve the segfault.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> This isn't a commit in the fakeroot tree, rather a patch from the Debian
> packaging.

  Normally we'd just bump fakeroot, but I guess the patch isn't there yet in 1.28.

> 
> I would appreciate if this was added to the 2022.02 stable tree.

  Plus, that would make backporting iffy.

  Therefore, applied to master. However.

> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   .../fakeroot/fix-prototype-generation.patch   | 59 +++++++++++++++++++
>   1 file changed, 59 insertions(+)
>   create mode 100644 package/fakeroot/fix-prototype-generation.patch
> 
> diff --git a/package/fakeroot/fix-prototype-generation.patch b/package/fakeroot/fix-prototype-generation.patch
> new file mode 100644
> index 000000000000..38d32ff3b22c
> --- /dev/null
> +++ b/package/fakeroot/fix-prototype-generation.patch

  Patches should be named 0001-XXXX even if they are downloaded from upstream. 
Actually, if it can literally be downloaded, you could just as well have used 
FAKEROOT_PATCH = https://...

> @@ -0,0 +1,59 @@
> +Subject: Fix prototype generation for openat
> +Author: Christoph Biedl <debian.axhn@manchmal.in-ulm.de>
> +Date: 2021-12-30
> +Bug-Debian: https://bugs.debian.org/995393
> +Forwarded: Yes (implicitely)
> +
> +    As jrtc27 pointed out in IRC, ppc64el is more strict than other
> +    architectures when it comes to va_arg handling:
> +
> +        it's that ppc64le uses the elfv2 abi, and for variadic calls you
> +        must reserve space for a parameter save area
> +
> +    So enhance wrapawk to create a proper prototype and argument
> +    handling although it's specific to the openat call. Also add the
> +    missing documentation for the sixth column to wrapfunc.inp.
> +

  Missing Signed-off-by. Again, this needs to be added even if the patch is 
literally downloaded.

  Both problems are reported by check-package.

  Regards,
  Arnout

> +--- a/wrapawk
> ++++ b/wrapawk
> +@@ -37,7 +37,25 @@
> +   argtype=$3;
> +   argname=$4;
> +   MACRO=$5;
> +-  if(MACRO){
> ++  openat_extra=$6;
> ++  if(openat_extra){
> ++    print "  {(void(*))&next_" name ", \"" name "\"},"  > structfile;
> ++    print "extern " ret " (*next_" name ")" openat_extra ";" > headerfile;
> ++    print ret " (*next_" name ")" openat_extra "=tmp_" name ";"> deffile;
> ++
> ++    print ret " tmp_" name,  openat_extra "{"           > tmpffile;
> ++    print "  mode_t mode = 0;"                          > tmpffile;
> ++    print "  if (flags & O_CREAT) {"                    > tmpffile;
> ++    print "    va_list args;"                           > tmpffile;
> ++    print "    va_start(args, flags);"                  > tmpffile;
> ++    print "    mode = va_arg(args, int);"               > tmpffile;
> ++    print "    va_end(args);"                           > tmpffile;
> ++    print "  }"                                         > tmpffile;
> ++    print "  load_library_symbols();"                   > tmpffile;
> ++    print "  return  next_" name,  argname ";"          > tmpffile;
> ++    print "}"                                           > tmpffile;
> ++    print ""                                            > tmpffile;
> ++  } else if(MACRO){
> +     print "  {(void(*))&NEXT_" MACRO "_NOARG, " name "_QUOTE},"  > structfile;
> +     print "extern " ret " (*NEXT_" MACRO "_NOARG)" argtype ";" > headerfile;
> +     print ret " (*NEXT_" MACRO "_NOARG)" argtype "=TMP_" MACRO ";"> deffile;
> +--- a/wrapfunc.inp
> ++++ b/wrapfunc.inp
> +@@ -9,8 +9,10 @@
> + /**/									  */
> + /* each line of this file lists 4 fields, seperated by a ";".		  */
> + /* The first field is the name of the wrapped function, then it's return  */
> +-/* value. After that come the function arguments with types, and the last */
> ++/* value. After that come the function arguments with types, and the fifth */
> + /* field contains the function arguments without types.                   */
> ++/* A sixth field is a special needed when wrapping the openat syscall.    */
> ++/* Otherwise it's like the third (function arguments with types).         */
> + /**/
> +
> + /* __*xstat are used on glibc systems instead of just *xstat. */
Peter Korsgaard March 29, 2022, 7:47 p.m. UTC | #3
>>>>> "Joel" == Joel Stanley <joel@jms.id.au> writes:

 > When generating a filesystem image on a power10 build machine running
 > Ubuntu, we see a segfault when fakeroot is running chmod.

 > This has been reported and fixed upstream in Debian in version 1.26-1.2:

 >  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995393#53

 > Add the samae patch to resolve the segfault.

 > Signed-off-by: Joel Stanley <joel@jms.id.au>
 > ---
 > This isn't a commit in the fakeroot tree, rather a patch from the Debian
 > packaging.

 > I would appreciate if this was added to the 2022.02 stable tree.

 > Signed-off-by: Joel Stanley <joel@jms.id.au>

Committed to 2022.02.x, thanks.
diff mbox series

Patch

diff --git a/package/fakeroot/fix-prototype-generation.patch b/package/fakeroot/fix-prototype-generation.patch
new file mode 100644
index 000000000000..38d32ff3b22c
--- /dev/null
+++ b/package/fakeroot/fix-prototype-generation.patch
@@ -0,0 +1,59 @@ 
+Subject: Fix prototype generation for openat
+Author: Christoph Biedl <debian.axhn@manchmal.in-ulm.de>
+Date: 2021-12-30
+Bug-Debian: https://bugs.debian.org/995393
+Forwarded: Yes (implicitely)
+
+    As jrtc27 pointed out in IRC, ppc64el is more strict than other
+    architectures when it comes to va_arg handling:
+
+        it's that ppc64le uses the elfv2 abi, and for variadic calls you
+        must reserve space for a parameter save area
+
+    So enhance wrapawk to create a proper prototype and argument
+    handling although it's specific to the openat call. Also add the
+    missing documentation for the sixth column to wrapfunc.inp.
+
+--- a/wrapawk
++++ b/wrapawk
+@@ -37,7 +37,25 @@
+   argtype=$3;
+   argname=$4;
+   MACRO=$5;
+-  if(MACRO){
++  openat_extra=$6;
++  if(openat_extra){
++    print "  {(void(*))&next_" name ", \"" name "\"},"  > structfile;
++    print "extern " ret " (*next_" name ")" openat_extra ";" > headerfile;
++    print ret " (*next_" name ")" openat_extra "=tmp_" name ";"> deffile;
++
++    print ret " tmp_" name,  openat_extra "{"           > tmpffile;
++    print "  mode_t mode = 0;"                          > tmpffile;
++    print "  if (flags & O_CREAT) {"                    > tmpffile;
++    print "    va_list args;"                           > tmpffile;
++    print "    va_start(args, flags);"                  > tmpffile;
++    print "    mode = va_arg(args, int);"               > tmpffile;
++    print "    va_end(args);"                           > tmpffile;
++    print "  }"                                         > tmpffile;
++    print "  load_library_symbols();"                   > tmpffile;
++    print "  return  next_" name,  argname ";"          > tmpffile;
++    print "}"                                           > tmpffile;
++    print ""                                            > tmpffile;
++  } else if(MACRO){
+     print "  {(void(*))&NEXT_" MACRO "_NOARG, " name "_QUOTE},"  > structfile;
+     print "extern " ret " (*NEXT_" MACRO "_NOARG)" argtype ";" > headerfile;
+     print ret " (*NEXT_" MACRO "_NOARG)" argtype "=TMP_" MACRO ";"> deffile;
+--- a/wrapfunc.inp
++++ b/wrapfunc.inp
+@@ -9,8 +9,10 @@
+ /**/									  */
+ /* each line of this file lists 4 fields, seperated by a ";".		  */
+ /* The first field is the name of the wrapped function, then it's return  */
+-/* value. After that come the function arguments with types, and the last */
++/* value. After that come the function arguments with types, and the fifth */
+ /* field contains the function arguments without types.                   */
++/* A sixth field is a special needed when wrapping the openat syscall.    */
++/* Otherwise it's like the third (function arguments with types).         */
+ /**/
+ 
+ /* __*xstat are used on glibc systems instead of just *xstat. */