linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory
@ 2020-10-26 19:32 Sven Joachim
  2020-10-26 19:32 ` [PATCH 2/2] builddeb: Consolidate consecutive chmod calls into one Sven Joachim
  2020-10-28  6:00 ` [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Masahiro Yamada
  0 siblings, 2 replies; 6+ messages in thread
From: Sven Joachim @ 2020-10-26 19:32 UTC (permalink / raw)
  To: linux-kbuild, Masahiro Yamada, Michal Marek
  Cc: linux-kernel, Sven Joachim, Guillem Jover

Building 5.10-rc1 in a setgid directory failed with the following
error:

dpkg-deb: error: control directory has bad permissions 2755 (must be
>=0755 and <=0775)

When building with fakeroot, the earlier chown call would have removed
the setgid bits, but in a rootless build they remain.

Fixes: 3e8541803624 ("builddeb: Enable rootless builds")
Cc: Guillem Jover <guillem@hadrons.org>
Signed-off-by: Sven Joachim <svenjoac@gmx.de>
---
 scripts/package/builddeb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 1b11f8993629..91a502bb97e8 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -45,6 +45,8 @@ create_package() {
 	chmod -R go-w "$pdir"
 	# in case we are in a restrictive umask environment like 0077
 	chmod -R a+rX "$pdir"
+	# in case we build in a setuid/setgid directory
+	chmod -R ug-s "$pdir"

 	# Create the package
 	dpkg-gencontrol -p$pname -P"$pdir"
--
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] builddeb: Consolidate consecutive chmod calls into one
  2020-10-26 19:32 [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Sven Joachim
@ 2020-10-26 19:32 ` Sven Joachim
  2020-10-28  6:52   ` Masahiro Yamada
  2020-10-28  6:00 ` [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Masahiro Yamada
  1 sibling, 1 reply; 6+ messages in thread
From: Sven Joachim @ 2020-10-26 19:32 UTC (permalink / raw)
  To: linux-kbuild, Masahiro Yamada, Michal Marek; +Cc: linux-kernel, Sven Joachim

No need to call chmod three times when it can do everything at once.

Signed-off-by: Sven Joachim <svenjoac@gmx.de>
---
 scripts/package/builddeb | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 91a502bb97e8..81ec6414726c 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -42,11 +42,7 @@ create_package() {
 	else
 		chown -R root:root "$pdir"
 	fi
-	chmod -R go-w "$pdir"
-	# in case we are in a restrictive umask environment like 0077
-	chmod -R a+rX "$pdir"
-	# in case we build in a setuid/setgid directory
-	chmod -R ug-s "$pdir"
+	chmod -R go-w,a+rX,ug-s "$pdir"

 	# Create the package
 	dpkg-gencontrol -p$pname -P"$pdir"
--
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory
  2020-10-26 19:32 [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Sven Joachim
  2020-10-26 19:32 ` [PATCH 2/2] builddeb: Consolidate consecutive chmod calls into one Sven Joachim
@ 2020-10-28  6:00 ` Masahiro Yamada
  2020-10-28 18:30   ` Sven Joachim
  1 sibling, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2020-10-28  6:00 UTC (permalink / raw)
  To: Sven Joachim
  Cc: Linux Kbuild mailing list, Michal Marek,
	Linux Kernel Mailing List, Guillem Jover

On Tue, Oct 27, 2020 at 4:32 AM Sven Joachim <svenjoac@gmx.de> wrote:
>
> Building 5.10-rc1 in a setgid directory failed with the following
> error:
>
> dpkg-deb: error: control directory has bad permissions 2755 (must be
> >=0755 and <=0775)
>
> When building with fakeroot, the earlier chown call would have removed
> the setgid bits, but in a rootless build they remain.
>


Applied to linux-kbuild. Thanks.

I agreed with "g-s" but was not sure about "u-s"
because nothing is explained about setuid,
and the setuid bit against directories seems to have no effect.





It was interesting to read this article:
https://superuser.com/questions/471844/why-is-setuid-ignored-on-directories



Also, it is summarized in the wikipedia
https://en.wikipedia.org/wiki/Setuid#setuid_and_setgid_on_directories

"The setuid permission set on a directory is ignored on most UNIX and
Linux systems.[citation needed] However FreeBSD can be configured to
interpret setuid in a manner similar to setgid, in which case it
forces all files and sub-directories created in a directory to be
owned by that directory's owner - a simple form of inheritance.[5]
This is generally not needed on most systems derived from BSD, since
by default directories are treated as if their setgid bit is always
set, regardless of the actual value. As is stated in open(2), "When a
new file is created it is given the group of the directory which
contains it.""


After all, I am convinced that it would not hurt to do "u-s"
although I have never tested kernel builds on FreeBSD.










> Fixes: 3e8541803624 ("builddeb: Enable rootless builds")
> Cc: Guillem Jover <guillem@hadrons.org>
> Signed-off-by: Sven Joachim <svenjoac@gmx.de>
> ---
>  scripts/package/builddeb | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 1b11f8993629..91a502bb97e8 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -45,6 +45,8 @@ create_package() {
>         chmod -R go-w "$pdir"
>         # in case we are in a restrictive umask environment like 0077
>         chmod -R a+rX "$pdir"
> +       # in case we build in a setuid/setgid directory
> +       chmod -R ug-s "$pdir"
>
>         # Create the package
>         dpkg-gencontrol -p$pname -P"$pdir"
> --
> 2.28.0
>


--
Best Regards

Masahiro Yamada

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] builddeb: Consolidate consecutive chmod calls into one
  2020-10-26 19:32 ` [PATCH 2/2] builddeb: Consolidate consecutive chmod calls into one Sven Joachim
@ 2020-10-28  6:52   ` Masahiro Yamada
  2020-10-29 15:39     ` [PATCH] " Sven Joachim
  0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2020-10-28  6:52 UTC (permalink / raw)
  To: Sven Joachim
  Cc: Linux Kbuild mailing list, Michal Marek, Linux Kernel Mailing List

On Tue, Oct 27, 2020 at 4:32 AM Sven Joachim <svenjoac@gmx.de> wrote:
>
> No need to call chmod three times when it can do everything at once.
>
> Signed-off-by: Sven Joachim <svenjoac@gmx.de>
> ---
>  scripts/package/builddeb | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 91a502bb97e8..81ec6414726c 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -42,11 +42,7 @@ create_package() {
>         else
>                 chown -R root:root "$pdir"
>         fi
> -       chmod -R go-w "$pdir"
> -       # in case we are in a restrictive umask environment like 0077
> -       chmod -R a+rX "$pdir"
> -       # in case we build in a setuid/setgid directory
> -       chmod -R ug-s "$pdir"
> +       chmod -R go-w,a+rX,ug-s "$pdir"


You added the comment in 1/2, then
you are deleting it in this patch.

Could you keep the comments for clarification?


# a+rX in case we are in a restrictive umask environment like 0077
# ug-s in case we build in a setuid/setgid directory
chmod -R go-w,a+rX,ug-s "$pdir"






>         # Create the package
>         dpkg-gencontrol -p$pname -P"$pdir"
> --
> 2.28.0
>


--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory
  2020-10-28  6:00 ` [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Masahiro Yamada
@ 2020-10-28 18:30   ` Sven Joachim
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Joachim @ 2020-10-28 18:30 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Michal Marek,
	Linux Kernel Mailing List, Guillem Jover

On 2020-10-28 15:00 +0900, Masahiro Yamada wrote:

> On Tue, Oct 27, 2020 at 4:32 AM Sven Joachim <svenjoac@gmx.de> wrote:
>>
>> Building 5.10-rc1 in a setgid directory failed with the following
>> error:
>>
>> dpkg-deb: error: control directory has bad permissions 2755 (must be
>> >=0755 and <=0775)
>>
>> When building with fakeroot, the earlier chown call would have removed
>> the setgid bits, but in a rootless build they remain.
>>
>
>
> Applied to linux-kbuild. Thanks.

I don't see it there, have you pushed it out yet?

> I agreed with "g-s" but was not sure about "u-s"
> because nothing is explained about setuid,
> and the setuid bit against directories seems to have no effect.
>
>
>
>
>
> It was interesting to read this article:
> https://superuser.com/questions/471844/why-is-setuid-ignored-on-directories
>
>
>
> Also, it is summarized in the wikipedia
> https://en.wikipedia.org/wiki/Setuid#setuid_and_setgid_on_directories
>
> "The setuid permission set on a directory is ignored on most UNIX and
> Linux systems.[citation needed] However FreeBSD can be configured to
> interpret setuid in a manner similar to setgid, in which case it
> forces all files and sub-directories created in a directory to be
> owned by that directory's owner - a simple form of inheritance.[5]
> This is generally not needed on most systems derived from BSD, since
> by default directories are treated as if their setgid bit is always
> set, regardless of the actual value. As is stated in open(2), "When a
> new file is created it is given the group of the directory which
> contains it.""
>
>
> After all, I am convinced that it would not hurt to do "u-s"
> although I have never tested kernel builds on FreeBSD.

Agreed, setuid directories should not end up in the .deb files even if
that bit does currently nothing.

Cheers,
       Sven

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] builddeb: Consolidate consecutive chmod calls into one
  2020-10-28  6:52   ` Masahiro Yamada
@ 2020-10-29 15:39     ` Sven Joachim
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Joachim @ 2020-10-29 15:39 UTC (permalink / raw)
  To: linux-kbuild, Masahiro Yamada, Michal Marek; +Cc: linux-kernel, Sven Joachim

No need to call chmod three times when it can do everything at once.

Signed-off-by: Sven Joachim <svenjoac@gmx.de>
---
 scripts/package/builddeb | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 91a502bb97e8..6a100c449579 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -42,11 +42,9 @@ create_package() {
 	else
 		chown -R root:root "$pdir"
 	fi
-	chmod -R go-w "$pdir"
-	# in case we are in a restrictive umask environment like 0077
-	chmod -R a+rX "$pdir"
-	# in case we build in a setuid/setgid directory
-	chmod -R ug-s "$pdir"
+	# a+rX in case we are in a restrictive umask environment like 0077
+	# ug-s in case we build in a setuid/setgid directory
+	chmod -R go-w,a+rX,ug-s "$pdir"

 	# Create the package
 	dpkg-gencontrol -p$pname -P"$pdir"
--
2.29.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-29 15:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 19:32 [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Sven Joachim
2020-10-26 19:32 ` [PATCH 2/2] builddeb: Consolidate consecutive chmod calls into one Sven Joachim
2020-10-28  6:52   ` Masahiro Yamada
2020-10-29 15:39     ` [PATCH] " Sven Joachim
2020-10-28  6:00 ` [PATCH 1/2] builddeb: Fix rootless build in setuid/setgid directory Masahiro Yamada
2020-10-28 18:30   ` Sven Joachim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).