diff mbox series

[bpf-next,v2,2/3] selftests/bpf: Ensure securityfs mount before writing ima policy

Message ID 20201202223944.456903-2-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 81 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>

SecurityFS may not be mounted even if it is enabled in the kernel
config.

Signed-off-by: KP Singh <kpsingh@google.com>
---
 tools/testing/selftests/bpf/ima_setup.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

KP Singh Dec. 2, 2020, 10:44 p.m. UTC | #1 
On Wed, Dec 2, 2020 at 11:39 PM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> SecurityFS may not be mounted even if it is enabled in the kernel
> config.
>
> Signed-off-by: KP Singh <kpsingh@google.com>

Fixes: 34b82d3ac105 ("bpf: Add a selftest for bpf_ima_inode_hash")
Reported-by: Andrii Nakryiko <andrii@kernel.org>
Andrii Nakryiko Dec. 2, 2020, 11:13 p.m. UTC | #2 
On Wed, Dec 2, 2020 at 2:39 PM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> SecurityFS may not be mounted even if it is enabled in the kernel
> config.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---

please add those tags in v2 :)

>  tools/testing/selftests/bpf/ima_setup.sh | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> index d864c62c1207..1da2d97a0e72 100755
> --- a/tools/testing/selftests/bpf/ima_setup.sh
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -14,6 +14,20 @@ usage()
>          exit 1
>  }
>
> +ensure_mount_securityfs()
> +{
> +        local securityfs_dir=$(grep "securityfs" /proc/mounts | awk '{print $2}')
> +
> +        if [ -z "${securityfs_dir}" ]; then
> +               securityfs_dir=/sys/kernel/security
> +               mount -t securityfs security "${securityfs_dir}"
> +       fi

again, something is off with indentation here

> +
> +       if [ ! -d "${securityfs_dir}" ]; then
> +               echo "${securityfs_dir} :securityfs is not mounted" && exit 1

nit: " :" => ": " or it was intended this way?


> +       fi
> +}
> +
>  setup()
>  {
>          local tmp_dir="$1"
> @@ -33,6 +47,7 @@ setup()
>          cp "${TEST_BINARY}" "${mount_dir}"
>         local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="\([^"]*\)".*/\1/')"
>
> +        ensure_mount_securityfs
>          echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
>  }
>
> --
> 2.29.2.576.ga3fc446d84-goog
>
KP Singh Dec. 3, 2020, 12:49 a.m. UTC | #3 
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>
> >
> > SecurityFS may not be mounted even if it is enabled in the kernel
> > config.
> >
> > Signed-off-by: KP Singh <kpsingh@google.com>
> > ---
>
> please add those tags in v2 :)

Done.

>
> >  tools/testing/selftests/bpf/ima_setup.sh | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> > index d864c62c1207..1da2d97a0e72 100755
> > --- a/tools/testing/selftests/bpf/ima_setup.sh
> > +++ b/tools/testing/selftests/bpf/ima_setup.sh
> > @@ -14,6 +14,20 @@ usage()
> >          exit 1
> >  }
> >
> > +ensure_mount_securityfs()
> > +{
> > +        local securityfs_dir=$(grep "securityfs" /proc/mounts | awk '{print $2}')
> > +
> > +        if [ -z "${securityfs_dir}" ]; then
> > +               securityfs_dir=/sys/kernel/security
> > +               mount -t securityfs security "${securityfs_dir}"
> > +       fi
>
> again, something is off with indentation here

Yeah, changed it to spaces and added a patch at the end which changes
it to tabs.
Checkpatch did not catch it, maybe it ignores shell scripts :(

>
> > +
> > +       if [ ! -d "${securityfs_dir}" ]; then
> > +               echo "${securityfs_dir} :securityfs is not mounted" && exit 1
>
> nit: " :" => ": " or it was intended this way?

Fixes. Thanks.

>
>
> > +       fi
> > +}
> > +
> >  setup()
> >  {
> >          local tmp_dir="$1"
> > @@ -33,6 +47,7 @@ setup()
> >          cp "${TEST_BINARY}" "${mount_dir}"
> >         local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="\([^"]*\)".*/\1/')"
> >
> > +        ensure_mount_securityfs
> >          echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
> >  }
> >
> > --
> > 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 d864c62c1207..1da2d97a0e72 100755
--- a/tools/testing/selftests/bpf/ima_setup.sh
+++ b/tools/testing/selftests/bpf/ima_setup.sh
@@ -14,6 +14,20 @@  usage()
         exit 1
 }
 
+ensure_mount_securityfs()
+{
+        local securityfs_dir=$(grep "securityfs" /proc/mounts | awk '{print $2}')
+
+        if [ -z "${securityfs_dir}" ]; then
+		securityfs_dir=/sys/kernel/security
+		mount -t securityfs security "${securityfs_dir}"
+	fi
+
+	if [ ! -d "${securityfs_dir}" ]; then
+		echo "${securityfs_dir} :securityfs is not mounted" && exit 1
+	fi
+}
+
 setup()
 {
         local tmp_dir="$1"
@@ -33,6 +47,7 @@  setup()
         cp "${TEST_BINARY}" "${mount_dir}"
 	local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="\([^"]*\)".*/\1/')"
 
+        ensure_mount_securityfs
         echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
 }