From mboxrd@z Thu Jan  1 00:00:00 1970
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org
X-Spam-Level: 
X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI,
	RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham
	autolearn_force=no version=3.4.4
Received: (qmail 27872 invoked from network); 3 Nov 2021 13:21:13 -0000
Received: from mother.openwall.net (195.42.179.200)
  by inbox.vuxu.org with ESMTPUTF8; 3 Nov 2021 13:21:13 -0000
Received: (qmail 28280 invoked by uid 550); 3 Nov 2021 13:21:11 -0000
Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm
Precedence: bulk
List-Post: <mailto:musl@lists.openwall.com>
List-Help: <mailto:musl-help@lists.openwall.com>
List-Unsubscribe: <mailto:musl-unsubscribe@lists.openwall.com>
List-Subscribe: <mailto:musl-subscribe@lists.openwall.com>
List-ID: <musl.lists.openwall.com>
Reply-To: musl@lists.openwall.com
Received: (qmail 28239 invoked from network); 3 Nov 2021 13:21:10 -0000
Date: Wed, 3 Nov 2021 09:20:57 -0400
From: Rich Felker <dalias@libc.org>
To: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Cc: musl@lists.openwall.com
Message-ID: <20211103132057.GE7074@brightrain.aerifal.cx>
References: <20211103124757.15214-1-marian.buschsieweke@ovgu.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211103124757.15214-1-marian.buschsieweke@ovgu.de>
User-Agent: Mutt/1.5.21 (2010-09-15)
Subject: Re: [musl] [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,',')...