netcat: add forced dependence on BusyBox

Message ID 20171012151418.3523-2-casantos@datacom.ind.br
State Accepted
Headers show
Series
  • netcat: add forced dependence on BusyBox
Related show

Commit Message

Carlos Santos Oct. 12, 2017, 3:14 p.m.
It may be necessary if packages become built in parallel, leading to a
race condition on the creation of the "nc" link. Moreover, netcat is as
shy as BusyBox and would not override an existing file/link, so we must
remove $(TARGET_DIR)/usr/bin/nc in advance.

Of course this still leaves a race conditon if other netcat competitors
but we must assume thet the user is a grown-up person who knows what is
doing.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 package/netcat/netcat.mk | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Romain Naour March 31, 2018, 10:34 a.m. | #1
Hi Carlos,

Le 12/10/2017 à 17:14, Carlos Santos a écrit :
> It may be necessary if packages become built in parallel, leading to a
> race condition on the creation of the "nc" link. Moreover, netcat is as
> shy as BusyBox and would not override an existing file/link, so we must
> remove $(TARGET_DIR)/usr/bin/nc in advance.
> 
> Of course this still leaves a race conditon if other netcat competitors
> but we must assume thet the user is a grown-up person who knows what is
> doing.

It seems you are using a custom busybox config file because the netcat applet is
disabled by default.

I'm agree, the dependency on busybox is missing but the TARGET_HOOKS doesn't
seems necessary. I removed it and netcat package is able to override the busybox
symlink.

 /usr/bin/install -c netcat output/target/usr/bin/netcat

Maybe we should check if other packages depending on
BR2_PACKAGE_BUSYBOX_SHOW_OTHERS have a dependency on busybox.

Best regards,
Romain

> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
>  package/netcat/netcat.mk | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/package/netcat/netcat.mk b/package/netcat/netcat.mk
> index eb7ddcac27..c032eea56d 100644
> --- a/package/netcat/netcat.mk
> +++ b/package/netcat/netcat.mk
> @@ -9,4 +9,15 @@ NETCAT_SITE = http://downloads.sourceforge.net/project/netcat/netcat/$(NETCAT_VE
>  NETCAT_LICENSE = GPL-2.0+
>  NETCAT_LICENSE_FILES = COPYING
>  
> +# Ensure Busybox gets built/installed before, so that this package
> +# overrides Busybox nc. We must remove an existing file/link because
> +# netcat is as shy as Busybox and would not override existing files.
> +ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> +NETCAT_DEPENDENCIES += busybox
> +define NETCAT_RMOVE_NC_LINK
> +	rm -f $(TARGET_DIR)/usr/bin/nc
> +endef
> +NETCAT_PRE_INSTALL_TARGET_HOOKS += NETCAT_RMOVE_NC_LINK
> +endif
> +
>  $(eval $(autotools-package))
>
Arnout Vandecappelle March 31, 2018, 1:03 p.m. | #2
On 31-03-18 12:34, Romain Naour wrote:
> Hi Carlos,
> 
> Le 12/10/2017 à 17:14, Carlos Santos a écrit :
>> It may be necessary if packages become built in parallel, leading to a
>> race condition on the creation of the "nc" link. Moreover, netcat is as
>> shy as BusyBox and would not override an existing file/link, so we must
>> remove $(TARGET_DIR)/usr/bin/nc in advance.
>>
>> Of course this still leaves a race conditon if other netcat competitors
>> but we must assume thet the user is a grown-up person who knows what is
>> doing.
> 
> It seems you are using a custom busybox config file because the netcat applet is
> disabled by default.
> 
> I'm agree, the dependency on busybox is missing but the TARGET_HOOKS doesn't
> seems necessary. I removed it and netcat package is able to override the busybox
> symlink.
> 
>  /usr/bin/install -c netcat output/target/usr/bin/netcat

 And anyway, in the context of PPS we will eventually have to reverse the
busybox dependencies, i.e. busybox will depend on netcat.


> Maybe we should check if other packages depending on
> BR2_PACKAGE_BUSYBOX_SHOW_OTHERS have a dependency on busybox.

 check-uniq-files should cover this.

 Regards,
 Arnout

