[PATCH v14 03/12] selftests/landlock: Test IOCTL support

Günther Noack gnoack at google.com
Thu Apr 18 11:10:05 UTC 2024


On Fri, Apr 12, 2024 at 05:17:44PM +0200, Mickaël Salaün wrote:
> On Fri, Apr 05, 2024 at 09:40:31PM +0000, Günther Noack wrote:
> > Exercises Landlock's IOCTL feature in different combinations of
> > handling and permitting the LANDLOCK_ACCESS_FS_IOCTL_DEV right, and in
> > different combinations of using files and directories.
> > 
> > Signed-off-by: Günther Noack <gnoack at google.com>
> > ---
> >  tools/testing/selftests/landlock/fs_test.c | 227 ++++++++++++++++++++-
> >  1 file changed, 224 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> > index 418ad745a5dd..8a72e26d4977 100644
> > --- a/tools/testing/selftests/landlock/fs_test.c
> > +++ b/tools/testing/selftests/landlock/fs_test.c
> 
> > +TEST_F_FORK(ioctl, handle_dir_access_file)
> > +{
> > +	const int flag = 0;
> > +	const struct rule rules[] = {
> > +		{
> > +			.path = "/dev",
> > +			.access = variant->allowed,
> > +		},
> > +		{},
> > +	};
> > +	int file_fd, ruleset_fd;
> > +
> > +	/* Enables Landlock. */
> > +	ruleset_fd = create_ruleset(_metadata, variant->handled, rules);
> > +	ASSERT_LE(0, ruleset_fd);
> > +	enforce_ruleset(_metadata, ruleset_fd);
> > +	ASSERT_EQ(0, close(ruleset_fd));
> > +
> > +	file_fd = open("/dev/tty", variant->open_mode);
> 
> Why /dev/tty? Could we use /dev/null or something less tied to the
> current context and less sensitive?

Absolutely, good point -- I'm switching to /dev/zero.

I am dropping the TCGETS test and only keeping FIONREAD,
but these two IOCTL commands work the same way as far as Landlock is concerned.


> > +TEST_F_FORK(ioctl, handle_file_access_file)
> > +{
> > +	const int flag = 0;
> > +	const struct rule rules[] = {
> > +		{
> > +			.path = "/dev/tty0",
> 
> Same here (and elsewhere), /dev/null or a similar harmless device file
> would be better.

Ditto, /dev/zero it is.


> > +	if (variant->allowed & LANDLOCK_ACCESS_FS_READ_DIR) {
> > +		SKIP(return, "LANDLOCK_ACCESS_FS_READ_DIR "
> > +			     "can not be granted on files");
> 
> Can we avoid using SKIP() in this case?  Tests should only be skipped
> when not supported, in other words, we should be able to run all tests
> with the right combination of architecture and kernel options.

Good point.  After double checking, I realized that this was left over from an
earlier test where we still had more fixture variants.  I'm removing that check.


Thanks for the review!
—Günther



More information about the Linux-security-module-archive mailing list