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,
	T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4
Received: (qmail 17945 invoked from network); 14 May 2022 04:24:24 -0000
Received: from mother.openwall.net (195.42.179.200)
  by inbox.vuxu.org with ESMTPUTF8; 14 May 2022 04:24:24 -0000
Received: (qmail 5300 invoked by uid 550); 14 May 2022 04:24:22 -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 5262 invoked from network); 14 May 2022 04:24:21 -0000
Date: Sat, 14 May 2022 00:24:09 -0400
From: Rich Felker <dalias@libc.org>
To: Oliver Ford <ojford@gmail.com>
Cc: musl@lists.openwall.com
Message-ID: <20220514042409.GO7074@brightrain.aerifal.cx>
References: <20210915221155.3977763-4-hi@alyssa.is>
 <20210920042140.GT13220@brightrain.aerifal.cx>
 <20220109031819.GO7074@brightrain.aerifal.cx>
 <878rvj1tut.fsf@alyssa.is>
 <20220113174037.GA7074@brightrain.aerifal.cx>
 <875yqn1n8g.fsf@alyssa.is>
 <20220512140835.GJ7074@brightrain.aerifal.cx>
 <CAGMVOduojZiBk_oy5aUhc3RkiKPZHvSYN=2qXArHPQ34RziX1A@mail.gmail.com>
 <20220512211014.GL7074@brightrain.aerifal.cx>
 <CAGMVOdtX2pw9CeOEmgcqv65zX8q5bC+K5tA8XtHS2X9Zr5wkUA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAGMVOdtX2pw9CeOEmgcqv65zX8q5bC+K5tA8XtHS2X9Zr5wkUA@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Subject: Re: [musl] [PATCH musl v2 3/3] mntent: fix parsing lines with
 optional fields

On Fri, May 13, 2022 at 10:39:17PM +0100, Oliver Ford wrote:
> On Thu, May 12, 2022 at 10:10 PM Rich Felker <dalias@libc.org> wrote:
> > This fails to check that the match is at the start of an option
> > (preceded by a ',' or at the beginning of string) and fails to
> > continue if the first match is a false positive (e.g. "ro" in
> > "symlinkroot,ro"). It's possible to solve this still using strstr in a
> > loop, but it might be easier to just iterate delimiters and strcmp.
> >
> > Also, I'm not sure if hasmntopt is supposed to return a match or not
> > for something like "uid" in "uid=1001"; if so, being followed by '='
> > also needs to be considered valid match.
> >
> > Rich
> 
> Comparing glibc and bionic, they both match when followed by an '='.
> So the below function handles that, and replaces strstr with an
> strncmp and a loop. If this version is ok I'll submit a patch?
> 
> char *hasmntopt(const struct mntent *mnt, const char *opt)
> {
>     char *ptr = mnt->mnt_opts;
>     size_t len = strlen(opt);
> 
>     while (ptr) {
>         char *end = ptr + len;

This has UB unless ptr points to at least len bytes. You can only
evaluate ptr+len or ptr[len] after confirming that. The strncmp is the
most natural (and free) way of confirming it. You could assign end
after the strncmp but I think just using ptr[len] instead of *end
makes it easier.

>         if (!strncmp(ptr, opt, len) &&
>                  (*end == '\0' || *end == ',' || *end == '=')) return ptr;
>         ptr = strchr(ptr, ',');
>         if (ptr) ptr++;
>     }
> 
>     return NULL;

Minor style nit: NULL is generally not used in musl anymore; just 0 is
fine here.

BTW you dropped the list from CC; I re-added it.

Rich