diff mbox series

[bpf-next,v2,1/3] selftests/bpf: Update ima_setup.sh for busybox

Message ID 20201202223944.456903-1-kpsingh@chromium.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next,v2,1/3] selftests/bpf: Update ima_setup.sh for busybox | expand

Checks

Context Check Description
netdev/cover_letter warning Series does not have a cover letter
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 85 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

KP Singh Dec. 2, 2020, 10:39 p.m. UTC 
From: KP Singh <kpsingh@google.com>

* losetup on busybox does not output the name of loop device on using
  -f with --show. It also dosn't support -j to find the loop devices
  for a given backing file. losetup is updated to use "-a" which is
  available on busybox.
* blkid does not support options (-s and -o) to only display the uuid.
* Not all environments have mkfs.ext4, the test requires a loop device
  with a backing image file which could formatted with any filesystem.
  Update to using mkfs.ext2 which is available on busybox.

Fixes: 34b82d3ac105 ("bpf: Add a selftest for bpf_ima_inode_hash")
Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: KP Singh <kpsingh@google.com>
---
 tools/testing/selftests/bpf/ima_setup.sh | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Andrii Nakryiko Dec. 2, 2020, 11:13 p.m. UTC | #1 
On Wed, Dec 2, 2020 at 2:39 PM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> * losetup on busybox does not output the name of loop device on using
>   -f with --show. It also dosn't support -j to find the loop devices
>   for a given backing file. losetup is updated to use "-a" which is
>   available on busybox.
> * blkid does not support options (-s and -o) to only display the uuid.
> * Not all environments have mkfs.ext4, the test requires a loop device
>   with a backing image file which could formatted with any filesystem.
>   Update to using mkfs.ext2 which is available on busybox.
>
> Fixes: 34b82d3ac105 ("bpf: Add a selftest for bpf_ima_inode_hash")
> Reported-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---

Thanks for the fixes!

You have three related patches, please add a cover letter to the patch
set as well.

>  tools/testing/selftests/bpf/ima_setup.sh | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> index 15490ccc5e55..d864c62c1207 100755
> --- a/tools/testing/selftests/bpf/ima_setup.sh
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -3,6 +3,7 @@
>
>  set -e
>  set -u
> +set -o pipefail
>
>  IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
>  TEST_BINARY="/bin/true"
> @@ -23,13 +24,15 @@ setup()
>
>          dd if=/dev/zero of="${mount_img}" bs=1M count=10
>
> -        local loop_device="$(losetup --find --show ${mount_img})"
> +        losetup -f "${mount_img}"
> +        local loop_device=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1)
>
> -        mkfs.ext4 "${loop_device}"
> +        mkfs.ext2 "${loop_device:?}"
>          mount "${loop_device}" "${mount_dir}"
>
>          cp "${TEST_BINARY}" "${mount_dir}"
> -        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> +       local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="\([^"]*\)".*/\1/')"

tabs/spaces mix up here?

> +
>          echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
>  }
>
> @@ -38,7 +41,8 @@ cleanup() {
>          local mount_img="${tmp_dir}/test.img"
>          local mount_dir="${tmp_dir}/mnt"
>
> -        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
> +        local loop_devices=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1)

didn't know about :?, fancy


> +
>          for loop_dev in "${loop_devices}"; do
>                  losetup -d $loop_dev
>          done
> --
> 2.29.2.576.ga3fc446d84-goog
>
KP Singh Dec. 3, 2020, 12:51 a.m. UTC | #2 
On Thu, Dec 3, 2020 at 12:13 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Dec 2, 2020 at 2:39 PM KP Singh <kpsingh@chromium.org> wrote:
> >
> > From: KP Singh <kpsingh@google.com>
> >
> > * losetup on busybox does not output the name of loop device on using
> >   -f with --show. It also dosn't support -j to find the loop devices
> >   for a given backing file. losetup is updated to use "-a" which is
> >   available on busybox.
> > * blkid does not support options (-s and -o) to only display the uuid.
> > * Not all environments have mkfs.ext4, the test requires a loop device
> >   with a backing image file which could formatted with any filesystem.
> >   Update to using mkfs.ext2 which is available on busybox.
> >
> > Fixes: 34b82d3ac105 ("bpf: Add a selftest for bpf_ima_inode_hash")
> > Reported-by: Andrii Nakryiko <andrii@kernel.org>
> > Signed-off-by: KP Singh <kpsingh@google.com>
> > ---
>
> Thanks for the fixes!
>
> You have three related patches, please add a cover letter to the patch
> set as well.

