* [musl] Re: [PATCH] iconv: Add check null-check for scd pointer [not found] <20240324192503.16512-1-maks.mishinFZ@gmail.com> @ 2024-03-24 19:33 ` Rich Felker 2024-03-25 8:53 ` alice 0 siblings, 1 reply; 4+ messages in thread From: Rich Felker @ 2024-03-24 19:33 UTC (permalink / raw) To: Maks Mishin; +Cc: musl On Sun, Mar 24, 2024 at 10:25:03PM +0300, Maks Mishin wrote: > After having been assigned to a NULL value at iconv.c:230, > pointer 'scd' is dereferenced at iconv.c:383. > > Found by RASU JSC. > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> > --- > src/locale/iconv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/locale/iconv.c b/src/locale/iconv.c > index 7fb2e1ef..e0d200b8 100644 > --- a/src/locale/iconv.c > +++ b/src/locale/iconv.c > @@ -232,6 +232,8 @@ size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restri > scd = (void *)cd; > cd = scd->base_cd; > } > + if (scd == NULL) return x; > + > unsigned to = extract_to(cd); > unsigned from = extract_from(cd); > const unsigned char *map = charmaps+from+1; > -- > 2.30.2 This makes iconv non-functional for non-stateful conversions. The claim by the static analysis tool is false. It is not dereferenced in the code path where it's null because in that code path, type==ISO2022_JP is never true. This tool you are using is really junk. You should stop sending untested and obviously incorrect patches to projects, and advise any projects that have accepted your patches that they may have been dangerously incorrect. Rich ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] Re: [PATCH] iconv: Add check null-check for scd pointer 2024-03-24 19:33 ` [musl] Re: [PATCH] iconv: Add check null-check for scd pointer Rich Felker @ 2024-03-25 8:53 ` alice 2024-04-11 17:56 ` Maks Mishin 0 siblings, 1 reply; 4+ messages in thread From: alice @ 2024-03-25 8:53 UTC (permalink / raw) To: musl, Maks Mishin On Sun Mar 24, 2024 at 7:33 PM UTC, Rich Felker wrote: > On Sun, Mar 24, 2024 at 10:25:03PM +0300, Maks Mishin wrote: > > After having been assigned to a NULL value at iconv.c:230, > > pointer 'scd' is dereferenced at iconv.c:383. > > > > Found by RASU JSC. > > > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> > > --- > > src/locale/iconv.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/src/locale/iconv.c b/src/locale/iconv.c > > index 7fb2e1ef..e0d200b8 100644 > > --- a/src/locale/iconv.c > > +++ b/src/locale/iconv.c > > @@ -232,6 +232,8 @@ size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restri > > scd = (void *)cd; > > cd = scd->base_cd; > > } > > + if (scd == NULL) return x; > > + > > unsigned to = extract_to(cd); > > unsigned from = extract_from(cd); > > const unsigned char *map = charmaps+from+1; > > -- > > 2.30.2 > > This makes iconv non-functional for non-stateful conversions. The > claim by the static analysis tool is false. It is not dereferenced in > the code path where it's null because in that code path, > type==ISO2022_JP is never true. > > This tool you are using is really junk. You should stop sending > untested and obviously incorrect patches to projects, and advise any > projects that have accepted your patches that they may have been > dangerously incorrect. I'm pretty sure RASU JSC is not a tool but rather the Rusatom State Atomic Corporation JSC, i.e. a branch at the Russian atomic energy company. > > Rich ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] Re: [PATCH] iconv: Add check null-check for scd pointer 2024-03-25 8:53 ` alice @ 2024-04-11 17:56 ` Maks Mishin 2024-04-11 18:43 ` Markus Wichmann 0 siblings, 1 reply; 4+ messages in thread From: Maks Mishin @ 2024-04-11 17:56 UTC (permalink / raw) To: alice; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 2052 bytes --] Alice, that's right. Rich, I'm sorry, but it's now always possible to test a particular function. Can you tell me how you are testing the library? This will help me make more meaningful patches. пн, 25 мар. 2024 г. в 11:53, alice <alice@ayaya.dev>: > On Sun Mar 24, 2024 at 7:33 PM UTC, Rich Felker wrote: > > On Sun, Mar 24, 2024 at 10:25:03PM +0300, Maks Mishin wrote: > > > After having been assigned to a NULL value at iconv.c:230, > > > pointer 'scd' is dereferenced at iconv.c:383. > > > > > > Found by RASU JSC. > > > > > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> > > > --- > > > src/locale/iconv.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/src/locale/iconv.c b/src/locale/iconv.c > > > index 7fb2e1ef..e0d200b8 100644 > > > --- a/src/locale/iconv.c > > > +++ b/src/locale/iconv.c > > > @@ -232,6 +232,8 @@ size_t iconv(iconv_t cd, char **restrict in, > size_t *restrict inb, char **restri > > > scd = (void *)cd; > > > cd = scd->base_cd; > > > } > > > + if (scd == NULL) return x; > > > + > > > unsigned to = extract_to(cd); > > > unsigned from = extract_from(cd); > > > const unsigned char *map = charmaps+from+1; > > > -- > > > 2.30.2 > > > > This makes iconv non-functional for non-stateful conversions. The > > claim by the static analysis tool is false. It is not dereferenced in > > the code path where it's null because in that code path, > > type==ISO2022_JP is never true. > > > > This tool you are using is really junk. You should stop sending > > untested and obviously incorrect patches to projects, and advise any > > projects that have accepted your patches that they may have been > > dangerously incorrect. > > I'm pretty sure RASU JSC is not a tool but rather the Rusatom State Atomic > Corporation JSC, i.e. a branch at the Russian atomic energy company. > > > > > Rich > > -- С уважением, Максим Мишин +7 (915) 958-41-07 maks.mishinFZ@gmail.com [-- Attachment #2: Type: text/html, Size: 3135 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] Re: [PATCH] iconv: Add check null-check for scd pointer 2024-04-11 17:56 ` Maks Mishin @ 2024-04-11 18:43 ` Markus Wichmann 0 siblings, 0 replies; 4+ messages in thread From: Markus Wichmann @ 2024-04-11 18:43 UTC (permalink / raw) To: musl Am Thu, Apr 11, 2024 at 08:56:56PM +0300 schrieb Maks Mishin: > Rich, I'm sorry, but it's now always possible to test a particular function. > Can you tell me how you are testing the library? > This will help me make more meaningful patches. You write tests and link them against musl, and then execute them. The most comprehensive test suite is probably libc-test (http://nsz.repo.hu/git/?p=libc-test), but for this it would have been overkill. In case of something like iconv(), I would probably create a utility similar to iconv(1), link it against musl, then write a shell script with a few test cases. If you get the expected output, everything is fine. If not, you broke something. It would be harder to test something like if_nameindex(), which has a different right answer on every given machine. But iconv() does not depend on anything in the system. You just give it input charset, output charset, and an input, and it generates the same output every time, or else it is broken. Ciao, Markus ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-11 18:44 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20240324192503.16512-1-maks.mishinFZ@gmail.com> 2024-03-24 19:33 ` [musl] Re: [PATCH] iconv: Add check null-check for scd pointer Rich Felker 2024-03-25 8:53 ` alice 2024-04-11 17:56 ` Maks Mishin 2024-04-11 18:43 ` Markus Wichmann
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).