> 
> Best regards,
> Romain
> 
>>
>> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>> ---
>>  package/netcat/netcat.mk | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/package/netcat/netcat.mk b/package/netcat/netcat.mk
>> index eb7ddcac27..c032eea56d 100644
>> --- a/package/netcat/netcat.mk
>> +++ b/package/netcat/netcat.mk
>> @@ -9,4 +9,15 @@ NETCAT_SITE = http://downloads.sourceforge.net/project/netcat/netcat/$(NETCAT_VE
>>  NETCAT_LICENSE = GPL-2.0+
>>  NETCAT_LICENSE_FILES = COPYING
>>  
>> +# Ensure Busybox gets built/installed before, so that this package
>> +# overrides Busybox nc. We must remove an existing file/link because
>> +# netcat is as shy as Busybox and would not override existing files.
>> +ifeq ($(BR2_PACKAGE_BUSYBOX),y)
>> +NETCAT_DEPENDENCIES += busybox
>> +define NETCAT_RMOVE_NC_LINK
>> +	rm -f $(TARGET_DIR)/usr/bin/nc
>> +endef
>> +NETCAT_PRE_INSTALL_TARGET_HOOKS += NETCAT_RMOVE_NC_LINK
>> +endif
>> +
>>  $(eval $(autotools-package))
>>
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Yann E. MORIN March 31, 2018, 1:17 p.m. | #3
Arnout, All,

On 2018-03-31 15:03 +0200, Arnout Vandecappelle spake thusly:
> On 31-03-18 12:34, Romain Naour wrote:
> > Hi Carlos,
> > 
> > Le 12/10/2017 à 17:14, Carlos Santos a écrit :
> >> It may be necessary if packages become built in parallel, leading to a
> >> race condition on the creation of the "nc" link. Moreover, netcat is as
> >> shy as BusyBox and would not override an existing file/link, so we must
> >> remove $(TARGET_DIR)/usr/bin/nc in advance.
> >>
> >> Of course this still leaves a race conditon if other netcat competitors
> >> but we must assume thet the user is a grown-up person who knows what is
> >> doing.
> > 
> > It seems you are using a custom busybox config file because the netcat applet is
> > disabled by default.
> > 
> > I'm agree, the dependency on busybox is missing but the TARGET_HOOKS doesn't
> > seems necessary. I removed it and netcat package is able to override the busybox
> > symlink.
> > 
> >  /usr/bin/install -c netcat output/target/usr/bin/netcat
> 
>  And anyway, in the context of PPS we will eventually have to reverse the
> busybox dependencies, i.e. busybox will depend on netcat.

Actually, we can do that even before PPS...

Regards,
Yann E. MORIN.

