Re: [Audit-test-developer] [PATCH 4/5] audit-test/filter: add aarch64 support

[Jiri J. <jja...@re...>]

Hello,
the following is just a quick look and by no means a complete review.

On 07/03/2014 09:45 AM, AKASHI Takahiro wrote:
> On arm64/aarch64, some system calls are implemented in glibc using other
> primitive system calls, say open() vs. openat(). Therefore, audit logs
> have only records for primitive ones.
> 
> This patch adds work-arounds for these cases.
> 
> Signed-off-by: AKASHI Takahiro <tak...@li...>
> ---
>  audit-test/filter/tests/test_auid.bash             |    9 +++++++--
>  audit-test/filter/tests/test_class_attr.bash       |   13 +++++++++++++
>  audit-test/filter/tests/test_dev_inode.bash        |   11 ++++++++---
>  audit-test/filter/tests/test_success.bash          |    6 +++++-
>  audit-test/filter/tests/test_syscall.bash          |    6 +++++-
>  audit-test/filter/tests/test_type.bash             |    9 +++++++--
>  audit-test/filter/tests/test_watch_dir_remove.bash |   20 ++++++++++++--------
>  audit-test/filter/tests/test_watch_open.bash       |   10 ++++++++--
>  audit-test/filter/tests/test_watch_remove.bash     |    4 ++++
>  audit-test/rules.mk                                |    6 ++++--
>  10 files changed, 73 insertions(+), 21 deletions(-)
> 
> diff --git a/audit-test/filter/tests/test_auid.bash b/audit-test/filter/tests/test_auid.bash
> index c165cf3..63098b7 100755
> --- a/audit-test/filter/tests/test_auid.bash
> +++ b/audit-test/filter/tests/test_auid.bash
> @@ -33,8 +33,13 @@ do_open_file $tmp1
>  augrok --seek=$log_mark "name==$tmp1" "auid==$user_auid" \
>      && exit_error "Unexpected record found."
>  
> -auditctl -a exit,always -F arch=b$MODE -S open -F auid=$user_auid
> -prepend_cleanup "auditctl -d exit,always -F arch=b$MODE -S open -F auid=$user_auid"
> +if [ ${MACHINE} = "aarch64" ]; then
> +syscall_name="openat"
> +else
> +syscall_name="open"
> +fi

[ "$MACHINE" = "aarch64" ] && syscall_name="openat" || syscall_name="open"
would have been perhaps more compact, but yours works as well.

> +auditctl -a exit,always -F arch=b$MODE -S $syscall_name -F auid=$user_auid
> +prepend_cleanup "auditctl -d exit,always -F arch=b$MODE -S $syscall_name -F auid=$user_auid"
>  
>  # audit log marker
>  log_mark=$(stat -c %s $audit_log)

<snip>

> diff --git a/audit-test/rules.mk b/audit-test/rules.mk
> index 25c9758..4af7c13 100644
> --- a/audit-test/rules.mk
> +++ b/audit-test/rules.mk
> @@ -186,13 +186,15 @@ run.bash:
>  	[[ -f run.bash ]] || ln -sfn $(TOPDIR)/utils/run.bash run.bash
>  
>  run: all
> -	@$(check_set_PPROFILE); \
> +	@export MACHINE=$(MACHINE); \
> +	$(check_set_PPROFILE); \
>  	$(check_set_PASSWD); \
>  	./run.bash --header; \
>  	./run.bash
>  
>  rerun: all
> -	@$(check_set_PPROFILE); \
> +	@export MACHINE=$(MACHINE); \
> +	$(check_set_PPROFILE); \
>  	$(check_set_PASSWD); \
>  	./run.bash --rerun
>  endif
> 

Can't we do this in a less hack-ish way? What about this?

diff --git a/audit-test/rules.mk b/audit-test/rules.mk
index fd2f8a5..15b81e0 100644
--- a/audit-test/rules.mk
+++ b/audit-test/rules.mk
@@ -48,6 +48,8 @@ LINK_AR               = $(AR) rc $@ $^
 LINK_EXE       = $(CC) $(LDFLAGS) -o $@ $^ $(LOADLIBES) $(LDLIBS)
 LINK_SO                = $(CC) $(LDFLAGS) -shared -o $@ $^ $(LOADLIBES)
$(LDLIBS)

+export MACHINE
+
 # If MODE isn't set explicitly, the default for the machine is used
 export NATIVE   = $(strip $(shell file /bin/bash | awk -F'[ -]' '{print
$$3}'))
 export MODE     ?= $(NATIVE)


Jiri