[Buildroot] [PATCH 1/4] package/libnss: fix build failure with ARM without NEON extension

[Buildroot] [PATCH 1/4] package/libnss: fix build failure with ARM without NEON extension

[Giulio Benetti]

At the moment libnss assumes that every ARM has NEON extension but it's
not that way. So add a patch to make it aware of it and use native
functions in place of NEON optimized ones.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
Already sent upstream:
https://bugzilla.mozilla.org/show_bug.cgi?id=1590676
---
 ...ix-build-if-arm-doesn-t-support-NEON.patch | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch

diff --git a/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
new file mode 100644
index 0000000000..e536282371
--- /dev/null
+++ b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
@@ -0,0 +1,47 @@
+From 946bc2864b0de7b63a4ec415e42e622d591a8753 Mon Sep 17 00:00:00 2001
+From: Giulio Benetti <giulio.benetti@benettiengineering.com>
+Date: Wed, 23 Oct 2019 11:47:03 +0200
+Subject: [PATCH] Bug 1590676 - Fix build if arm doesn't support NEON
+
+At the moment NSS assumes that ARM supports NEON extension but this is
+not true and leads to build failure on ARM without NEON extension.
+Add check to assure USE_HW_AES is not defined if ARM without NEON
+extension is used.
+
+Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
+---
+ nss/lib/freebl/aes-armv8.c | 3 ++-
+ nss/lib/freebl/rijndael.c  | 3 ++-
+ 2 files changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/nss/lib/freebl/aes-armv8.c b/nss/lib/freebl/aes-armv8.c
+index 40d5e2d34..a5df53891 100644
+--- a/nss/lib/freebl/aes-armv8.c
++++ b/nss/lib/freebl/aes-armv8.c
+@@ -7,7 +7,8 @@
+ 
+ #if (defined(__clang__) ||                            \
+      (defined(__GNUC__) && defined(__GNUC_MINOR__) && \
+-      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))))
++      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))) && \
++      (defined(__ARM_NEON) || defined(__ARM_NEON__)))
+ 
+ #ifndef __ARM_FEATURE_CRYPTO
+ #error "Compiler option is invalid"
+diff --git a/nss/lib/freebl/rijndael.c b/nss/lib/freebl/rijndael.c
+index 26bd58ee0..23893f419 100644
+--- a/nss/lib/freebl/rijndael.c
++++ b/nss/lib/freebl/rijndael.c
+@@ -20,7 +20,8 @@
+ #include "gcm.h"
+ #include "mpi.h"
+ 
+-#if !defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)
++#if (!defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)) || \
++    (!defined(__ARM_NEON) && !defined(__ARM_NEON__))
+ // not test yet on big endian platform of arm
+ #undef USE_HW_AES
+ #endif
+-- 
+2.20.1
+
-- 
2.20.1

[Buildroot] [PATCH 2/4] package/libnss: fix build warning

[Giulio Benetti]

Add patch to fix build warning due to uninitialized variable.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
https://bugzilla.mozilla.org/show_bug.cgi?id=1590678
---
 ...ve-Wmaybe-uninitialized-warning-in-t.patch | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch

