[Buildroot] [PATCH] sstrip preserve permissions

[Buildroot] [PATCH] sstrip preserve permissions

[Danomi Manchego]

Unlike "strip", the "sstrip" does not preserve the file permissions
of its target.  So if you have a package that sets special permissions,
such as the setuid bit, sstrip will remove it.  This patch adds some
minimal lines to preserve the permissions of stripped files.

Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
---
 .../sstrip/sstrip-20154-preserve-permissions.patch |   43 ++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 package/sstrip/sstrip-20154-preserve-permissions.patch

diff --git a/package/sstrip/sstrip-20154-preserve-permissions.patch b/package/sstrip/sstrip-20154-preserve-permissions.patch
new file mode 100644
index 0000000..0bd9162
--- /dev/null
+++ b/package/sstrip/sstrip-20154-preserve-permissions.patch
@@ -0,0 +1,43 @@
+diff -urN sstrip.ORIG/src/sstrip.c sstrip/src/sstrip.c
+--- sstrip.ORIG/src/sstrip.c	2010-12-11 23:59:51.464374000 -0500
++++ sstrip/src/sstrip.c	2012-02-21 11:58:56.320295827 -0500
+@@ -60,6 +60,10 @@
+ #include	<fcntl.h>
+ #include	<elf.h>
+ 
++#include <sys/types.h>
++#include <sys/stat.h>
++#include <unistd.h>
++
+ #ifndef TRUE
+ #define	TRUE		1
+ #define	FALSE		0
+@@ -427,6 +431,7 @@
+ 
+ 	for (arg = argv + 1 ; *arg != NULL ; ++arg) {
+ 		filename = *arg;
++		struct stat sb;
+ 
+ 		fd = open(*arg, O_RDWR);
+ 		if (fd < 0) {
+@@ -435,6 +440,9 @@
+ 			continue;
+ 		}
+ 
++		/* Get original file's permissions */
++		if (fstat(fd, &sb) == -1) { perror("fstat"); sb.st_mode = 0; }
++
+ 		switch (readelfheaderident(fd, &e.ehdr32)) {
+ 			case ELFCLASS32:
+ 				if (!(readelfheader32(fd, &e.ehdr32)					&&
+@@ -458,6 +466,10 @@
+ 				++failures;
+ 				break;
+ 		}
++
++		/* Set original file's permissions, including setuid */
++		if (sb.st_mode != 0) { fchmod(fd, sb.st_mode & 07777); }
++
+ 		close(fd);
+ 	}
+ 
-- 
1.7.9.5

[Buildroot] [PATCH] sstrip preserve permissions

[Peter Korsgaard]

>>>>> "Danomi" == Danomi Manchego <danomimanchego123@gmail.com> writes:

 Danomi> Unlike "strip", the "sstrip" does not preserve the file permissions
 Danomi> of its target.  So if you have a package that sets special permissions,
 Danomi> such as the setuid bit, sstrip will remove it.  This patch adds some
 Danomi> minimal lines to preserve the permissions of stripped files.

What is the use case for this? From a quick look it seems sstrip just
truncates the source file, so permissions shouldn't change.

setuid (to root) doesn't make much sense as we're building as non-root
and fixing up permissions at the end using fakeroot (E.G. see
BUSYBOX_PERMISSIONS).


 Danomi> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
 Danomi> ---
 Danomi>  .../sstrip/sstrip-20154-preserve-permissions.patch |   43 ++++++++++++++++++++
 Danomi>  1 file changed, 43 insertions(+)
 Danomi>  create mode 100644 package/sstrip/sstrip-20154-preserve-permissions.patch

 Danomi> diff --git a/package/sstrip/sstrip-20154-preserve-permissions.patch b/package/sstrip/sstrip-20154-preserve-permissions.patch
 Danomi> new file mode 100644
 Danomi> index 0000000..0bd9162
 Danomi> --- /dev/null
 Danomi> +++ b/package/sstrip/sstrip-20154-preserve-permissions.patch
 Danomi> @@ -0,0 +1,43 @@
 Danomi> +diff -urN sstrip.ORIG/src/sstrip.c sstrip/src/sstrip.c
 Danomi> +--- sstrip.ORIG/src/sstrip.c	2010-12-11 23:59:51.464374000 -0500
 Danomi> ++++ sstrip/src/sstrip.c	2012-02-21 11:58:56.320295827 -0500
 Danomi> +@@ -60,6 +60,10 @@
 Danomi> + #include	<fcntl.h>
 Danomi> + #include	<elf.h>
 Danomi> + 
 Danomi> ++#include <sys/types.h>
 Danomi> ++#include <sys/stat.h>
 Danomi> ++#include <unistd.h>
 Danomi> ++
 Danomi> + #ifndef TRUE
 Danomi> + #define	TRUE		1
 Danomi> + #define	FALSE		0
 Danomi> +@@ -427,6 +431,7 @@
 Danomi> + 
 Danomi> + 	for (arg = argv + 1 ; *arg != NULL ; ++arg) {
 Danomi> + 		filename = *arg;
 Danomi> ++		struct stat sb;
 Danomi> + 
 Danomi> + 		fd = open(*arg, O_RDWR);
 Danomi> + 		if (fd < 0) {
 Danomi> +@@ -435,6 +440,9 @@
 Danomi> + 			continue;
 Danomi> + 		}
 Danomi> + 
 Danomi> ++		/* Get original file's permissions */
 Danomi> ++		if (fstat(fd, &sb) == -1) { perror("fstat"); sb.st_mode = 0; }
 Danomi> ++
 Danomi> + 		switch (readelfheaderident(fd, &e.ehdr32)) {
 Danomi> + 			case ELFCLASS32:
 Danomi> + 				if (!(readelfheader32(fd, &e.ehdr32)					&&
 Danomi> +@@ -458,6 +466,10 @@
 Danomi> + 				++failures;
 Danomi> + 				break;
 Danomi> + 		}
 Danomi> ++
 Danomi> ++		/* Set original file's permissions, including setuid */
 Danomi> ++		if (sb.st_mode != 0) { fchmod(fd, sb.st_mode & 07777); }
 Danomi> ++
 Danomi> + 		close(fd);
 Danomi> + 	}
 Danomi> + 
 Danomi> -- 
 Danomi> 1.7.9.5

 Danomi> _______________________________________________
 Danomi> buildroot mailing list
 Danomi> buildroot at busybox.net
 Danomi> http://lists.busybox.net/mailman/listinfo/buildroot


-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] sstrip preserve permissions

[Danomi Manchego]

Hi Peter,

>  Danomi> Unlike "strip", the "sstrip" does not preserve the file permissions
>  Danomi> of its target.  So if you have a package that sets special permissions,
>  Danomi> such as the setuid bit, sstrip will remove it.  This patch adds some
>  Danomi> minimal lines to preserve the permissions of stripped files.
>
> What is the use case for this? From a quick look it seems sstrip just

We have a proprietary CGI app that gets called by lighttpd, so the
caller isn't root.  I don't quite remember the exact operation that
caused us to do a chmod +s.  That's when we noticed that the +s perms
were disappearing when sstrip was used, but conserved when strip was
used.


> truncates the source file, so permissions shouldn't change.

Nope.  Just try it on a file that gets cut down.  For example:

	$ ll output/target/usr/bin/i2cdetect
	-rwxr-xr-x 1 dmocelo dmocelo 34726 Nov  7 21:34
output/target/usr/bin/i2cdetect*

	$ chmod a+s output/target/usr/bin/i2cdetect

	$ ll output/target/usr/bin/i2cdetect
	-rwsr-sr-x 1 dmocelo dmocelo 34726 Nov  7 21:34
output/target/usr/bin/i2cdetect*

	$ # now I'll run a make with sstrip enabled
	$ make
	...

	$ ll output/target/usr/bin/i2cdetect
	-rwxr-xr-x 1 dmocelo dmocelo 11630 Nov  7 21:36
output/target/usr/bin/i2cdetect*


> setuid (to root) doesn't make much sense as we're building as non-root
> and fixing up permissions at the end using fakeroot (E.G. see
> BUSYBOX_PERMISSIONS).

We make our packages to be friendly to multiple build systems, so the
package makefile's install operation is taking care of the special
permission, rather than using a buildroot-specific mechanism.

But regardless, I was thinking that the difference compared to normal
strip operation was reason enough to want to preserve permissions.
No?

Danomi -