> > Maybe we should check if other packages depending on
> > BR2_PACKAGE_BUSYBOX_SHOW_OTHERS have a dependency on busybox.
> 
>  check-uniq-files should cover this.
> 
>  Regards,
>  Arnout
> 
> > 
> > Best regards,
> > Romain
> > 
> >>
> >> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> >> ---
> >>  package/netcat/netcat.mk | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/package/netcat/netcat.mk b/package/netcat/netcat.mk
> >> index eb7ddcac27..c032eea56d 100644
> >> --- a/package/netcat/netcat.mk
> >> +++ b/package/netcat/netcat.mk
> >> @@ -9,4 +9,15 @@ NETCAT_SITE = http://downloads.sourceforge.net/project/netcat/netcat/$(NETCAT_VE
> >>  NETCAT_LICENSE = GPL-2.0+
> >>  NETCAT_LICENSE_FILES = COPYING
> >>  
> >> +# Ensure Busybox gets built/installed before, so that this package
> >> +# overrides Busybox nc. We must remove an existing file/link because
> >> +# netcat is as shy as Busybox and would not override existing files.
> >> +ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> >> +NETCAT_DEPENDENCIES += busybox
> >> +define NETCAT_RMOVE_NC_LINK
> >> +	rm -f $(TARGET_DIR)/usr/bin/nc
> >> +endef
> >> +NETCAT_PRE_INSTALL_TARGET_HOOKS += NETCAT_RMOVE_NC_LINK
> >> +endif
> >> +
> >>  $(eval $(autotools-package))
> >>
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
> > 
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Arnout Vandecappelle March 31, 2018, 10:25 p.m. | #4
On 31-03-18 12:34, Romain Naour wrote:
> Hi Carlos,
> 
> Le 12/10/2017 à 17:14, Carlos Santos a écrit :
>> It may be necessary if packages become built in parallel, leading to a
>> race condition on the creation of the "nc" link. Moreover, netcat is as
>> shy as BusyBox and would not override an existing file/link, so we must
>> remove $(TARGET_DIR)/usr/bin/nc in advance.
>>
>> Of course this still leaves a race conditon if other netcat competitors
>> but we must assume thet the user is a grown-up person who knows what is
>> doing.
> 
> It seems you are using a custom busybox config file because the netcat applet is
> disabled by default.
> 
> I'm agree, the dependency on busybox is missing but the TARGET_HOOKS doesn't
> seems necessary. I removed it and netcat package is able to override the busybox
> symlink.
> 
>  /usr/bin/install -c netcat output/target/usr/bin/netcat

 I've applied, after removing the unneeded removal of nc.

 If it turns out to be needed after all, please submit a follow-up patch.

 Regards,
 Arnout

> 
> Maybe we should check if other packages depending on
> BR2_PACKAGE_BUSYBOX_SHOW_OTHERS have a dependency on busybox.
[snip]
Arnout Vandecappelle March 31, 2018, 10:31 p.m. | #5
On 01-04-18 00:25, Arnout Vandecappelle wrote:
> 
> 
> On 31-03-18 12:34, Romain Naour wrote:
>> Hi Carlos,
>>
>> Le 12/10/2017 à 17:14, Carlos Santos a écrit :
>>> It may be necessary if packages become built in parallel, leading to a
>>> race condition on the creation of the "nc" link. Moreover, netcat is as
>>> shy as BusyBox and would not override an existing file/link, so we must
>>> remove $(TARGET_DIR)/usr/bin/nc in advance.
>>>
>>> Of course this still leaves a race conditon if other netcat competitors
>>> but we must assume thet the user is a grown-up person who knows what is
>>> doing.
>>
>> It seems you are using a custom busybox config file because the netcat applet is
>> disabled by default.
>>
>> I'm agree, the dependency on busybox is missing but the TARGET_HOOKS doesn't
>> seems necessary. I removed it and netcat package is able to override the busybox
>> symlink.
>>
>>  /usr/bin/install -c netcat output/target/usr/bin/netcat
> 
>  I've applied, after removing the unneeded removal of nc.
> 
>  If it turns out to be needed after all, please submit a follow-up patch.

 OK, so it *was* needed, so I pushed a follow-up commit that does the removal.

 I've done that unconditionally, though, the 'nc' could come from somewhere else
as well.

 Regards,
 Arnout

Patch

diff --git a/package/netcat/netcat.mk b/package/netcat/netcat.mk
index eb7ddcac27..c032eea56d 100644
--- a/package/netcat/netcat.mk
+++ b/package/netcat/netcat.mk
@@ -9,4 +9,15 @@  NETCAT_SITE = http://downloads.sourceforge.net/project/netcat/netcat/$(NETCAT_VE
 NETCAT_LICENSE = GPL-2.0+
 NETCAT_LICENSE_FILES = COPYING
 
+# Ensure Busybox gets built/installed before, so that this package
+# overrides Busybox nc. We must remove an existing file/link because
+# netcat is as shy as Busybox and would not override existing files.
+ifeq ($(BR2_PACKAGE_BUSYBOX),y)
+NETCAT_DEPENDENCIES += busybox
+define NETCAT_RMOVE_NC_LINK
+	rm -f $(TARGET_DIR)/usr/bin/nc
+endef
+NETCAT_PRE_INSTALL_TARGET_HOOKS += NETCAT_RMOVE_NC_LINK
+endif
+
 $(eval $(autotools-package))