linux-kernel - Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test for bpftool access to read-only protected maps

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]

Message-ID: <20250611213947.21534-1-slava.imameev@crowdstrike.com>
Date: Thu, 12 Jun 2025 07:39:47 +1000
From: Slava Imameev <slava.imameev@...wdstrike.com>
To: <qmo@...nel.org>
CC: <andrii@...nel.org>, <ast@...nel.org>, <bpf@...r.kernel.org>,
        <daniel@...earbox.net>, <eddyz87@...il.com>, <haoluo@...gle.com>,
        <john.fastabend@...il.com>, <jolsa@...nel.org>,
        <justin.deschamp@...wdstrike.com>, <kpsingh@...nel.org>,
        <linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
        <mark.fontana@...wdstrike.com>, <martin.lau@...ux.dev>,
        <mykolal@...com>, <sdf@...ichev.me>, <shuah@...nel.org>,
        <slava.imameev@...wdstrike.com>, <song@...nel.org>,
        <yonghong.song@...ux.dev>
Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test for bpftool access to read-only protected maps

> > +	make -C tools/bpf/bpftool -s -j"$ncpus" OUTPUT="$output_dir"/ >/dev/null
> > +	echo ... finished building bpftool
> > +	cd "$pwd"
> > +}
>
>
> Given that you're reusing the BPF selftests infra, you shouldn't have to
> rebuild bpftool as part of the test. It's already built from the
> Makefile, and other tests just assume it's available already (see
> test_bpftool.py, test_bpftool.sh).


Agree, the build step will be removed for v3.


> > +	# Test write access to the map
> > +	if "$bpftool_path" map update name "$map_name" key $key value $value; then
> > +		if [ "$write_should_succeed" = "true" ]; then
> > +			echo "  Write access to $map_name succeeded as expected"
> > +		else
> > +			echo "  Write access to $map_name succeeded but should have failed"
> > +			exit 1
> > +		fi
> > +	else
> > +		if [ "$write_should_succeed" = "true" ]; then
> > +			echo "  Write access to $map_name failed but should have succeeded"
> > +			exit 1
> > +		else
> > +			echo "  Write access to $map_name failed as expected"
> > +		fi
> > +	fi
> 
> 
> Can we try to delete an item as well, please?

I added an item deletion test to v3.

> > +
> > +	# Pin the map to the BPF filesystem
> > +	"$bpftool_path" map pin name "$map_name" "$pin_path"
> > +	if [ -e "$pin_path" ]; then
> > +		echo "  Successfully pinned $map_name to $pin_path"
> > +	else
> > +		echo "  Failed to pin $map_name"
> > +		exit 1
> > +	fi
> > +
> > +	# Test read access to the pinned map
> > +	if "$bpftool_path" map lookup pinned "$pin_path" key $key; then
> > +		echo "  Read access to pinned $map_name succeeded"
> > +	else
> > +		echo "  Read access to pinned $map_name failed"
> > +		exit 1
> > +	fi
> > +
> > +	# Test write access to the pinned map
> > +	if "$bpftool_path" map update pinned "$pin_path" key $key value $value; then
> > +		if [ "$write_should_succeed" = "true" ]; then
> > +			echo "  Write access to pinned $map_name succeeded as expected"
> > +		else
> > +			echo "  Write access to pinned $map_name succeeded but should have failed"
> > +			exit 1
> > +		fi
> > +	else
> > +		if [ "$write_should_succeed" = "true" ]; then
> > +			echo "  Write access to pinned $map_name failed but should have succeeded"
> > +			exit 1
> > +		else
> > +			echo "  Write access to pinned $map_name failed as expected"
> > +		fi
> > +	fi
> 
> 
> Maybe refactor lookup/update as a function that you can call before and
> after pinning the map? (I don't mind much.)


I changed it as suggested for v3.


> > +check_bpffs() {
> > +	if [ -z "$BPF_FS" ]; then
> > +		echo "Could not run test without bpffs mounted"
> 
> 
> Why not? Bpftool will attempt to mount it for you if it's not available
> (create_and_mount_bpffs_dir()).
> 
> You could mount it manually to a specific location and unmount it during
>  the clean-up phase, if you wanted to be sure that the test doesn't have
> any side effect on the filesystem.


I made changes as suggested for v3.


> > +# Load and attach the BPF programs to control maps access
> > +"$BPFTOOL_PATH" prog loadall "$BPF_FILE_PATH" "$BPF_DIR"/prog autoattach
> > +
> > +# Test protected map (write should fail)
> > +test_map_access "$PROTECTED_MAP_NAME" "false" "$BPFTOOL_PATH" "$BPF_DIR"
> > +
> > +# Test not protected map (write should succeed)
> > +test_map_access "$NOT_PROTECTED_MAP_NAME" "true" "$BPFTOOL_PATH" "$BPF_DIR"
> 
> 
> We could also test map creation here (possibly even with inner maps).

I added a test for map-of-maps creation for v3.