On Sun, Feb 06, 2022 at 12:06:27PM +0100, Omar Polo wrote:
> here's a revised diff.  it's equivalent in practice to the previous one,
> but hopefully less scary :)

Thanks for this.  I have no means of testing this though (the kernel version I
have here on Arch Linux doesn't seem to offer Landlock) but I have a few
comment in-line below.

> P.S.: now that I think of it, there's still a thing that can be
> improved.  I went with compat/landlock.c because it was an easy way to
> add the support, but should we move that file elsewhere?

I've addressed this in a different reply in this thread.

> +#ifdef HAVE_LINUX_LANDLOCK_H
> +	/* revoke fs access */
> +	if (landlock_no_fs() == -1) {
> +		err = got_error_from_errno("landlock_no_fs");
> +		got_privsep_send_error(&ibuf, err);
> +		return 1;
> +	}
>  #endif
> +#endif

Is the main interface to landlock via landlock_no_fs() in all cases where
we're using it here?  If so, perhaps it would be better to do this in
got_compat.h:

#ifndef HAVE_LINUX_LANDLOCK_H
#define landlock_no_fs() (0)
#endif

That way, the difference in peppering the main codebase shrinks, meaning we
won't have too many merge conflicts in the future.

Kindly,
Thomas