Re: [Buildroot] [PATCH 04/31] package/diffutils: disable busybox diff

["Yann E. MORIN" <yann.morin.1998@free.fr>]

François, All,

On 2024-03-16 11:59 +0100, François Perrad spake thusly:
> Le ven. 15 mars 2024 à 20:57, Arnout Vandecappelle via buildroot <[1]
> buildroot@buildroot.org> a écrit :
>     On 13/03/2024 10:13, Francois Perrad wrote:
>     > Signed-off-by: Francois Perrad <[2]francois.perrad@gadz.org>
>     > ---
>     >   package/diffutils/[3]diffutils.mk | 4 ++++
>     >   1 file changed, 4 insertions(+)
>     >
>     > diff --git a/package/diffutils/[4]diffutils.mk b/package/diffutils/[5]
>     diffutils.mk
>     > index 111926686..fe20a0e67 100644
>     > --- a/package/diffutils/[6]diffutils.mk
>     > +++ b/package/diffutils/[7]diffutils.mk
>     > @@ -18,4 +18,8 @@ ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
>     >   DIFFUTILS_CONF_ENV += gl_cv_func_getopt_gnu=yes
>     >   endif
>     >   
>     > +define DIFFUTILS_BUSYBOX_CONFIG_FIXUPS
>     > +     $(call KCONFIG_DISABLE_OPT,CONFIG_DIFF)
> 
>       I don't understand the point of this series. In general, there is no
>     reason to
>     disable applets from busybox just because there's a "full" alternative
>     installed
>     on the system. Usually the busybox size difference is completely negligible
>     compared to the the "full" installation size. And the busybox one can still
>     be
>     useful in some cases (when called as 'busybox diff' instead of 'diff').
> 
>       If you really care about removing the redundant busybox options, you can
>     easily supply a custom busybox config.
> 
>       Is there a really good reason for all this?
> 
> In fact, in this serie, the only important patches are the 2 first.

I didn't understand what was so important in the first patch, so you'd
have to extend the commit log with a bit more explanations, should you
respin it later. Also, it seems totally unrelated to the rest of the
series, so it should probably be sent separately.

> I wrote the nexts, like this one, in order to show an use case of the 2nd patch
> "package/busybox: handle LIBFOO_BUSYBOX_CONFIG_FIXUPS".

As for patch 2, I see that it is modelled after the similar feature we
have for the linux kernel. It would seem reasonable to have in the first
place. However, I still fail to see what the point really is.

Indeed, for the kernel, the feature is there because packages need a
feature from the kerenl, so we want to allo them to enable it, very
seldom to disable them [0].

But for busybox, there is not need to explicitly disable options: the
install order guarantees that the full-blown variant win over the
busybox applets.

Also, as Arnout pointed out, it is still possible and interesting to
keep the applets in busybox even when the ful-blown variant is enabled.
Indeed, it is possible to build an initramfs that only contains busybox,
which applets are used until the final root is mounted and swithroot-ed
into.

So, we do not ned a feature to _disable_ options in Busybox.

Now, on the other side of the coin, do we need a feature that allows
packages to _enable_ options in Busybox? Unlike the kernel, it is
possible to have a generic Busybox configuration that works "almost
everywhere" (well, two: one for MMU, one for noMMU). So that's the path
we have chosen in Buildroot: we cary those two configs, and there is no
reason to automatically tweak those; a concerned user will have to
provide their own, tailored to their particular use-case.

> Note that my goal is not to reducing the image size, but to reducing the attack
> surface (from a cyber security point of view).

Although I do laud the effort, this goes against the points mentioned
above. A user who wants to address security will have to review their
Busybox config and decide whether to drop or keep options, based on
their use-case (esp. the initramfs case).

> Busybox uses the Kconfig infrastructure.
> Currently, it could be configured via a full custom defconfig or via fragments.
> But, I think that altering the configuration by fixups could be also useful.

The fact that it uses the Kconfig infra is no reason in itself to make
it behave like the kernel (otherwise we'd have to do the same for all
the other kconfig-based packages) [1]

So, given all the above, I would say that we do not need this feature at
all for busybox.

[0] we have only three packages that disable kernel options:
  - systemd disables legacy sysfs (which were entirely dropped in v6.4)
  - kernel-module-imx-gpu-viv (a driver) disables an incompatible driver
    specific to the IMX kernel fork
  - zfs which needs stuff unavailable when those options are set (like
    unused ksyms)
  Those are really special cases; usually packages need to enable
  options.

[1] and if we went that route, then there would be no reason why we
would not have to have a mechanism for packages to be able to force
--{enable,disable,with,without}-option of other, autoconf-based packages,
or -D{ENABLE,DISABLE}_OPTION for other, cmake-based packages, and
similarly for all infras. Nope; no, no, no...

Regards,
Yann E. MORIN.

> 
> Francois
>  
> 
>       Regards,
>       Arnout
> 
>     > +endef
>     > +
>     >   $(eval $(autotools-package))
>     _______________________________________________
>     buildroot mailing list
>     [8]buildroot@buildroot.org
>     [9]https://lists.buildroot.org/mailman/listinfo/buildroot
> 
> 
> References:
> 
> [1] mailto:buildroot@buildroot.org
> [2] mailto:francois.perrad@gadz.org
> [3] http://diffutils.mk/
> [4] http://diffutils.mk/
> [5] http://diffutils.mk/
> [6] http://diffutils.mk/
> [7] http://diffutils.mk/
> [8] mailto:buildroot@buildroot.org
> [9] https://lists.buildroot.org/mailman/listinfo/buildroot

> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot


-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot