Date: Wed, 3 Nov 2021 09:20:57 -0400 From: Rich Felker <dalias@...c.org> To: Marian Buschsieweke <marian.buschsieweke@...u.de> Cc: musl@...ts.openwall.com Subject: Re: [PATCH] fix error handling in getsubopt() 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,',')...
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.