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 30337 invoked from network); 12 May 2022 21:10:31 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 12 May 2022 21:10:31 -0000 Received: (qmail 19999 invoked by uid 550); 12 May 2022 21:10:29 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 19967 invoked from network); 12 May 2022 21:10:28 -0000 Date: Thu, 12 May 2022 17:10:15 -0400 From: Rich Felker To: Oliver Ford Cc: musl@lists.openwall.com Message-ID: <20220512211014.GL7074@brightrain.aerifal.cx> References: <20210915221155.3977763-1-hi@alyssa.is> <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Thu, May 12, 2022 at 09:58:30PM +0100, Oliver Ford wrote: > On Thu, May 12, 2022 at 3:08 PM Rich Felker wrote: > > > > - There's also an independent bug in hasmntent that was reported a > > long time ago then lost: it will return false positives when one > > mntopt name is a substring of another. strstr is just not the right > > operation here, at least not without added logic to ensure matching > > on a whole option boundary. This is a separate issue that calls for > > a separate patch though, not a blocker on the patch under discussion > > here. > > > Looking at this, the "hasmntopt" function does match options where the > string is part of the option but not the whole option. So the opt "ro" > will correctly match the "ro" (for read-only) option, but also match > an option that contains "symlinkroot". > > The following version of the function keeps the initial strstr, but > adds an extra check so that it doesn't match unless the next character > is either a comma or nul. If there's no other special cases we need to > handle, I'll submit as a > patch? > > char *hasmntopt(const struct mntent *mnt, const char *opt) > { > char *op = strstr(mnt->mnt_opts, opt); > > if (op == NULL) return NULL; > size_t len = strlen(opt); > char *end = op + len; > if (*end == '\0' || *end == ',') return op; > > return NULL; > } 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