[pacman-dev] [PATCH 1/2] makepkg: make $pkgdir non-accessible during build()
The idea of having separate build() and package() functions is that build() is run as a normal uses and package() as (fake)root. Any files placed in $pkgdir during build() can have the wrong permissions. Restrict access to $pkgdir during build() - unless there is no package() function. Also, set $pkgdir to something "useful" during build(). For split packages, this uses "<path>/pkg/$pkgbase" because it is not obvious which $pkgdir is being referred to. Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bcc415b..ec39c2e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -152,7 +152,7 @@ clean_up() { # If it's a clean exit and -c/--clean has been passed... msg "$(gettext "Cleaning up...")" - rm -rf "$pkgdir" "$srcdir" + rm -rf "$pkgdirbase" "$srcdir" if [[ -n $pkgbase ]]; then local fullver=$(get_full_version) # Can't do this unless the BUILDSCRIPT has been sourced. @@ -1519,7 +1519,7 @@ tidy_install() { if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then warning "$(gettext "Package contains reference to %s")" "\$srcdir" fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdir}" ; then + if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then warning "$(gettext "Package contains reference to %s")" "\$pkgdir" fi @@ -2344,16 +2344,14 @@ restore_package_variables() { run_split_packaging() { local pkgname_backup=${pkgname[@]} for pkgname in ${pkgname_backup[@]}; do - pkgdir="$pkgdir/$pkgname" - mkdir -p "$pkgdir" - chmod a-s "$pkgdir" + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir" backup_package_variables run_package $pkgname tidy_install create_package create_debug_package restore_package_variables - pkgdir="${pkgdir%/*}" done pkgname=${pkgname_backup[@]} } @@ -2689,12 +2687,16 @@ epoch=${epoch:-0} if [[ $BUILDDIR = "$startdir" ]]; then srcdir="$BUILDDIR/src" - pkgdir="$BUILDDIR/pkg" + pkgdirbase="$BUILDDIR/pkg" else srcdir="$BUILDDIR/$pkgbase/src" - pkgdir="$BUILDDIR/$pkgbase/pkg" + pkgdirbase="$BUILDDIR/$pkgbase/pkg" + fi +# set pkgdir to something "sensible" for (not recommended) use during build() +pkgdir="$pkgdirbase/$pkgbase" + if (( GENINTEG )); then mkdir -p "$srcdir" chmod a-s "$srcdir" @@ -2767,9 +2769,10 @@ if (( INFAKEROOT )); then exit 0 # $E_OK fi + chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then - pkgdir="$pkgdir/$pkgname" - mkdir -p "$pkgdir" + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir" if (( ! PKGFUNC )); then if (( ! REPKG )); then if (( BUILDFUNC )); then @@ -2786,7 +2789,6 @@ if (( INFAKEROOT )); then tidy_install create_package create_debug_package - pkgdir="${pkgdir%/*}" else run_split_packaging fi @@ -2881,7 +2883,7 @@ if (( NOEXTRACT )); then warning "$(gettext "Using existing %s tree")" "src/" elif (( REPKG )); then if (( ! PKGFUNC && ! SPLITPKG )) \ - && { [[ ! -d $pkgdir ]] || dir_is_empty "$pkgdir"; }; then + && { [[ ! -d $pkgdirbase ]] || dir_is_empty "$pkgdirbase"; }; then error "$(gettext "The package directory is empty, there is nothing to repackage!")" plain "$(gettext "Aborting...")" exit 1 @@ -2900,22 +2902,27 @@ if (( NOBUILD )); then exit 0 #E_OK else # check for existing pkg directory; don't remove if we are repackaging - if [[ -d $pkgdir ]] && (( ! REPKG || PKGFUNC || SPLITPKG )); then + if [[ -d $pkgdirbase ]] && (( ! REPKG || PKGFUNC || SPLITPKG )); then msg "$(gettext "Removing existing %s directory...")" "pkg/" - rm -rf "$pkgdir" + rm -rf "$pkgdirbase" fi - mkdir -p "$pkgdir" - chmod a-s "$pkgdir" + mkdir -p "$pkgdirbase" + chmod a-srwx "$pkgdirbase" cd_safe "$startdir" # if we are root or if fakeroot is not enabled, then we don't use it if ! check_buildenv "fakeroot" "y" || (( EUID == 0 )); then if (( ! REPKG )); then + if (( ! ( SPLITPKG || PKGFUNC ) )); then + chmod 755 "$pkgdirbase" + mkdir "$pkgdir" + fi (( BUILDFUNC )) && run_build (( CHECKFUNC )) && run_check fi + chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then - pkgdir="$pkgdir/$pkgname" + pkgdir="$pkgdirbase/$pkgname" mkdir -p "$pkgdir" if (( PKGFUNC )); then run_package @@ -2926,7 +2933,6 @@ else tidy_install create_package create_debug_package - pkgdir="${pkgdir%/*}" else run_split_packaging fi -- 1.8.1.2
On 2 February 2013 02:51, Allan McRae <allan@archlinux.org> wrote:
The idea of having separate build() and package() functions is that build() is run as a normal uses and package() as (fake)root. Any files placed in $pkgdir during build() can have the wrong permissions.
Restrict access to $pkgdir during build() - unless there is no package() function.
Also, set $pkgdir to something "useful" during build(). For split packages, this uses "<path>/pkg/$pkgbase" because it is not obvious which $pkgdir is being referred to.
Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bcc415b..ec39c2e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -152,7 +152,7 @@ clean_up() {
# If it's a clean exit and -c/--clean has been passed... msg "$(gettext "Cleaning up...")" - rm -rf "$pkgdir" "$srcdir" + rm -rf "$pkgdirbase" "$srcdir" if [[ -n $pkgbase ]]; then local fullver=$(get_full_version) # Can't do this unless the BUILDSCRIPT has been sourced. @@ -1519,7 +1519,7 @@ tidy_install() { if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then warning "$(gettext "Package contains reference to %s")" "\$srcdir" fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdir}" ; then + if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then warning "$(gettext "Package contains reference to %s")" "\$pkgdir" fi
@@ -2344,16 +2344,14 @@ restore_package_variables() { run_split_packaging() { local pkgname_backup=${pkgname[@]} for pkgname in ${pkgname_backup[@]}; do - pkgdir="$pkgdir/$pkgname" - mkdir -p "$pkgdir" - chmod a-s "$pkgdir" + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir" backup_package_variables run_package $pkgname tidy_install create_package create_debug_package restore_package_variables - pkgdir="${pkgdir%/*}" done pkgname=${pkgname_backup[@]} } @@ -2689,12 +2687,16 @@ epoch=${epoch:-0}
if [[ $BUILDDIR = "$startdir" ]]; then srcdir="$BUILDDIR/src" - pkgdir="$BUILDDIR/pkg" + pkgdirbase="$BUILDDIR/pkg" else srcdir="$BUILDDIR/$pkgbase/src" - pkgdir="$BUILDDIR/$pkgbase/pkg" + pkgdirbase="$BUILDDIR/$pkgbase/pkg" + fi
+# set pkgdir to something "sensible" for (not recommended) use during build() +pkgdir="$pkgdirbase/$pkgbase" + if (( GENINTEG )); then mkdir -p "$srcdir" chmod a-s "$srcdir" @@ -2767,9 +2769,10 @@ if (( INFAKEROOT )); then exit 0 # $E_OK fi
+ chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then - pkgdir="$pkgdir/$pkgname" - mkdir -p "$pkgdir" + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir"
Looks like removing the “-p” option causes a crash when using “--repackage” on a PKGBUILD with no package() function. ==> WARNING: Using a PKGBUILD without a package() function is deprecated. ==> Checking runtime dependencies... ==> Checking buildtime dependencies... ==> Entering fakeroot environment... mkdir: cannot create directory ‘/home/proj/pacman/pacman/pkg/test’: File exists /usr/bin/fakeroot: line 178: 5004 User defined signal 1 FAKEROOTKEY=$FAKEROOTKEY LD_LIBRARY_PATH="$PATHS" LD_PRELOAD="$LIB" "$@" Not that I ever used this option, just seeing what it takes to trigger the empty package directory error below.
if (( ! PKGFUNC )); then if (( ! REPKG )); then if (( BUILDFUNC )); then @@ -2786,7 +2789,6 @@ if (( INFAKEROOT )); then tidy_install create_package create_debug_package - pkgdir="${pkgdir%/*}" else run_split_packaging fi @@ -2881,7 +2883,7 @@ if (( NOEXTRACT )); then warning "$(gettext "Using existing %s tree")" "src/" elif (( REPKG )); then if (( ! PKGFUNC && ! SPLITPKG )) \ - && { [[ ! -d $pkgdir ]] || dir_is_empty "$pkgdir"; }; then + && { [[ ! -d $pkgdirbase ]] || dir_is_empty "$pkgdirbase"; }; then error "$(gettext "The package directory is empty, there is nothing to repackage!")" plain "$(gettext "Aborting...")" exit 1 @@ -2900,22 +2902,27 @@ if (( NOBUILD )); then exit 0 #E_OK else # check for existing pkg directory; don't remove if we are repackaging - if [[ -d $pkgdir ]] && (( ! REPKG || PKGFUNC || SPLITPKG )); then + if [[ -d $pkgdirbase ]] && (( ! REPKG || PKGFUNC || SPLITPKG )); then msg "$(gettext "Removing existing %s directory...")" "pkg/" - rm -rf "$pkgdir" + rm -rf "$pkgdirbase" fi - mkdir -p "$pkgdir" - chmod a-s "$pkgdir" + mkdir -p "$pkgdirbase" + chmod a-srwx "$pkgdirbase"
Nice touch, makes it hard to accidentally install files there during build().
cd_safe "$startdir"
# if we are root or if fakeroot is not enabled, then we don't use it if ! check_buildenv "fakeroot" "y" || (( EUID == 0 )); then if (( ! REPKG )); then + if (( ! ( SPLITPKG || PKGFUNC ) )); then + chmod 755 "$pkgdirbase" + mkdir "$pkgdir" + fi (( BUILDFUNC )) && run_build (( CHECKFUNC )) && run_check fi + chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then - pkgdir="$pkgdir/$pkgname" + pkgdir="$pkgdirbase/$pkgname" mkdir -p "$pkgdir" if (( PKGFUNC )); then run_package @@ -2926,7 +2933,6 @@ else tidy_install create_package create_debug_package - pkgdir="${pkgdir%/*}" else run_split_packaging fi
On 02/02/13 14:16, Martin Panter wrote:
On 2 February 2013 02:51, Allan McRae <allan@archlinux.org> wrote:
The idea of having separate build() and package() functions is that build() is run as a normal uses and package() as (fake)root. Any files placed in $pkgdir during build() can have the wrong permissions.
Restrict access to $pkgdir during build() - unless there is no package() function.
Also, set $pkgdir to something "useful" during build(). For split packages, this uses "<path>/pkg/$pkgbase" because it is not obvious which $pkgdir is being referred to.
Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bcc415b..ec39c2e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -152,7 +152,7 @@ clean_up() {
# If it's a clean exit and -c/--clean has been passed... msg "$(gettext "Cleaning up...")" - rm -rf "$pkgdir" "$srcdir" + rm -rf "$pkgdirbase" "$srcdir" if [[ -n $pkgbase ]]; then local fullver=$(get_full_version) # Can't do this unless the BUILDSCRIPT has been sourced. @@ -1519,7 +1519,7 @@ tidy_install() { if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then warning "$(gettext "Package contains reference to %s")" "\$srcdir" fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdir}" ; then + if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then warning "$(gettext "Package contains reference to %s")" "\$pkgdir" fi
@@ -2344,16 +2344,14 @@ restore_package_variables() { run_split_packaging() { local pkgname_backup=${pkgname[@]} for pkgname in ${pkgname_backup[@]}; do - pkgdir="$pkgdir/$pkgname" - mkdir -p "$pkgdir" - chmod a-s "$pkgdir" + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir" backup_package_variables run_package $pkgname tidy_install create_package create_debug_package restore_package_variables - pkgdir="${pkgdir%/*}" done pkgname=${pkgname_backup[@]} } @@ -2689,12 +2687,16 @@ epoch=${epoch:-0}
if [[ $BUILDDIR = "$startdir" ]]; then srcdir="$BUILDDIR/src" - pkgdir="$BUILDDIR/pkg" + pkgdirbase="$BUILDDIR/pkg" else srcdir="$BUILDDIR/$pkgbase/src" - pkgdir="$BUILDDIR/$pkgbase/pkg" + pkgdirbase="$BUILDDIR/$pkgbase/pkg" + fi
+# set pkgdir to something "sensible" for (not recommended) use during build() +pkgdir="$pkgdirbase/$pkgbase" + if (( GENINTEG )); then mkdir -p "$srcdir" chmod a-s "$srcdir" @@ -2767,9 +2769,10 @@ if (( INFAKEROOT )); then exit 0 # $E_OK fi
+ chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then - pkgdir="$pkgdir/$pkgname" - mkdir -p "$pkgdir" + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir"
Looks like removing the “-p” option causes a crash when using “--repackage” on a PKGBUILD with no package() function.
==> WARNING: Using a PKGBUILD without a package() function is deprecated. ==> Checking runtime dependencies... ==> Checking buildtime dependencies... ==> Entering fakeroot environment... mkdir: cannot create directory ‘/home/proj/pacman/pacman/pkg/test’: File exists /usr/bin/fakeroot: line 178: 5004 User defined signal 1 FAKEROOTKEY=$FAKEROOTKEY LD_LIBRARY_PATH="$PATHS" LD_PRELOAD="$LIB" "$@"
Not that I ever used this option, just seeing what it takes to trigger the empty package directory error below.
Nice catch... I forgot "-p" is not just to create parents! Fixed on my working branch. Allan
participants (2)
-
Allan McRae
-
Martin Panter