* [musl] [PATCH] fix error handling in getsubopt()
@ 2021-11-03 12:47 Marian Buschsieweke
2021-11-03 13:20 ` Rich Felker
0 siblings, 1 reply; 2+ messages in thread
From: Marian Buschsieweke @ 2021-11-03 12:47 UTC (permalink / raw)
To: musl; +Cc: Marian Buschsieweke
The man page of getsubopt says
> int getsubopt(char **restrict optionp, char *const *restrict tokens,
> char **restrict valuep);
>
> [...]
> RETURN VALUE
> If the first suboption in optionp is recognized, getsubopt()
> returns the index of the matching suboption element in tokens.
> Otherwise, -1 is returned and *valuep is the entire name[=value]
> string.
This means, *val should be set to the value *opt had upon the call of
getsubopt on failure, but this is not the case. This fixes the behavior.
This fixes a segmentation fault in the option parsing in v4l2-ctl for the
-c parameter. (E.g. v4l2-ctl -c foo=bar will segfault without this.)
---
src/misc/getsubopt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/misc/getsubopt.c b/src/misc/getsubopt.c
index 53ee9573..37cca186 100644
--- a/src/misc/getsubopt.c
+++ b/src/misc/getsubopt.c
@@ -19,5 +19,6 @@ int getsubopt(char **opt, char *const *keys, char **val)
else if (s[l]) continue;
return i;
}
+ *val = s;
return -1;
}
--
2.33.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [musl] [PATCH] fix error handling in getsubopt()
2021-11-03 12:47 [musl] [PATCH] fix error handling in getsubopt() Marian Buschsieweke
@ 2021-11-03 13:20 ` Rich Felker
0 siblings, 0 replies; 2+ messages in thread
From: Rich Felker @ 2021-11-03 13:20 UTC (permalink / raw)
To: Marian Buschsieweke; +Cc: musl
Thanks for sending this to the list.
On Wed, Nov 03, 2021 at 01:47:57PM +0100, Marian Buschsieweke wrote:
> The man page of getsubopt says
>
> > int getsubopt(char **restrict optionp, char *const *restrict tokens,
> > char **restrict valuep);
> >
> > [...]
> > RETURN VALUE
> > If the first suboption in optionp is recognized, getsubopt()
> > returns the index of the matching suboption element in tokens.
> > Otherwise, -1 is returned and *valuep is the entire name[=value]
> > string.
I went looking to replace this with a citation for the actual
specification, since the man page is not the specification for
standards-specified functions, and found that the above seems to be a
glibc extension, and it's not clear whether it meets the requirements
of the spec. On detailed reading, it's not even clear to me that the
nulling out of the ',' and updating of *opt we're doing are okay in
the error case; that's only specified for the case where a matching
key was found.
The (informative, i.e. non-normative) APPLICATION USAGE text in the
spec does mention:
"The value of *valuep when getsubopt() returns -1 is unspecified.
Historical implementations provide various incompatible extensions
to allow an application to access the suboption text that was not
found in the keylistp array."
which is not reassuring. Generally when there were historically
incompatible variants of something, musl does not offer any of them.
> This means, *val should be set to the value *opt had upon the call of
> getsubopt on failure, but this is not the case. This fixes the behavior.
>
> This fixes a segmentation fault in the option parsing in v4l2-ctl for the
> -c parameter. (E.g. v4l2-ctl -c foo=bar will segfault without this.)
I think there's still a discussion to be had about whether the
proposed change (or even the current behavior) is okay, but v4l2-ctl
relying on this is not a good thing, and it should be patched. AFAICT
it looks like they're trying to use getsubopt for something entirely
different from its purpose, where they really just want
strchr(foo,',')...
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-03 13:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 12:47 [musl] [PATCH] fix error handling in getsubopt() Marian Buschsieweke
2021-11-03 13:20 ` Rich Felker
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).