diff --git a/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
new file mode 100644
index 0000000000..956abeb4a5
--- /dev/null
+++ b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
@@ -0,0 +1,27 @@
+From 24bcc8860310149da1524dbf25f3bc77f6476b11 Mon Sep 17 00:00:00 2001
+From: Giulio Benetti <giulio.benetti@benettiengineering.com>
+Date: Wed, 23 Oct 2019 11:58:12 +0200
+Subject: [PATCH] Bug 1590678 - Remove -Wmaybe-uninitialized warning in
+ tls13esni.c
+
+Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
+---
+ nss/lib/ssl/tls13esni.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/nss/lib/ssl/tls13esni.c b/nss/lib/ssl/tls13esni.c
+index 4d2e12d62..a7ce6f568 100644
+--- a/nss/lib/ssl/tls13esni.c
++++ b/nss/lib/ssl/tls13esni.c
+@@ -728,7 +728,7 @@ tls13_ServerDecryptEsniXtn(const sslSocket *ss, const PRUint8 *in, unsigned int
+ {
+     sslReader rdr = SSL_READER(in, inLen);
+     PRUint64 suite;
+-    const ssl3CipherSuiteDef *suiteDef;
++    const ssl3CipherSuiteDef *suiteDef = NULL;
+     SSLAEADCipher aead = NULL;
+     TLSExtension *keyShareExtension;
+     TLS13KeyShareEntry *entry = NULL;
+-- 
+2.20.1
+
-- 
2.20.1

[Buildroot] [PATCH 3/4] package/libnss: remove useless NSS_ENABLE_ECC variable

[Giulio Benetti]

NSS_ENABLE_ECC is not supported anymore, then remove it from our
libnss.mk file.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 package/libnss/libnss.mk | 2 --
 1 file changed, 2 deletions(-)

diff --git a/package/libnss/libnss.mk b/package/libnss/libnss.mk
index 472936db89..ca1cde4c1c 100644
--- a/package/libnss/libnss.mk
+++ b/package/libnss/libnss.mk
@@ -50,7 +50,6 @@ LIBNSS_BUILD_VARS = \
 	NS_USE_GCC=1 \
 	NSS_DISABLE_GTESTS=1 \
 	NSS_USE_SYSTEM_SQLITE=1 \
-	NSS_ENABLE_ECC=1 \
 	NATIVE_CC="$(HOSTCC)" \
 	OS_ARCH="Linux" \
 	OS_RELEASE="2.6" \
@@ -121,7 +120,6 @@ HOST_LIBNSS_BUILD_VARS = \
 	NSS_USE_SYSTEM_SQLITE=1 \
 	SQLITE_INCLUDE_DIR=$(HOST_DIR)/include \
 	ZLIB_INCLUDE_DIR=$(HOST_DIR)/include \
-	NSS_ENABLE_ECC=1 \
 	NSS_ENABLE_WERROR=0
 
 HOST_LIBNSS_DEPENDENCIES = host-libnspr host-sqlite host-zlib
-- 
2.20.1

[Buildroot] [PATCH 4/4] package/libnss: remove NSS_ENABLE_WERROR=0

[Giulio Benetti]

All warnings have been fixed now, so let's remove NSS_ENABLE_WERROR and
leave any warning to show up.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 package/libnss/libnss.mk | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/package/libnss/libnss.mk b/package/libnss/libnss.mk
index ca1cde4c1c..e1df7bbf33 100644
--- a/package/libnss/libnss.mk
+++ b/package/libnss/libnss.mk
@@ -53,8 +53,7 @@ LIBNSS_BUILD_VARS = \
 	NATIVE_CC="$(HOSTCC)" \
 	OS_ARCH="Linux" \
 	OS_RELEASE="2.6" \
-	OS_TEST="$(LIBNSS_ARCH)" \
-	NSS_ENABLE_WERROR=0
+	OS_TEST="$(LIBNSS_ARCH)"
 
 # #pragma usage needs gcc >= 4.8
 # See https://bugzilla.mozilla.org/show_bug.cgi?id=1226179
@@ -119,8 +118,7 @@ HOST_LIBNSS_BUILD_VARS = \
 	NSS_DISABLE_GTESTS=1 \
 	NSS_USE_SYSTEM_SQLITE=1 \
 	SQLITE_INCLUDE_DIR=$(HOST_DIR)/include \
-	ZLIB_INCLUDE_DIR=$(HOST_DIR)/include \
-	NSS_ENABLE_WERROR=0
+	ZLIB_INCLUDE_DIR=$(HOST_DIR)/include
 
 HOST_LIBNSS_DEPENDENCIES = host-libnspr host-sqlite host-zlib
 
-- 
2.20.1

[Buildroot] [PATCH 4/4] package/libnss: remove NSS_ENABLE_WERROR=0

[Thomas Petazzoni]

On Wed, 23 Oct 2019 12:23:30 +0200
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> All warnings have been fixed now, so let's remove NSS_ENABLE_WERROR and
> leave any warning to show up.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

No, we generally don't want to enable -Werror in Buildroot builds, as
warnings keep popping up when upgrading gcc versions.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 4/4] package/libnss: remove NSS_ENABLE_WERROR=0

[Giulio Benetti]

Hello Thomas,

On 10/23/19 5:54 PM, Thomas Petazzoni wrote:
> On Wed, 23 Oct 2019 12:23:30 +0200
> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> 
>> All warnings have been fixed now, so let's remove NSS_ENABLE_WERROR and
>> leave any warning to show up.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> 
> No, we generally don't want to enable -Werror in Buildroot builds, as
> warnings keep popping up when upgrading gcc versions.

Ok, then I'll fix warnings upstream and keep -Werror disabled.

Thanks for reviewing
Best regards
-- 
Giulio Benetti
Benetti Engineering sas

[Buildroot] [PATCH 1/4] package/libnss: fix build failure with ARM without NEON extension

[Arnout Vandecappelle]

On 23/10/2019 12:23, Giulio Benetti wrote:
> At the moment libnss assumes that every ARM has NEON extension but it's
> not that way. So add a patch to make it aware of it and use native
> functions in place of NEON optimized ones.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

 You forgot:

Fixes:
http://autobuild.buildroot.net/results/1342d305d1aeebef7af54a83afc094fda12421e2

> ---
> Already sent upstream:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1590676
> ---
>  ...ix-build-if-arm-doesn-t-support-NEON.patch | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
> 
> diff --git a/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
> new file mode 100644
> index 0000000000..e536282371
> --- /dev/null
> +++ b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
> @@ -0,0 +1,47 @@
> +From 946bc2864b0de7b63a4ec415e42e622d591a8753 Mon Sep 17 00:00:00 2001
> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +Date: Wed, 23 Oct 2019 11:47:03 +0200
> +Subject: [PATCH] Bug 1590676 - Fix build if arm doesn't support NEON
> +
> +At the moment NSS assumes that ARM supports NEON extension but this is
> +not true and leads to build failure on ARM without NEON extension.
> +Add check to assure USE_HW_AES is not defined if ARM without NEON
> +extension is used.
> +
> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +---
> + nss/lib/freebl/aes-armv8.c | 3 ++-
> + nss/lib/freebl/rijndael.c  | 3 ++-
> + 2 files changed, 4 insertions(+), 2 deletions(-)
> +
> +diff --git a/nss/lib/freebl/aes-armv8.c b/nss/lib/freebl/aes-armv8.c
> +index 40d5e2d34..a5df53891 100644
> +--- a/nss/lib/freebl/aes-armv8.c
> ++++ b/nss/lib/freebl/aes-armv8.c

 That looks as if it is something that should be built only on armv8... Since
the build failure is in fact for an armv4, don't we have a bigger issue here?

 I believe 32-bit armv8 always has neon.

> +@@ -7,7 +7,8 @@
> + 
> + #if (defined(__clang__) ||                            \
> +      (defined(__GNUC__) && defined(__GNUC_MINOR__) && \
> +-      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))))
> ++      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))) && \
> ++      (defined(__ARM_NEON) || defined(__ARM_NEON__)))

 That parenthesis matching doesn't look right... I suppose with clang it doesn't
magically support neon, right? So there should be an additional set of
parenthesis around the defined(__clang__) ... > 8 part.


 Regards,
 Arnout

> + 
> + #ifndef __ARM_FEATURE_CRYPTO
> + #error "Compiler option is invalid"
> +diff --git a/nss/lib/freebl/rijndael.c b/nss/lib/freebl/rijndael.c
> +index 26bd58ee0..23893f419 100644
> +--- a/nss/lib/freebl/rijndael.c
> ++++ b/nss/lib/freebl/rijndael.c
> +@@ -20,7 +20,8 @@
> + #include "gcm.h"
> + #include "mpi.h"
> + 
> +-#if !defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)
> ++#if (!defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)) || \
> ++    (!defined(__ARM_NEON) && !defined(__ARM_NEON__))
> + // not test yet on big endian platform of arm
> + #undef USE_HW_AES
> + #endif
> +-- 
> +2.20.1
> +
> 

[Buildroot] [PATCH 2/4] package/libnss: fix build warning

[Arnout Vandecappelle]

On 23/10/2019 12:23, Giulio Benetti wrote:
> Add patch to fix build warning due to uninitialized variable.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
> https://bugzilla.mozilla.org/show_bug.cgi?id=1590678
> ---
>  ...ve-Wmaybe-uninitialized-warning-in-t.patch | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
> 
> diff --git a/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
> new file mode 100644
> index 0000000000..956abeb4a5
> --- /dev/null
> +++ b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
> @@ -0,0 +1,27 @@
> +From 24bcc8860310149da1524dbf25f3bc77f6476b11 Mon Sep 17 00:00:00 2001
> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +Date: Wed, 23 Oct 2019 11:58:12 +0200
> +Subject: [PATCH] Bug 1590678 - Remove -Wmaybe-uninitialized warning in
> + tls13esni.c
> +
> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +---
> + nss/lib/ssl/tls13esni.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/nss/lib/ssl/tls13esni.c b/nss/lib/ssl/tls13esni.c
> +index 4d2e12d62..a7ce6f568 100644
> +--- a/nss/lib/ssl/tls13esni.c
> ++++ b/nss/lib/ssl/tls13esni.c
> +@@ -728,7 +728,7 @@ tls13_ServerDecryptEsniXtn(const sslSocket *ss, const PRUint8 *in, unsigned int
> + {
> +     sslReader rdr = SSL_READER(in, inLen);
> +     PRUint64 suite;
> +-    const ssl3CipherSuiteDef *suiteDef;
> ++    const ssl3CipherSuiteDef *suiteDef = NULL;

 Although this looks OK at first sight, it's something that might be dangerous
(i.e. the wrong fix). Since this is a security package, I don't want to apply it
until upstream gives feedback.

 Regards,
 Arnout

> +     SSLAEADCipher aead = NULL;
> +     TLSExtension *keyShareExtension;
> +     TLS13KeyShareEntry *entry = NULL;
> +-- 
> +2.20.1
> +
> 

[Buildroot] [PATCH 3/4] package/libnss: remove useless NSS_ENABLE_ECC variable

[Arnout Vandecappelle]

On 23/10/2019 12:23, Giulio Benetti wrote:
> NSS_ENABLE_ECC is not supported anymore, then remove it from our
> libnss.mk file.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

 Applied to master, thanks.

 Regards,
 Arnout

> ---
>  package/libnss/libnss.mk | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/package/libnss/libnss.mk b/package/libnss/libnss.mk
> index 472936db89..ca1cde4c1c 100644
> --- a/package/libnss/libnss.mk
> +++ b/package/libnss/libnss.mk
> @@ -50,7 +50,6 @@ LIBNSS_BUILD_VARS = \
>  	NS_USE_GCC=1 \
>  	NSS_DISABLE_GTESTS=1 \
>  	NSS_USE_SYSTEM_SQLITE=1 \
> -	NSS_ENABLE_ECC=1 \
>  	NATIVE_CC="$(HOSTCC)" \
>  	OS_ARCH="Linux" \
>  	OS_RELEASE="2.6" \
> @@ -121,7 +120,6 @@ HOST_LIBNSS_BUILD_VARS = \
>  	NSS_USE_SYSTEM_SQLITE=1 \
>  	SQLITE_INCLUDE_DIR=$(HOST_DIR)/include \
>  	ZLIB_INCLUDE_DIR=$(HOST_DIR)/include \
> -	NSS_ENABLE_ECC=1 \
>  	NSS_ENABLE_WERROR=0
>  
>  HOST_LIBNSS_DEPENDENCIES = host-libnspr host-sqlite host-zlib
> 

[Buildroot] [PATCH 2/4] package/libnss: fix build warning

[Arnout Vandecappelle]

On 24/10/2019 10:47, Arnout Vandecappelle wrote:
> 
> 
> On 23/10/2019 12:23, Giulio Benetti wrote:
>> Add patch to fix build warning due to uninitialized variable.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> ---
>> https://bugzilla.mozilla.org/show_bug.cgi?id=1590678
>> ---
>>  ...ve-Wmaybe-uninitialized-warning-in-t.patch | 27 +++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>  create mode 100644 package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>
>> diff --git a/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>> new file mode 100644
>> index 0000000000..956abeb4a5
>> --- /dev/null
>> +++ b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>> @@ -0,0 +1,27 @@
>> +From 24bcc8860310149da1524dbf25f3bc77f6476b11 Mon Sep 17 00:00:00 2001
>> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> +Date: Wed, 23 Oct 2019 11:58:12 +0200
>> +Subject: [PATCH] Bug 1590678 - Remove -Wmaybe-uninitialized warning in
>> + tls13esni.c
>> +
>> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> +---
>> + nss/lib/ssl/tls13esni.c | 2 +-
>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>> +
>> +diff --git a/nss/lib/ssl/tls13esni.c b/nss/lib/ssl/tls13esni.c
>> +index 4d2e12d62..a7ce6f568 100644
>> +--- a/nss/lib/ssl/tls13esni.c
>> ++++ b/nss/lib/ssl/tls13esni.c
>> +@@ -728,7 +728,7 @@ tls13_ServerDecryptEsniXtn(const sslSocket *ss, const PRUint8 *in, unsigned int
>> + {
>> +     sslReader rdr = SSL_READER(in, inLen);
>> +     PRUint64 suite;
>> +-    const ssl3CipherSuiteDef *suiteDef;
>> ++    const ssl3CipherSuiteDef *suiteDef = NULL;
> 
>  Although this looks OK at first sight, it's something that might be dangerous
> (i.e. the wrong fix). Since this is a security package, I don't want to apply it
> until upstream gives feedback.

 On second thought, since we don't enable -Werror in Buildroot, we don't need
this patch AFAICS. So it's good that you reported it upstream, but we're not
going to apply this patch. I've marked it as Rejected.

 Regards,
 Arnout

[Buildroot] [PATCH 2/4] package/libnss: fix build warning

[Arnout Vandecappelle]

 One more :-)

On 24/10/2019 10:50, Arnout Vandecappelle wrote:
> 
> 
> On 24/10/2019 10:47, Arnout Vandecappelle wrote:
>>
>>
>> On 23/10/2019 12:23, Giulio Benetti wrote:
>>> Add patch to fix build warning due to uninitialized variable.
>>>
>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>> ---
>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1590678
>>> ---
>>>  ...ve-Wmaybe-uninitialized-warning-in-t.patch | 27 +++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>>  create mode 100644 package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>>
>>> diff --git a/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>> new file mode 100644
>>> index 0000000000..956abeb4a5
>>> --- /dev/null
>>> +++ b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>> @@ -0,0 +1,27 @@
>>> +From 24bcc8860310149da1524dbf25f3bc77f6476b11 Mon Sep 17 00:00:00 2001
>>> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>> +Date: Wed, 23 Oct 2019 11:58:12 +0200
>>> +Subject: [PATCH] Bug 1590678 - Remove -Wmaybe-uninitialized warning in
>>> + tls13esni.c
>>> +
>>> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>> +---
>>> + nss/lib/ssl/tls13esni.c | 2 +-
>>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>>> +
>>> +diff --git a/nss/lib/ssl/tls13esni.c b/nss/lib/ssl/tls13esni.c
>>> +index 4d2e12d62..a7ce6f568 100644
>>> +--- a/nss/lib/ssl/tls13esni.c
>>> ++++ b/nss/lib/ssl/tls13esni.c
>>> +@@ -728,7 +728,7 @@ tls13_ServerDecryptEsniXtn(const sslSocket *ss, const PRUint8 *in, unsigned int
>>> + {
>>> +     sslReader rdr = SSL_READER(in, inLen);
>>> +     PRUint64 suite;
>>> +-    const ssl3CipherSuiteDef *suiteDef;
>>> ++    const ssl3CipherSuiteDef *suiteDef = NULL;
>>
>>  Although this looks OK at first sight, it's something that might be dangerous
>> (i.e. the wrong fix). Since this is a security package, I don't want to apply it
>> until upstream gives feedback.
> 
>  On second thought, since we don't enable -Werror in Buildroot, we don't need
> this patch AFAICS. So it's good that you reported it upstream, but we're not
> going to apply this patch. I've marked it as Rejected.

 If upstream decides that this really *is* an uninitialized variable reference
and not just a limitation of the compiler's analysis, it's a potential security
issue, so in that case please resubmit.

 Regards,
 Arnout

[Buildroot] [PATCH 2/4] package/libnss: fix build warning

[Giulio Benetti]

Hi Arnout,

On 10/24/19 10:51 AM, Arnout Vandecappelle wrote:
>   One more :-)
> 
> On 24/10/2019 10:50, Arnout Vandecappelle wrote:
>>
>>
>> On 24/10/2019 10:47, Arnout Vandecappelle wrote:
>>>
>>>
>>> On 23/10/2019 12:23, Giulio Benetti wrote:
>>>> Add patch to fix build warning due to uninitialized variable.
>>>>
>>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>>> ---
>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1590678
>>>> ---
>>>>   ...ve-Wmaybe-uninitialized-warning-in-t.patch | 27 +++++++++++++++++++
>>>>   1 file changed, 27 insertions(+)
>>>>   create mode 100644 package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>>>
>>>> diff --git a/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>>> new file mode 100644
>>>> index 0000000000..956abeb4a5
>>>> --- /dev/null
>>>> +++ b/package/libnss/0004-Bug-1590678-Remove-Wmaybe-uninitialized-warning-in-t.patch
>>>> @@ -0,0 +1,27 @@
>>>> +From 24bcc8860310149da1524dbf25f3bc77f6476b11 Mon Sep 17 00:00:00 2001
>>>> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>>> +Date: Wed, 23 Oct 2019 11:58:12 +0200
>>>> +Subject: [PATCH] Bug 1590678 - Remove -Wmaybe-uninitialized warning in
>>>> + tls13esni.c
>>>> +
>>>> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>>> +---
>>>> + nss/lib/ssl/tls13esni.c | 2 +-
>>>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> +
>>>> +diff --git a/nss/lib/ssl/tls13esni.c b/nss/lib/ssl/tls13esni.c
>>>> +index 4d2e12d62..a7ce6f568 100644
>>>> +--- a/nss/lib/ssl/tls13esni.c
>>>> ++++ b/nss/lib/ssl/tls13esni.c
>>>> +@@ -728,7 +728,7 @@ tls13_ServerDecryptEsniXtn(const sslSocket *ss, const PRUint8 *in, unsigned int
>>>> + {
>>>> +     sslReader rdr = SSL_READER(in, inLen);
>>>> +     PRUint64 suite;
>>>> +-    const ssl3CipherSuiteDef *suiteDef;
>>>> ++    const ssl3CipherSuiteDef *suiteDef = NULL;
>>>
>>>   Although this looks OK at first sight, it's something that might be dangerous
>>> (i.e. the wrong fix). Since this is a security package, I don't want to apply it
>>> until upstream gives feedback.
>>
>>   On second thought, since we don't enable -Werror in Buildroot, we don't need
>> this patch AFAICS. So it's good that you reported it upstream, but we're not
>> going to apply this patch. I've marked it as Rejected.
> 
>   If upstream decides that this really *is* an uninitialized variable reference
> and not just a limitation of the compiler's analysis, it's a potential security
> issue, so in that case please resubmit.

Ok then,

thanks for reviewing!
Best regards
-- 
Giulio Benetti
Benetti Engineering sas

[Buildroot] [PATCH 1/4] package/libnss: fix build failure with ARM without NEON extension

[Giulio Benetti]

Hi Arnout,

On 10/23/19 11:44 PM, Arnout Vandecappelle wrote:
> 
> 
> On 23/10/2019 12:23, Giulio Benetti wrote:
>> At the moment libnss assumes that every ARM has NEON extension but it's
>> not that way. So add a patch to make it aware of it and use native
>> functions in place of NEON optimized ones.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> 
>   You forgot:
> 
> Fixes:
> http://autobuild.buildroot.net/results/1342d305d1aeebef7af54a83afc094fda12421e2

Oops, yes.

>> ---
>> Already sent upstream:
>> https://bugzilla.mozilla.org/show_bug.cgi?id=1590676
>> ---
>>   ...ix-build-if-arm-doesn-t-support-NEON.patch | 47 +++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>   create mode 100644 package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
>>
>> diff --git a/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
>> new file mode 100644
>> index 0000000000..e536282371
>> --- /dev/null
>> +++ b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
>> @@ -0,0 +1,47 @@
>> +From 946bc2864b0de7b63a4ec415e42e622d591a8753 Mon Sep 17 00:00:00 2001
>> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> +Date: Wed, 23 Oct 2019 11:47:03 +0200
>> +Subject: [PATCH] Bug 1590676 - Fix build if arm doesn't support NEON
>> +
>> +At the moment NSS assumes that ARM supports NEON extension but this is
>> +not true and leads to build failure on ARM without NEON extension.
>> +Add check to assure USE_HW_AES is not defined if ARM without NEON
>> +extension is used.
>> +
>> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> +---
>> + nss/lib/freebl/aes-armv8.c | 3 ++-
>> + nss/lib/freebl/rijndael.c  | 3 ++-
>> + 2 files changed, 4 insertions(+), 2 deletions(-)
>> +
>> +diff --git a/nss/lib/freebl/aes-armv8.c b/nss/lib/freebl/aes-armv8.c
>> +index 40d5e2d34..a5df53891 100644
>> +--- a/nss/lib/freebl/aes-armv8.c
>> ++++ b/nss/lib/freebl/aes-armv8.c
> 
>   That looks as if it is something that should be built only on armv8... Since
> the build failure is in fact for an armv4, don't we have a bigger issue here?
> 
>   I believe 32-bit armv8 always has neon.

Would it be a good idea to provide a variable to enable or not building 
aes-armv8 and define or not USE_HW_AES? I mean, this is a Makefile and I 
don't know how to make libnss capable of understand if there is NEON 
support or not, do you have any suggestions?

>> +@@ -7,7 +7,8 @@
>> +
>> + #if (defined(__clang__) ||                            \
>> +      (defined(__GNUC__) && defined(__GNUC_MINOR__) && \
>> +-      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))))
>> ++      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))) && \
>> ++      (defined(__ARM_NEON) || defined(__ARM_NEON__)))
> 
>   That parenthesis matching doesn't look right... I suppose with clang it doesn't
> magically support neon, right? So there should be an additional set of
> parenthesis around the defined(__clang__) ... > 8 part.