Will do.

>
> >  tools/testing/selftests/bpf/ima_setup.sh | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> > index 15490ccc5e55..d864c62c1207 100755
> > --- a/tools/testing/selftests/bpf/ima_setup.sh
> > +++ b/tools/testing/selftests/bpf/ima_setup.sh
> > @@ -3,6 +3,7 @@
> >
> >  set -e
> >  set -u
> > +set -o pipefail
> >
> >  IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> >  TEST_BINARY="/bin/true"
> > @@ -23,13 +24,15 @@ setup()
> >
> >          dd if=/dev/zero of="${mount_img}" bs=1M count=10
> >
> > -        local loop_device="$(losetup --find --show ${mount_img})"
> > +        losetup -f "${mount_img}"
> > +        local loop_device=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1)
> >
> > -        mkfs.ext4 "${loop_device}"
> > +        mkfs.ext2 "${loop_device:?}"
> >          mount "${loop_device}" "${mount_dir}"
> >
> >          cp "${TEST_BINARY}" "${mount_dir}"
> > -        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> > +       local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="\([^"]*\)".*/\1/')"
>
> tabs/spaces mix up here?

Yeah, it seems like the whole file is formatted with spaces and I
added tabs here. Added a patch
at the end of the series which reindents this. I have not added the
"Fixes" tag since it's not
a fix, do feel free to add it, if you think that's the norm for these
kinds of indentation fixes.

>
> > +
> >          echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
> >  }
> >
> > @@ -38,7 +41,8 @@ cleanup() {
> >          local mount_img="${tmp_dir}/test.img"
> >          local mount_dir="${tmp_dir}/mnt"
> >
> > -        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
> > +        local loop_devices=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1)
>
> didn't know about :?, fancy

Thanks :) It has helped me in rm -rf ${dir:?}/${file:?}

>
>
> > +
> >          for loop_dev in "${loop_devices}"; do
> >                  losetup -d $loop_dev
> >          done
> > --
> > 2.29.2.576.ga3fc446d84-goog
> >
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
index 15490ccc5e55..d864c62c1207 100755
--- a/tools/testing/selftests/bpf/ima_setup.sh
+++ b/tools/testing/selftests/bpf/ima_setup.sh
@@ -3,6 +3,7 @@ 
 
 set -e
 set -u
+set -o pipefail
 
 IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
 TEST_BINARY="/bin/true"
@@ -23,13 +24,15 @@  setup()
 
         dd if=/dev/zero of="${mount_img}" bs=1M count=10
 
-        local loop_device="$(losetup --find --show ${mount_img})"
+        losetup -f "${mount_img}"
+        local loop_device=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1)
 
-        mkfs.ext4 "${loop_device}"
+        mkfs.ext2 "${loop_device:?}"
         mount "${loop_device}" "${mount_dir}"
 
         cp "${TEST_BINARY}" "${mount_dir}"
-        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
+	local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="\([^"]*\)".*/\1/')"
+
         echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
 }
 
@@ -38,7 +41,8 @@  cleanup() {
         local mount_img="${tmp_dir}/test.img"
         local mount_dir="${tmp_dir}/mnt"
 
-        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
+        local loop_devices=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1)
+
         for loop_dev in "${loop_devices}"; do
                 losetup -d $loop_dev
         done