diff mbox series

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

Message ID 20191023102330.20842-2-giulio.benetti@benettiengineering.com
State Rejected
Headers show
Series None | expand

Commit Message

Giulio Benetti Oct. 23, 2019, 10:23 a.m. UTC 
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

Comments

Arnout Vandecappelle Oct. 24, 2019, 8:47 a.m. UTC | #1 
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
> +
>
Arnout Vandecappelle Oct. 24, 2019, 8:50 a.m. UTC | #2 
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
Arnout Vandecappelle Oct. 24, 2019, 8:51 a.m. UTC | #3 
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
Giulio Benetti Oct. 24, 2019, 10:49 a.m. UTC | #4 
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
diff mbox series

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
+