Oops again, you're right.

Thanks for reviewing
Best regards
-- 
Giulio Benetti
Benetti Engineering sas

[Buildroot] [PATCH v2] package/libnss: fix build failure with ARM without NEON extension

[Giulio Benetti]

At the moment libnss assumes that every ARM has NEON extension but it's
not that way. So add a patch to make it aware of it and use native
functions in place of NEON optimized ones.

Fixes:
http://autobuild.buildroot.net/results/1342d305d1aeebef7af54a83afc094fda12421e2/

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
V1->V2:
* added Fixes to commit log
* fixed #ifdef statement in patch
---
 ...ix-build-if-arm-doesn-t-support-NEON.patch | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch

diff --git a/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
new file mode 100644
index 0000000000..1d7f63611e
--- /dev/null
+++ b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
@@ -0,0 +1,50 @@
+From 76f35f9acd6c6b34318438bef30b0bdfdfd13f16 Mon Sep 17 00:00:00 2001
+From: Giulio Benetti <giulio.benetti@benettiengineering.com>
+Date: Wed, 23 Oct 2019 11:47:03 +0200
+Subject: [PATCH] Bug 1590676 - Fix build if arm doesn't support NEON
+
+At the moment NSS assumes that ARM supports NEON extension but this is
+not true and leads to build failure on ARM without NEON extension.
+Add check to assure USE_HW_AES is not defined if ARM without NEON
+extension is used.
+
+Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
+---
+ nss/lib/freebl/aes-armv8.c | 5 +++--
+ nss/lib/freebl/rijndael.c  | 3 ++-
+ 2 files changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/nss/lib/freebl/aes-armv8.c b/nss/lib/freebl/aes-armv8.c
+index 40d5e2d34..d1504e6fb 100644
+--- a/nss/lib/freebl/aes-armv8.c
++++ b/nss/lib/freebl/aes-armv8.c
+@@ -5,9 +5,10 @@
+ #include "secerr.h"
+ #include "rijndael.h"
+ 
+-#if (defined(__clang__) ||                            \
++#if ((defined(__clang__) ||                            \
+      (defined(__GNUC__) && defined(__GNUC_MINOR__) && \
+-      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))))
++      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8)))) && \
++      (defined(__ARM_NEON) || defined(__ARM_NEON__)))
+ 
+ #ifndef __ARM_FEATURE_CRYPTO
+ #error "Compiler option is invalid"
+diff --git a/nss/lib/freebl/rijndael.c b/nss/lib/freebl/rijndael.c
+index 26bd58ee0..23893f419 100644
+--- a/nss/lib/freebl/rijndael.c
++++ b/nss/lib/freebl/rijndael.c
+@@ -20,7 +20,8 @@
+ #include "gcm.h"
+ #include "mpi.h"
+ 
+-#if !defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)
++#if (!defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)) || \
++    (!defined(__ARM_NEON) && !defined(__ARM_NEON__))
+ // not test yet on big endian platform of arm
+ #undef USE_HW_AES
+ #endif
+-- 
+2.20.1
+
-- 
2.20.1

