Re: [Buildroot] [PATCH 2/7] package/minijail: new package

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

Hello José,

On Fri, 10 Dec 2021 16:12:04 +0200
José Pekkarinen <jose.pekkarinen@unikie.com> wrote:

> This patch adds a new package for minijail.
> 
> Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com>

I wanted to apply after fixing the issues pointed by others, but there
are in fact other issues, which I cannot resolve directly. See below.

> diff --git a/package/minijail/0001-Fix-prlimit-call.patch b/package/minijail/0001-Fix-prlimit-call.patch
> new file mode 100644
> index 0000000000..9f6902ed43
> --- /dev/null
> +++ b/package/minijail/0001-Fix-prlimit-call.patch
> @@ -0,0 +1,29 @@
> +From 09348f06104bf8101a24a0bce235a75a214e1380 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Jos=C3=A9=20Pekkarinen?= <jose.pekkarinen@unikie.com>
> +Date: Fri, 10 Dec 2021 14:20:30 +0200
> +Subject: [PATCH] Fix prlimit call

What are you "fixing"? You're replacing a prlimit() call by by
setrlimit(), but why? What problem does it solve? Why is it correct to
do that?

> diff --git a/package/minijail/0002-Fix-static-assert.patch b/package/minijail/0002-Fix-static-assert.patch
> new file mode 100644
> index 0000000000..48139e8baa
> --- /dev/null
> +++ b/package/minijail/0002-Fix-static-assert.patch
> @@ -0,0 +1,30 @@
> +From b5d91b793942747e5126e75abca2eebad60ab478 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Jos=C3=A9=20Pekkarinen?= <jose.pekkarinen@unikie.com>
> +Date: Fri, 10 Dec 2021 14:21:38 +0200
> +Subject: [PATCH] Fix static assert
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com>
> +---
> + libminijail.c | 3 ---
> + 1 file changed, 3 deletions(-)
> +
> +diff --git a/libminijail.c b/libminijail.c
> +index 45f133e..8323742 100644
> +--- a/libminijail.c
> ++++ b/libminijail.c
> +@@ -2620,9 +2620,6 @@ static int fd_is_open(int fd)
> + 	return fcntl(fd, F_GETFD) != -1 || errno != EBADF;
> + }
> + 
> +-static_assert(FD_SETSIZE >= MAX_PRESERVED_FDS * 2 - 1,
> +-	      "If true, ensure_no_fd_conflict will always find an unused fd.");

You're not fixing the static_assert() here but simply dropping it. Why?
When does it fail? What is the problem with it? Why is it safe to drop
it?

Could you take into account these comments as well as the others
received, and send an update patch?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot