diff mbox series

[LEDE-DEV,4/4] toolchain: musl: update to current HEAD

Message ID 8eaac4e0afd2f71517b019d5859482e972880278.1511108316.git.chunkeey@gmail.com
State Accepted
Headers show
Series [LEDE-DEV,1/4] firmware: ath10k-firmware: update QCA4019 firmware to 10.4-3.2.1-00058 | expand

Commit Message

Christian Lamparter Nov. 19, 2017, 4:19 p.m. UTC
Changes:

72656157 fix fgetwc when decoding a character that crosses buffer boundary
a223dbd2 add reverse iconv mappings for JIS-based encodings
105eff9d generalize iconv framework for 8-bit codepages
a71b46cf fix malloc state corruption when ldso rejects loading a second libc
d060edf6 reformat cjk iconv tables to be diff-friendly, match tool output
c21051e9 prevent fork's errno from being clobbered by atfork handlers
a39f20bf add iso-2022-jp support (decoding only) to iconv
5b546faa add iconv framework for decoding stateful encodings
0df5b39a simplify/optimize iconv utf-8 case
9eb6dd51 handle ascii range individually in each iconv case
bff59d13 move iconv_close to its own translation unit
79f49eff refactor iconv conversion descriptor encoding/decoding
30fdda6c fix getaddrinfo error code for non-numeric service with AI_NUMERICSERV
67b29947 fix mismatched type of __pthread_tsd_run_dtors weak definition
13935337 s390x: use generic ioctl.h
4dc44ce8 microblaze: add statx syscall from linux v4.13
ffd048a0 aarch64: add extra_context struct from linux v4.13
6651ef1f add new tcp.h socket options from linux v4.13
14ced228 add new fcntl.h macros from linux v4.13
754f66af ioctl TIOCGPTPEER from linux v4.13
c35a8bf4 add SO_ getsockopt options from linux v4.13
5daaed6a s390x: add syscall number for s390_guarded_storage from linux v4.12
2dc6760f i386: add arch_prctl syscall number from linux v4.12
840d45be aarch64: add new HWCAP_* flags from linux v4.12
4c811227 add ARPHDR_VSOCKMON from linux v4.12
54f04d99 add new SO_ socket options from linux v4.12
9864f60e add statx syscall numbers from linux v4.11
c519658c add TCP_NLA_* enums from linux v4.11
ee3ae782 add TCP_FASTOPEN_CONNECT tcp socket option from linux v4.11
3eb82f73 add ETH_P_IBOE from linux v4.11
bd1560f6 update aarch64 hwcap.h for linux v4.11
cee73f0c add kexec_file_load syscall number on powerpc from linux v4.10
8f569557 add microblaze syscall numbers from linux v4.10
d8004030 add TFD_TIMER_CANCEL_ON_SET that timerfd.h was missing
f5638c22 add ETH_MIN_MTU and ETH_MAX_MTU from linux v4.10
01369691 add IP_RECVFRAGSIZE and IPV6_RECVFRAGSIZE from linux v4.10
5c596ed8 add SCM_TIMESTAMPING_OPT_STATS and related TCP_ enums from linux v4.10
6fc6ca1a adjust posix_spawn dup2 action behavior to match future requirements

Cc: Syrone Wong <wong.syrone@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 toolchain/musl/common.mk                         |  4 +-
 toolchain/musl/patches/900-iconv_size_hack.patch | 70 +++++++++++++++++-------
 2 files changed, 53 insertions(+), 21 deletions(-)

Comments

Syrone Wong Nov. 20, 2017, 12:17 a.m. UTC | #1
Hi,

Any specific reason to introduce these changes?


Best Regards,
Syrone Wong


