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.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,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 3043 invoked from network); 27 Aug 2021 15:45:35 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 27 Aug 2021 15:45:35 -0000 Received: (qmail 29749 invoked by uid 550); 27 Aug 2021 15:45:33 -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 29731 invoked from network); 27 Aug 2021 15:45:32 -0000 X-Virus-Scanned: Debian amavisd-new at disroot.org Mime-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1630079118; bh=p2SYXDwu6sPHIMlcT6sEMoRLhsppFvnoM1tktfgD/E4=; h=Cc:Subject:From:To:Date:In-Reply-To; b=DHnoketgGCn/YWeXsBSrVXIIEYoTLc0VPDNDk76z7HEbSa0skT5XIJ6tMQC10zzji VgFkPAbPjSwWhz4MfwLDDgQRU04IhkDuqtYqd1LuJffTAsyrOBSvFiKAA9Ds6bnR8Z JN+KvQs11BIKRuLtFUnKBQPKs1ewe+dsTuoVtZ8TtZJcWUfIHVaVFRbBA/urj5gTTY qYpeMXbweOceo7huJegbbDHLY9QDE+nz1NaLpCCBtqXhu+s+BIk4xVzL9iFhqxtsNc qwEVqsUFFie79FsZMJnJUSUIQF6K40BL67k5SeGjqB10102ItWupFaMk9fazJielkS drUkyoKJeB4UQ== Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Cc: "Alyssa Ross" From: =?utf-8?q?=C3=89rico_Nogueira?= To: Date: Fri, 27 Aug 2021 12:27:36 -0300 Message-Id: In-Reply-To: <20210821085420.474615-4-hi@alyssa.is> Subject: Re: [musl] [PATCH musl 3/3] mntent: fix parsing lines with optional fields On Sat Aug 21, 2021 at 5:54 AM -03, Alyssa Ross wrote: > According to fstab(5), the last two fields are optional, but this > wasn't accepted by Musl. After this change, only the first field is > required, which matches Glibc's behaviour. > > Using sscanf as before, it would have been impossible to differentiate > between 0 fields and 4 fields, because sscanf would have returned 0 in > both cases due to the use of assignment suppression and %n for the > string fields (which is important to avoid copying any strings). So > instead, before calling sscanf, initialize every string to the empty > string, and then we can check which strings are empty afterwards to > know how many fields were matched. Besides typing change noted below, the change sounds reasonable. If you want to fix another issue around mntent, hasmntopts has some troubles too :p > --- > > We could also be stricter about it, and enforce that the first four > fields are present, since the man page says only the last two are > optional. Doing that would be a simple change of checking for the > presence of mnt_opts instead of mnt_fsname at the end of my patch. > > src/misc/mntent.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/misc/mntent.c b/src/misc/mntent.c > index eabb8200..5a68f0b9 100644 > --- a/src/misc/mntent.c > +++ b/src/misc/mntent.c > @@ -21,7 +21,8 @@ int endmntent(FILE *f) > =20 > struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, > int buflen) > { > - int cnt, n[8], use_internal =3D (linebuf =3D=3D SENTINEL); > + int use_internal =3D (linebuf =3D=3D SENTINEL); > + size_t len, i, n[8]; Try avoiding unrelated changes in the commit, since they can introduce subtle bugs. In this case, making n size_t[] instead of int[] will lead to pointer type mismatches in the sscanf call, given that %n expects an int*. I don't know if *scanf guarantees it won't read enough to go past INT_MAX, though, so making a change to size_t[] and using %ln might make sense. Deferring to someone else to answer that. > =20 > mnt->mnt_freq =3D 0; > mnt->mnt_passno =3D 0; > @@ -39,10 +40,14 @@ struct mntent *getmntent_r(FILE *f, struct mntent > *mnt, char *linebuf, int bufle > errno =3D ERANGE; > return 0; > } > - cnt =3D sscanf(linebuf, " %n%*s%n %n%*s%n %n%*s%n %n%*s%n %d %d", > - n, n+1, n+2, n+3, n+4, n+5, n+6, n+7, > - &mnt->mnt_freq, &mnt->mnt_passno); > - } while (cnt < 2 || linebuf[n[0]] =3D=3D '#'); > + > + len =3D strlen(linebuf); > + for (i =3D 0; i < sizeof n / sizeof *n; i++) n[i] =3D len; > + if (sscanf(linebuf, " %n%*s%n %n%*s%n %n%*s%n %n%*s%n %d %d", > + n, n+1, n+2, n+3, n+4, n+5, n+6, n+7, > + &mnt->mnt_freq, &mnt->mnt_passno) =3D=3D EOF && ferror(f)) > + return 0; > + } while (linebuf[n[0]] =3D=3D '#'); > =20 > linebuf[n[1]] =3D 0; > linebuf[n[3]] =3D 0; > @@ -54,6 +60,9 @@ struct mntent *getmntent_r(FILE *f, struct mntent > *mnt, char *linebuf, int bufle > mnt->mnt_type =3D linebuf+n[4]; > mnt->mnt_opts =3D linebuf+n[6]; > =20 > + if (!*mnt->mnt_fsname) > + return 0; > + > return mnt; > } > =20 > -- > 2.32.0