[Buildroot] [PATCH v2] package/libnss: fix build failure with ARM without NEON extension

[Giulio Benetti]

On 10/28/19 5:55 PM, Giulio Benetti wrote:
> At the moment libnss assumes that every ARM has NEON extension but it's
> not that way. So add a patch to make it aware of it and use native
> functions in place of NEON optimized ones.
> 
> Fixes:
> http://autobuild.buildroot.net/results/1342d305d1aeebef7af54a83afc094fda12421e2/
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
> V1->V2:
> * added Fixes to commit log
> * fixed #ifdef statement in patch
> ---
>   ...ix-build-if-arm-doesn-t-support-NEON.patch | 50 +++++++++++++++++++
>   1 file changed, 50 insertions(+)
>   create mode 100644 package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
> 
> diff --git a/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
> new file mode 100644
> index 0000000000..1d7f63611e
> --- /dev/null
> +++ b/package/libnss/0003-Bug-1590676-Fix-build-if-arm-doesn-t-support-NEON.patch
> @@ -0,0 +1,50 @@
> +From 76f35f9acd6c6b34318438bef30b0bdfdfd13f16 Mon Sep 17 00:00:00 2001
> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +Date: Wed, 23 Oct 2019 11:47:03 +0200
> +Subject: [PATCH] Bug 1590676 - Fix build if arm doesn't support NEON
> +
> +At the moment NSS assumes that ARM supports NEON extension but this is
> +not true and leads to build failure on ARM without NEON extension.
> +Add check to assure USE_HW_AES is not defined if ARM without NEON
> +extension is used.
> +
> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +---
> + nss/lib/freebl/aes-armv8.c | 5 +++--
> + nss/lib/freebl/rijndael.c  | 3 ++-
> + 2 files changed, 5 insertions(+), 3 deletions(-)
> +
> +diff --git a/nss/lib/freebl/aes-armv8.c b/nss/lib/freebl/aes-armv8.c
> +index 40d5e2d34..d1504e6fb 100644
> +--- a/nss/lib/freebl/aes-armv8.c
> ++++ b/nss/lib/freebl/aes-armv8.c
> +@@ -5,9 +5,10 @@
> + #include "secerr.h"
> + #include "rijndael.h"
> +
> +-#if (defined(__clang__) ||                            \
> ++#if ((defined(__clang__) ||                            \
> +      (defined(__GNUC__) && defined(__GNUC_MINOR__) && \
> +-      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8))))
> ++      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8)))) && \
> ++      (defined(__ARM_NEON) || defined(__ARM_NEON__)))
> +
> + #ifndef __ARM_FEATURE_CRYPTO
> + #error "Compiler option is invalid"
> +diff --git a/nss/lib/freebl/rijndael.c b/nss/lib/freebl/rijndael.c
> +index 26bd58ee0..23893f419 100644
> +--- a/nss/lib/freebl/rijndael.c
> ++++ b/nss/lib/freebl/rijndael.c
> +@@ -20,7 +20,8 @@
> + #include "gcm.h"
> + #include "mpi.h"
> +
> +-#if !defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)
> ++#if (!defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)) || \
> ++    (!defined(__ARM_NEON) && !defined(__ARM_NEON__))

This part is wrong, I've sent upstream corrected patch[1] and I'm going 
to send v3 patch for buildroot.

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1590676

> + // not test yet on big endian platform of arm
> + #undef USE_HW_AES
> + #endif
> +--
> +2.20.1
> +
> 

-- 
Giulio Benetti
Benetti Engineering sas