On Mon, Nov 20, 2017 at 12:19 AM, Christian Lamparter
<chunkeey@gmail.com> wrote:
> Changes:
>
> 72656157 fix fgetwc when decoding a character that crosses buffer boundary
> a223dbd2 add reverse iconv mappings for JIS-based encodings
> 105eff9d generalize iconv framework for 8-bit codepages
> a71b46cf fix malloc state corruption when ldso rejects loading a second libc
> d060edf6 reformat cjk iconv tables to be diff-friendly, match tool output
> c21051e9 prevent fork's errno from being clobbered by atfork handlers
> a39f20bf add iso-2022-jp support (decoding only) to iconv
> 5b546faa add iconv framework for decoding stateful encodings
> 0df5b39a simplify/optimize iconv utf-8 case
> 9eb6dd51 handle ascii range individually in each iconv case
> bff59d13 move iconv_close to its own translation unit
> 79f49eff refactor iconv conversion descriptor encoding/decoding
> 30fdda6c fix getaddrinfo error code for non-numeric service with AI_NUMERICSERV
> 67b29947 fix mismatched type of __pthread_tsd_run_dtors weak definition
> 13935337 s390x: use generic ioctl.h
> 4dc44ce8 microblaze: add statx syscall from linux v4.13
> ffd048a0 aarch64: add extra_context struct from linux v4.13
> 6651ef1f add new tcp.h socket options from linux v4.13
> 14ced228 add new fcntl.h macros from linux v4.13
> 754f66af ioctl TIOCGPTPEER from linux v4.13
> c35a8bf4 add SO_ getsockopt options from linux v4.13
> 5daaed6a s390x: add syscall number for s390_guarded_storage from linux v4.12
> 2dc6760f i386: add arch_prctl syscall number from linux v4.12
> 840d45be aarch64: add new HWCAP_* flags from linux v4.12
> 4c811227 add ARPHDR_VSOCKMON from linux v4.12
> 54f04d99 add new SO_ socket options from linux v4.12
> 9864f60e add statx syscall numbers from linux v4.11
> c519658c add TCP_NLA_* enums from linux v4.11
> ee3ae782 add TCP_FASTOPEN_CONNECT tcp socket option from linux v4.11
> 3eb82f73 add ETH_P_IBOE from linux v4.11
> bd1560f6 update aarch64 hwcap.h for linux v4.11
> cee73f0c add kexec_file_load syscall number on powerpc from linux v4.10
> 8f569557 add microblaze syscall numbers from linux v4.10
> d8004030 add TFD_TIMER_CANCEL_ON_SET that timerfd.h was missing
> f5638c22 add ETH_MIN_MTU and ETH_MAX_MTU from linux v4.10
> 01369691 add IP_RECVFRAGSIZE and IPV6_RECVFRAGSIZE from linux v4.10
> 5c596ed8 add SCM_TIMESTAMPING_OPT_STATS and related TCP_ enums from linux v4.10
> 6fc6ca1a adjust posix_spawn dup2 action behavior to match future requirements
>
> Cc: Syrone Wong <wong.syrone@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  toolchain/musl/common.mk                         |  4 +-
>  toolchain/musl/patches/900-iconv_size_hack.patch | 70 +++++++++++++++++-------
>  2 files changed, 53 insertions(+), 21 deletions(-)
>
> diff --git a/toolchain/musl/common.mk b/toolchain/musl/common.mk
> index 2a61516372..a94a475571 100644
> --- a/toolchain/musl/common.mk
> +++ b/toolchain/musl/common.mk
> @@ -13,8 +13,8 @@ PKG_RELEASE=1
>
>  PKG_SOURCE_PROTO:=git
>  PKG_SOURCE_SUBDIR:=$(PKG_NAME)-$(PKG_VERSION)
> -PKG_SOURCE_VERSION:=eb03bde2f24582874cb72b56c7811bf51da0c817
> -PKG_MIRROR_HASH:=150808458007eeb0b977059f36f88127d1a1e80ddb6ad1837b5a63efd2958e34
> +PKG_SOURCE_VERSION:=72656157f54c47277b01ec85a6ba7c4084fea6c8
> +PKG_MIRROR_HASH:=a3d857c23c94aa96a4ad5f442aaf236e5a189a717273c4e4faf425988d98cd32
>  PKG_SOURCE_URL:=git://git.musl-libc.org/musl
>  PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION)-$(PKG_SOURCE_VERSION).tar.xz
>
> diff --git a/toolchain/musl/patches/900-iconv_size_hack.patch b/toolchain/musl/patches/900-iconv_size_hack.patch
> index db18fceb17..41cff5b033 100644
> --- a/toolchain/musl/patches/900-iconv_size_hack.patch
> +++ b/toolchain/musl/patches/900-iconv_size_hack.patch
> @@ -1,14 +1,14 @@
>  --- a/src/locale/iconv.c
>  +++ b/src/locale/iconv.c
> -@@ -39,6 +39,7 @@ static const unsigned char charmaps[] =
> +@@ -42,6 +42,7 @@ static const unsigned char charmaps[] =
>   "ucs4\0ucs4be\0utf32\0utf32be\0\0\300"
>   "ucs4le\0utf32le\0\0\303"
>   "ascii\0usascii\0iso646\0iso646us\0\0\307"
>  +#ifdef FULL_ICONV
>   "eucjp\0\0\320"
>   "shiftjis\0sjis\0\0\321"
> - "gb18030\0\0\330"
> -@@ -46,6 +47,7 @@ static const unsigned char charmaps[] =
> + "iso2022jp\0\0\322"
> +@@ -50,6 +51,7 @@ static const unsigned char charmaps[] =
>   "gb2312\0\0\332"
>   "big5\0bigfive\0cp950\0big5hkscs\0\0\340"
>   "euckr\0ksc5601\0ksx1001\0cp949\0\0\350"
> @@ -16,7 +16,7 @@
>   #include "codepages.h"
>   ;
>
> -@@ -53,6 +55,7 @@ static const unsigned short legacy_chars
> +@@ -60,6 +62,7 @@ static const unsigned short legacy_chars
>   #include "legacychars.h"
>   };
>
> @@ -24,45 +24,77 @@
>   static const unsigned short jis0208[84][94] = {
>   #include "jis0208.h"
>   };
> -@@ -72,6 +75,7 @@ static const unsigned short hkscs[] = {
> +@@ -79,6 +82,7 @@ static const unsigned short hkscs[] = {
>   static const unsigned short ksc[93][94] = {
>   #include "ksc.h"
>   };
>  +#endif
>
> - static int fuzzycmp(const unsigned char *a, const unsigned char *b)
> + static const unsigned short rev_jis[] = {
> + #include "revjis.h"
> +@@ -196,6 +200,7 @@ static unsigned legacy_map(const unsigne
> +       return x < 256 ? x : legacy_chars[x-256];
> + }
> +
> ++#ifdef FULL_ICONV
> + static unsigned uni_to_jis(unsigned c)
> + {
> +       unsigned nel = sizeof rev_jis / sizeof *rev_jis;
> +@@ -214,6 +219,7 @@ static unsigned uni_to_jis(unsigned c)
> +               }
> +       }
> + }
> ++#endif
> +
> + size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restrict out, size_t *restrict outb)
>   {
> -@@ -224,6 +228,7 @@ size_t iconv(iconv_t cd0, char **restric
> +@@ -285,6 +291,7 @@ size_t iconv(iconv_t cd, char **restrict
>                                 c = ((c-0xd7c0)<<10) + (d-0xdc00);
>                         }
>                         break;
>  +#ifdef FULL_ICONV
>                 case SHIFT_JIS:
> +                       if (c < 128) break;
>                         if (c-0xa1 <= 0xdf-0xa1) {
> -                               c += 0xff61-0xa1;
> -@@ -370,6 +375,7 @@ size_t iconv(iconv_t cd0, char **restric
> +@@ -476,6 +483,7 @@ size_t iconv(iconv_t cd, char **restrict
>                         c = ksc[c][d];
>                         if (!c) goto ilseq;
>                         break;
>  +#endif
>                 default:
> -                       if (c < 128+type) break;
> +                       if (!c) break;
>                         c = legacy_map(map, c);
> +@@ -516,6 +524,7 @@ size_t iconv(iconv_t cd, char **restrict
> +                               }
> +                       }
> +                       goto subst;
> ++#ifdef FULL_ICONV
> +               case SHIFT_JIS:
> +                       if (c < 128) goto revout;
> +                       if (c == 0xa5) {
> +@@ -589,6 +598,7 @@ size_t iconv(iconv_t cd, char **restrict
> +                       *(*out)++ = 'B';
> +                       *outb -= 8;
> +                       break;
> ++#endif
> +               case UCS2BE:
> +               case UCS2LE:
> +               case UTF_16BE:
>  --- a/src/locale/codepages.h
>  +++ b/src/locale/codepages.h
> -@@ -118,6 +118,7 @@
> - "\0\0\0\100\15\0\344\0\0\0\0\0\0\0\0\0\0\0\0\0\103\270\1\0\0\0\340\1\200\40"
> - "\230\0\0\0\0\0\44\341\12\0"
> +@@ -129,6 +129,7 @@
> + "\340\204\43\316\100\344\34\144\316\71\350\244\243\316\72\354\264\343\316\73"
> + "\21\361\44\317\74\364\30\145\17\124\146\345\243\317\76\374\134\304\327\77"
>
>  +#ifdef FULL_ICONV
>   "cp1250\0"
>   "windows1250\0"
> - "\0\0"
> -@@ -214,6 +215,7 @@
> - "\0\0\0\0\0\0\0\0\0\15\0\0\0\0\0\0\0\0\0\0\266\0\0\0\0\102\0\220\13\0"
> - "\0\234\2\0\0\0\0\0\0\0\0\244\202\13\0\0\0\0\100\15\0\0\0\0\0\0\0\0\0\0"
> - "\267\0\0\0\0\103\0\240\13\0\0\240\2\0\0\0\0\0\0\0\0\250\62\45\0"
> + "\0\40"
> +@@ -239,6 +240,7 @@
> + "\20\105\163\330\64\324\324\145\315\65\330\144\243\315\66\334\334\145\330\67"
> + "\340\204\43\316\100\344\224\143\316\71\350\244\243\316\72\205\265\343\316\73"
> + "\21\305\203\330\74\364\330\145\317\75\370\344\243\317\76\374\340\65\362\77"
>  +#endif
>
>   "koi8r\0"
> - "\0\0"
> + "\0\40"
> --
> 2.15.0
>
Christian Lamparter Nov. 20, 2017, 6:46 p.m. UTC | #2
Hello,

On Monday, November 20, 2017 8:17:05 AM CET Syrone Wong wrote:
> Hi,
> 
> Any specific reason to introduce these changes?
Actually yes. The LEDE Website prides itself with
"LEDE is actively updated [...]"
<https://lede-project.org/reasons_to_use_lede#security>

a71b46cf fix malloc state corruption when ldso rejects loading a second libc
The Thread on the MUSL ML is "[musl] diffutils crash in malloc".
Judging from the descussion, it can cause programs to misbehave starting with
1.1.17 and 1.1.18.

And there's more:
72656157 fix fgetwc when decoding a character that crosses buffer boundary
c21051e9 prevent fork's errno from being clobbered by atfork handlers
...

+ some UAPI updates, which will be nice to have for 4.14 development.

And Also, Rich made big changes to iconv and it broke the
900-iconv_size_hack.patch.

Regards,
Christian
Karl Palsson Nov. 20, 2017, 7:34 p.m. UTC | #3
Christian Lamparter <chunkeey@gmail.com> wrote:
> Hello,
> 
> On Monday, November 20, 2017 8:17:05 AM CET Syrone Wong wrote:
> > Hi,
> > 
> > Any specific reason to introduce these changes?
> Actually yes. The LEDE Website prides itself with
> "LEDE is actively updated [...]"
> <https://lede-project.org/reasons_to_use_lede#security>

Actively updated is one, "we're going to be standing on the
bleeding edge at all times, you're gonna get cut" is another.
Musl is also making releases quite regularly, it's not even 20
days since 17, and only 30 since 16. Surely musl would be
considering a release too for these issues?

Cheers,
Karl P

> 
> a71b46cf fix malloc state corruption when ldso rejects loading
> a second libc The Thread on the MUSL ML is "[musl] diffutils
> crash in malloc". Judging from the descussion, it can cause
> programs to misbehave starting with 1.1.17 and 1.1.18.
> 
> And there's more:
> 72656157 fix fgetwc when decoding a character that crosses
> buffer boundary c21051e9 prevent fork's errno from being
> clobbered by atfork handlers
> ...
> 
> + some UAPI updates, which will be nice to have for 4.14 development.
> 
> And Also, Rich made big changes to iconv and it broke the
> 900-iconv_size_hack.patch.
> 
> Regards,
> Christian
> 
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
Christian Lamparter Nov. 20, 2017, 9:49 p.m. UTC | #4
On Monday, November 20, 2017 7:34:31 PM CET Karl Palsson wrote:
> Christian Lamparter <chunkeey@gmail.com> wrote:
> > On Monday, November 20, 2017 8:17:05 AM CET Syrone Wong wrote:
> > > Any specific reason to introduce these changes?
> > Actually yes. The LEDE Website prides itself with
> > "LEDE is actively updated [...]"
> > <https://lede-project.org/reasons_to_use_lede#security>
> 
> Actively updated is one, "we're going to be standing on the
> bleeding edge at all times, you're gonna get cut" is another.
> Musl is also making releases quite regularly, it's not even 20
> days since 17, and only 30 since 16.

This is all documented on the musl ML.

musl 1.1.17 was rushed due to this CVE:
<http://www.cvedetails.com/cve/CVE-2017-15650/>

However, the 1.1.17 release had accumulated like 11 months of patches since
the previous 1.1.16 release. There have been several regressions that went 
unnoticed. But this only really came to light after 1.1.17 was already 
released. Because the different projects and people actually started to test
it now. 

In fact, Syrone Wong actually discovered the regression earlier.
<http://lists.infradead.org/pipermail/lede-dev/2017-October/009237.html>
(Koen issued an patch to update to the latest 1.1.16 git head and part of
the later discussion was also CC'd to the musl ML.)
But this wasn't fixed in time for 1.1.17.

However, this regression and two other problems then let to the 1.1.18 release
shortly thereafter. <https://marc.info/?l=musl&m=150860332132027&w=2>

(I think you meant the 1.1.17 and 1.1.18 release, right?
Because the 1.1.16 release is according to:
<http://git.musl-libc.org/cgit/musl> now 11 months old.

> Surely musl would be considering a release too for these issues?
It's not just issues and regressions. It's also UAPI updates and
updates to the LEDE specific iconv patch that was broken due to
Rich reworking the iconv. All these changes already start to pile-up.

Regards,
Christian

> > a71b46cf fix malloc state corruption when ldso rejects loading
> > a second libc The Thread on the MUSL ML is "[musl] diffutils
> > crash in malloc". Judging from the descussion, it can cause
> > programs to misbehave starting with 1.1.17 and 1.1.18.
> > 
> > And there's more:
> > 72656157 fix fgetwc when decoding a character that crosses
> > buffer boundary c21051e9 prevent fork's errno from being
> > clobbered by atfork handlers
> > ...
> > 
> > + some UAPI updates, which will be nice to have for 4.14 development.
> > 
> > And Also, Rich made big changes to iconv and it broke the
> > 900-iconv_size_hack.patch.
> > 
> > Regards,
> > Christian
Koen Vandeputte Nov. 21, 2017, 8:56 a.m. UTC | #5
> This is all documented on the musl ML.
>
> musl 1.1.17 was rushed due to this CVE:
> <http://www.cvedetails.com/cve/CVE-2017-15650/>
>
> However, the 1.1.17 release had accumulated like 11 months of patches since
> the previous 1.1.16 release. There have been several regressions that went
> unnoticed. But this only really came to light after 1.1.17 was already
> released. Because the different projects and people actually started to test
> it now.
>
> In fact, Syrone Wong actually discovered the regression earlier.
> <http://lists.infradead.org/pipermail/lede-dev/2017-October/009237.html>
> (Koen issued an patch to update to the latest 1.1.16 git head and part of
> the later discussion was also CC'd to the musl ML.)
> But this wasn't fixed in time for 1.1.17.
>
> However, this regression and two other problems then let to the 1.1.18 release
> shortly thereafter. <https://marc.info/?l=musl&m=150860332132027&w=2>
Yes,

Main reason in that specific case was mainly for these 2 fixes:

/9e01be6 fix signal masking race in pthread_create with priority (I use 
lots of threading & thread priority in my app) //51bdcdc fix OOB reads in Xbyte_memmem (used by memmem() ) /

fwiw, my 2 cents:

I try to carefully judge whether there's a good reason to bump head or 
not, and I mostly wait for min 48 hrs to let the latest commits soak a bit.

One could argue for backporting only importing changes (which I've done 
in the past), [1]
but it was argued that it's better sometimes to just bump the git head 
instead
as it can take months before a new release is done containing critical 
fixes, which is the reason for the switch to git download [2]

Maybe a balanced solution would be to wait for 1 .. 2  tested-by's 
before pushing these to master?

Bleeding edge /can/ lead to cuts .. and lets all try to ensure it 
doesn't exceed the severity of a papercut,
but thats the main reason why stable branches exist (like 17.01)


Basically,
As long as people bump something to head in a sanely fashion using 
common sense and an educated reason .. we should be fine in the long run.


[1] 
https://git.lede-project.org/?p=source.git;a=commit;h=2912f9f2a2e5997df069d38e20d85ff4cc51acef
[2] 
https://git.lede-project.org/?p=source.git;a=commit;h=a8a5cb9595cd64a48c1cea6a1478c11e022474a9


Koen
Christian Lamparter Nov. 21, 2017, 8:51 p.m. UTC | #6
On Tuesday, November 21, 2017 9:56:12 AM CET Koen Vandeputte wrote:
> > This is all documented on the musl ML.
> >
> > musl 1.1.17 was rushed due to this CVE:
> > <http://www.cvedetails.com/cve/CVE-2017-15650/>
> >
> > However, the 1.1.17 release had accumulated like 11 months of patches since
> > the previous 1.1.16 release. There have been several regressions that went
> > unnoticed. But this only really came to light after 1.1.17 was already
> > released. Because the different projects and people actually started to test
> > it now.
> >
> > In fact, Syrone Wong actually discovered the regression earlier.
> > <http://lists.infradead.org/pipermail/lede-dev/2017-October/009237.html>
> > (Koen issued an patch to update to the latest 1.1.16 git head and part of
> > the later discussion was also CC'd to the musl ML.)
> > But this wasn't fixed in time for 1.1.17.
> >
> > However, this regression and two other problems then let to the 1.1.18 release
> > shortly thereafter. <https://marc.info/?l=musl&m=150860332132027&w=2>
> Yes,
> 
> Main reason in that specific case was mainly for these 2 fixes:
> 
> /9e01be6 fix signal masking race in pthread_create with priority (I use 
> lots of threading & thread priority in my app) //51bdcdc fix OOB reads in Xbyte_memmem (used by memmem() ) /
>
Hm? Are you now talking about something else? I remember seeing these
commits listed in your musl update to 1.1.16+. 
<http://lists.infradead.org/pipermail/lede-dev/2017-October/009234.html>

But Felix did the update musl to 1.1.18 on the 5th (I noticed that he had
a patch queued). This update included the changes you listed, right?

> fwiw, my 2 cents:
>
> I try to carefully judge whether there's a good reason to bump head or 
> not, and I mostly wait for min 48 hrs to let the latest commits soak a bit.
"waiting" around doesn't help in regression cases. I think this
glob-regression is another case in point. The issue was discovered
more than a week before 1.1.17 was released:
<https://www.mail-archive.com/lede-dev@lists.infradead.org/msg09180.html>
and when I posted the request to move to 1.1.17 (again).
But it only received the attention, once people started testing the new 
1.1.17 and did bisections.

> One could argue for backporting only importing changes (which I've done
> in the past), [1]
> but it was argued that it's better sometimes to just bump the git head
> instead
> as it can take months before a new release is done containing critical
> fixes, which is the reason for the switch to git download [2]
> 
> Maybe a balanced solution would be to wait for 1 .. 2  tested-by's 
> before pushing these to master?
Great that you bring this up. So, did you test >this patch< already too?
And if so, what's your verdict? Does it perform as expected on your cns3xxx?
Can you please post a "tested-by" tag then?

As for pushing to master: From most past experience, a patch usually sits
in the patchwork queue for some time (a few days to months). Then one of
the commiter adds it to his/her public staging tree and if all went well, 
the patch moves to master eventually.
 
Regards,
Christian
Koen Vandeputte Nov. 22, 2017, 8:55 a.m. UTC | #7
>> Yes,
>>
>> Main reason in that specific case was mainly for these 2 fixes:
>>
>> /9e01be6 fix signal masking race in pthread_create with priority (I use
>> lots of threading & thread priority in my app) //51bdcdc fix OOB reads in Xbyte_memmem (used by memmem() ) /
>>
> Hm? Are you now talking about something else? I remember seeing these
> commits listed in your musl update to 1.1.16+.
> <http://lists.infradead.org/pipermail/lede-dev/2017-October/009234.html>
>
> But Felix did the update musl to 1.1.18 on the 5th (I noticed that he had
> a patch queued). This update included the changes you listed, right?
>
I was indeed referring to that one, as a mere example where I proposed 
to bump to head, just like you propose now. (got superseded)
http://patchwork.ozlabs.org/patch/823135/

>> fwiw, my 2 cents:
>>
>> I try to carefully judge whether there's a good reason to bump head or
>> not, and I mostly wait for min 48 hrs to let the latest commits soak a bit.
> "waiting" around doesn't help in regression cases. I think this
> glob-regression is another case in point. The issue was discovered
> more than a week before 1.1.17 was released:
> <https://www.mail-archive.com/lede-dev@lists.infradead.org/msg09180.html>
> and when I posted the request to move to 1.1.17 (again).
> But it only received the attention, once people started testing the new
> 1.1.17 and did bisections.
After Syrone's reply concerning the regression, we've immediately 
contacted the committer in a separate mail-thread to get in-depth asap :)
keep in mind the reply mentioned:

"when I update to c10bc61 powerpc{64}: fix MAP_NORESERVE and MAP_LOCKED in mman.h"

Which indicates it was not clear at that time which commit exactly 
caused it.
>
> Maybe a balanced solution would be to wait for 1 .. 2  tested-by's
> before pushing these to master?
> Great that you bring this up. So, did you test >this patch< already too?
> And if so, what's your verdict? Does it perform as expected on your cns3xxx?
> Can you please post a "tested-by" tag then?
I've added it to my daily build yesterday, and it's running on my stress 
setup currently. (9 boards: 6x cns3xxx & 3x imx6, all meshed up)
If all goes well, I'm more that happy to drop a tested-by within the 
next few hours (like I sometimes do for other people's patches too)

Feel free to do the same for mine ;)
>
> As for pushing to master: From most past experience, a patch usually sits
> in the patchwork queue for some time (a few days to months). Then one of
> the commiter adds it to his/her public staging tree and if all went well,
> the patch moves to master eventually.
>   
> Regards,
> Christian

Keep in mind that I'm defending your approach completely, as long as the 
reason to bump is valid (in general) :)

Koen
Koen Vandeputte Nov. 22, 2017, 11:07 a.m. UTC | #8
Tested-by: Koen Vandeputte <koen.vandeputte@ncentric.com>

Targets: cns3xxx, imx6
Kevin Darbyshire-Bryant Nov. 22, 2017, 11:47 a.m. UTC | #9
> On 22 Nov 2017, at 11:07, Koen Vandeputte <koen.vandeputte@ncentric.com> wrote:
> 
> Tested-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> 
> Targets: cns3xxx, imx6
> 
> Also
> 
> Tested-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
> 
> ar71xx

> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
Christian Lamparter Nov. 22, 2017, 7 p.m. UTC | #10
On Wednesday, November 22, 2017 9:55:39 AM CET Koen Vandeputte wrote:
> >> Yes,
> >>
> >> Main reason in that specific case was mainly for these 2 fixes:
> >>
> >> /9e01be6 fix signal masking race in pthread_create with priority (I use
> >> lots of threading & thread priority in my app) //51bdcdc fix OOB reads in Xbyte_memmem (used by memmem() ) /
> >>
> > Hm? Are you now talking about something else? I remember seeing these
> > commits listed in your musl update to 1.1.16+.
> > <http://lists.infradead.org/pipermail/lede-dev/2017-October/009234.html>
> >
> > But Felix did the update musl to 1.1.18 on the 5th (I noticed that he had
> > a patch queued). This update included the changes you listed, right?
> >
> I was indeed referring to that one, as a mere example where I proposed 
> to bump to head, just like you propose now. (got superseded)
> http://patchwork.ozlabs.org/patch/823135/
Ah ok, I misread this at first and got confused. Thank you for taking the
time to explain what was going on here.

In any case it seems like the commits entered Felix's staging tree.
<https://git.lede-project.org/?p=lede/nbd/staging.git>

> > Maybe a balanced solution would be to wait for 1 .. 2  tested-by's
> > before pushing these to master?
> > Great that you bring this up. So, did you test >this patch< already too?
> > And if so, what's your verdict? Does it perform as expected on your cns3xxx?
> > Can you please post a "tested-by" tag then?
> I've added it to my daily build yesterday, and it's running on my stress 
> setup currently. (9 boards: 6x cns3xxx & 3x imx6, all meshed up)
> If all goes well, I'm more that happy to drop a tested-by within the 
> next few hours (like I sometimes do for other people's patches too)
Yes, I saw Kevin and your mail too. Thank you both! :)

> Feel free to do the same for mine ;)
Yes, of course. Please do include me in the "CC:", this way it will
hit my inbox, instead of being delivered to just the Mailinglist folder.

The linux kernel ships with a rather useful perl script called:
get_maintainer.pl. (Yes, it can be a blessing but it can also be a curse)

Long ago, this script required a project maintain a special MAINTAINERS file.
However nowadays, the tool has the ability to extract this information from 
git by looking at the previous commits (Emails from Signed-off-by: Reviewed-by:
and Acked-by: tags). And for the most part, this is compatible with LEDE's 
patch guidelines. <https://lede-project.org/submitting-patches>

For example, for this patch it will generate the following output:

$ get_maintainer.pl 0004-toolchain-musl-update-to-current-HEAD.patch 
Felix Fietkau <nbd@nbd.name> (authored:3/7=43%,added_lines:5/17=29%,removed_lines:5/14=36%,authored:1/2=50%,added_lines:3/54=6%,removed_lines:3/22=14%)
Christian Lamparter <chunkeey@gmail.com> (authored:2/7=29%,added_lines:8/17=47%,removed_lines:5/14=36%,authored:1/2=50%,added_lines:51/54=94%,removed_lines:19/22=86%)
Koen Vandeputte <koen.vandeputte@ncentric.com> (authored:2/7=29%,added_lines:4/17=24%,removed_lines:4/14=29%)

So, this tool has picked you as a reviewer! ;)

If you (or anyone else) want to check it out, I've uploaded this slightly
modified version github gist [removed "linux kernel" references in help texts
and changed top_of_kernel_tree() func to work for the LEDE's root directory.]
<https://gist.github.com/chunkeey/59c4b814243e384c05d89cdef2ced916>
(To download, press "Raw" button)

Is there any interest for this?

Regards,
Christian
diff mbox series

Patch

diff --git a/toolchain/musl/common.mk b/toolchain/musl/common.mk
index 2a61516372..a94a475571 100644
--- a/toolchain/musl/common.mk
+++ b/toolchain/musl/common.mk
@@ -13,8 +13,8 @@  PKG_RELEASE=1
 
 PKG_SOURCE_PROTO:=git
 PKG_SOURCE_SUBDIR:=$(PKG_NAME)-$(PKG_VERSION)
-PKG_SOURCE_VERSION:=eb03bde2f24582874cb72b56c7811bf51da0c817
-PKG_MIRROR_HASH:=150808458007eeb0b977059f36f88127d1a1e80ddb6ad1837b5a63efd2958e34
+PKG_SOURCE_VERSION:=72656157f54c47277b01ec85a6ba7c4084fea6c8
+PKG_MIRROR_HASH:=a3d857c23c94aa96a4ad5f442aaf236e5a189a717273c4e4faf425988d98cd32
 PKG_SOURCE_URL:=git://git.musl-libc.org/musl
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION)-$(PKG_SOURCE_VERSION).tar.xz
 
diff --git a/toolchain/musl/patches/900-iconv_size_hack.patch b/toolchain/musl/patches/900-iconv_size_hack.patch
index db18fceb17..41cff5b033 100644
--- a/toolchain/musl/patches/900-iconv_size_hack.patch
+++ b/toolchain/musl/patches/900-iconv_size_hack.patch
@@ -1,14 +1,14 @@ 
 --- a/src/locale/iconv.c
 +++ b/src/locale/iconv.c
-@@ -39,6 +39,7 @@ static const unsigned char charmaps[] =
+@@ -42,6 +42,7 @@ static const unsigned char charmaps[] =
  "ucs4\0ucs4be\0utf32\0utf32be\0\0\300"
  "ucs4le\0utf32le\0\0\303"
  "ascii\0usascii\0iso646\0iso646us\0\0\307"
 +#ifdef FULL_ICONV
  "eucjp\0\0\320"
  "shiftjis\0sjis\0\0\321"
- "gb18030\0\0\330"
-@@ -46,6 +47,7 @@ static const unsigned char charmaps[] =
+ "iso2022jp\0\0\322"
+@@ -50,6 +51,7 @@ static const unsigned char charmaps[] =
  "gb2312\0\0\332"
  "big5\0bigfive\0cp950\0big5hkscs\0\0\340"
  "euckr\0ksc5601\0ksx1001\0cp949\0\0\350"
@@ -16,7 +16,7 @@ 
  #include "codepages.h"
  ;
  
-@@ -53,6 +55,7 @@ static const unsigned short legacy_chars
+@@ -60,6 +62,7 @@ static const unsigned short legacy_chars
  #include "legacychars.h"
  };
  
@@ -24,45 +24,77 @@ 
  static const unsigned short jis0208[84][94] = {
  #include "jis0208.h"
  };
-@@ -72,6 +75,7 @@ static const unsigned short hkscs[] = {
+@@ -79,6 +82,7 @@ static const unsigned short hkscs[] = {
  static const unsigned short ksc[93][94] = {
  #include "ksc.h"
  };
 +#endif
  
- static int fuzzycmp(const unsigned char *a, const unsigned char *b)
+ static const unsigned short rev_jis[] = {
+ #include "revjis.h"
+@@ -196,6 +200,7 @@ static unsigned legacy_map(const unsigne
+ 	return x < 256 ? x : legacy_chars[x-256];
+ }
+ 
++#ifdef FULL_ICONV
+ static unsigned uni_to_jis(unsigned c)
+ {
+ 	unsigned nel = sizeof rev_jis / sizeof *rev_jis;
+@@ -214,6 +219,7 @@ static unsigned uni_to_jis(unsigned c)
+ 		}
+ 	}
+ }
++#endif
+ 
+ size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restrict out, size_t *restrict outb)
  {
-@@ -224,6 +228,7 @@ size_t iconv(iconv_t cd0, char **restric
+@@ -285,6 +291,7 @@ size_t iconv(iconv_t cd, char **restrict
  				c = ((c-0xd7c0)<<10) + (d-0xdc00);
  			}
  			break;
 +#ifdef FULL_ICONV
  		case SHIFT_JIS:
+ 			if (c < 128) break;
  			if (c-0xa1 <= 0xdf-0xa1) {
- 				c += 0xff61-0xa1;
-@@ -370,6 +375,7 @@ size_t iconv(iconv_t cd0, char **restric
+@@ -476,6 +483,7 @@ size_t iconv(iconv_t cd, char **restrict
  			c = ksc[c][d];
  			if (!c) goto ilseq;
  			break;
 +#endif
  		default:
- 			if (c < 128+type) break;
+ 			if (!c) break;
  			c = legacy_map(map, c);
+@@ -516,6 +524,7 @@ size_t iconv(iconv_t cd, char **restrict
+ 				}
+ 			}
+ 			goto subst;
++#ifdef FULL_ICONV
+ 		case SHIFT_JIS:
+ 			if (c < 128) goto revout;
+ 			if (c == 0xa5) {
+@@ -589,6 +598,7 @@ size_t iconv(iconv_t cd, char **restrict
+ 			*(*out)++ = 'B';
+ 			*outb -= 8;
+ 			break;
++#endif
+ 		case UCS2BE:
+ 		case UCS2LE:
+ 		case UTF_16BE:
 --- a/src/locale/codepages.h
 +++ b/src/locale/codepages.h
-@@ -118,6 +118,7 @@
- "\0\0\0\100\15\0\344\0\0\0\0\0\0\0\0\0\0\0\0\0\103\270\1\0\0\0\340\1\200\40"
- "\230\0\0\0\0\0\44\341\12\0"
+@@ -129,6 +129,7 @@
+ "\340\204\43\316\100\344\34\144\316\71\350\244\243\316\72\354\264\343\316\73"
+ "\21\361\44\317\74\364\30\145\17\124\146\345\243\317\76\374\134\304\327\77"
  
 +#ifdef FULL_ICONV
  "cp1250\0"
  "windows1250\0"
- "\0\0"
-@@ -214,6 +215,7 @@
- "\0\0\0\0\0\0\0\0\0\15\0\0\0\0\0\0\0\0\0\0\266\0\0\0\0\102\0\220\13\0"
- "\0\234\2\0\0\0\0\0\0\0\0\244\202\13\0\0\0\0\100\15\0\0\0\0\0\0\0\0\0\0"
- "\267\0\0\0\0\103\0\240\13\0\0\240\2\0\0\0\0\0\0\0\0\250\62\45\0"
+ "\0\40"
+@@ -239,6 +240,7 @@
+ "\20\105\163\330\64\324\324\145\315\65\330\144\243\315\66\334\334\145\330\67"
+ "\340\204\43\316\100\344\224\143\316\71\350\244\243\316\72\205\265\343\316\73"
+ "\21\305\203\330\74\364\330\145\317\75\370\344\243\317\76\374\340\65\362\77"
 +#endif
  
  "koi8r\0"
- "\0\0"
+ "\0\40"