linux-kernel - Re: [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 15 May 2023 10:28:32 +0900
From:   Mark Brown <broonie@...nel.org>
To:     Ivan Orlov <ivan.orlov0322@...il.com>
Cc:     corbet@....net, akpm@...ux-foundation.org, perex@...ex.cz,
        tiwai@...e.com, skhan@...uxfoundation.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        alsa-devel@...a-project.org, linux-kselftest@...r.kernel.org,
        gregkh@...uxfoundation.org, himadrispandya@...il.com,
        linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver

On Sun, May 14, 2023 at 12:20:37AM +0400, Ivan Orlov wrote:

> +uid=$(id -u)
> +if [ $uid -ne 0 ]; then
> +	echo "$0: Must be run as root"
> +	exit 1
> +fi

It is not an error to run the selftest as a non-root user, the test
should be skipped.

> +modprobe snd-valsa

We don't check if the module was already loaded.

> +if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
> +	mount -t debugfs none /sys/kernel/debug
> +
> +	if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
> +		echo "$0: Error mounting debugfs"
> +		exit 4
> +	fi
> +fi

This will unconditionally attempt to mount debugfs in a standard
location and won't clean up after itself, if the system didn't have
debugfs mounted for some reason then this will leave it sitting there.

> +success="1"
> +arecord -D hw:CARD=valsa,DEV=0 -c 1 --buffer-size=8192 -f S16_LE -r 8000 --duration=4 out.wav
> +
> +if [[ $(< /sys/kernel/debug/valsa/ioctl_test) == "$success" ]]; then
> +	echo "$0: ioctl test: success"
> +else
> +	echo "$0: ioctl test: fail"
> +fi

Would it not be better to have a C program that actually calls the ioctl
rather than a custom debugfs thing that may or may not be wired up to do
the same thing as an ioctl would?  It seems like other than whatever
this ioctl test actually does this is all just a simplified version of
the existing pcm-test.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)