diff mbox series

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

Message ID 20201203005807.486320-2-kpsingh@chromium.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Fixes for ima selftest | expand

Checks

Context Check Description
netdev/cover_letter success Link
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. 3, 2020, 12:58 a.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. 3, 2020, 5:52 a.m. UTC | #1
On Wed, Dec 2, 2020 at 4:58 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

typo: doesn't

>   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.

... so parse it from full blkid output.

> * 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.

This one is great. It explains the problem, and what solution was
implemented, from the high-level. I'd just drop the " *" marks, it
makes it more pleasant to read as if it was written for humans, not
machines.

> 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(-)
>

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> index 15490ccc5e55..137f2d32598f 100755
> --- a/tools/testing/selftests/bpf/ima_setup.sh
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -3,6 +3,7 @@
>

[...]
KP Singh Dec. 3, 2020, 10:59 a.m. UTC | #2
On Thu, Dec 3, 2020 at 6:52 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Dec 2, 2020 at 4:58 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
>
> typo: doesn't

Fixed.

>
> >   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.
>
> ... so parse it from full blkid output.

Done.

>
> > * 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.
>
> This one is great. It explains the problem, and what solution was
> implemented, from the high-level. I'd just drop the " *" marks, it
> makes it more pleasant to read as if it was written for humans, not
> machines.

I tend to use "* " for bullet points from the markdown syntax
(as we use it heavily internally) I can avoid if you prefer / don't like it.


>
> > 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(-)
> >
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> > index 15490ccc5e55..137f2d32598f 100755
> > --- a/tools/testing/selftests/bpf/ima_setup.sh
> > +++ b/tools/testing/selftests/bpf/ima_setup.sh
> > @@ -3,6 +3,7 @@
> >
>
> [...]
Andrii Nakryiko Dec. 3, 2020, 7:18 p.m. UTC | #3
On Thu, Dec 3, 2020 at 3:00 AM KP Singh <kpsingh@chromium.org> wrote:
>
> On Thu, Dec 3, 2020 at 6:52 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Dec 2, 2020 at 4:58 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
> >
> > typo: doesn't
>
> Fixed.
>
> >
> > >   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.
> >
> > ... so parse it from full blkid output.
>
> Done.
>
> >
> > > * 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.
> >
> > This one is great. It explains the problem, and what solution was
> > implemented, from the high-level. I'd just drop the " *" marks, it
> > makes it more pleasant to read as if it was written for humans, not
> > machines.
>
> I tend to use "* " for bullet points from the markdown syntax
> (as we use it heavily internally) I can avoid if you prefer / don't like it.

A list of bullet points don't read as a coherent text. It's not the
end of the world, but it's also not a common style here either.

>
>
> >
> > > 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(-)
> > >
> >
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> > > index 15490ccc5e55..137f2d32598f 100755
> > > --- a/tools/testing/selftests/bpf/ima_setup.sh
> > > +++ b/tools/testing/selftests/bpf/ima_setup.sh
> > > @@ -3,6 +3,7 @@
> > >
> >
> > [...]
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
index 15490ccc5e55..137f2d32598f 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