Re: pkg/57145: gmake: *** INTERNAL: readdir: Operation not supported. Stop.

[gnats-admin%netbsd.org@localhost]

The following reply was made to PR pkg/57145; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: matthew green <mrg%NetBSD.org@localhost>
Cc: Andrew Cagney <andrew.cagney%gmail.com@localhost>,
	gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Subject: Re: pkg/57145: gmake: *** INTERNAL: readdir: Operation not supported.  Stop.
Date: Sun, 1 Sep 2024 15:36:05 +0000

 This is a multi-part message in MIME format.
 --=_UO/DuVWULAgwAF6z8mwyycn6j85jhood
 Content-Transfer-Encoding: quoted-printable
 
 > Date: Tue, 20 Aug 2024 07:03:34 +0000
 > From: mrg%NetBSD.org@localhost
 >=20
 > this is a gmake bug.  it checks errno after readdir() but readdir() is
 > not expected to set/clear errno for 'end of dir' condition, and thus
 > converts some prior random error into a readdir error.
 
 Not a gmake bug.  readdir is required to preserve errno in the
 end-of-directory case:
 
 DESCRIPTION
 
    [...]
    Applications wishing to check for error situations should set errno
    to 0 before calling readdir().  If errno is set to non-zero on
    return, an error occurred.
 
 RETURN VALUE
 
    [...] When the end of the directory is encountered, a null pointer
    shall be returned and errno is not changed.
 
 POSIX 2024: https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/fu=
 nctions/readdir.html
 SUSv2 1997: https://pubs.opengroup.org/onlinepubs/7908799/xsh/readdir.html
 
 It's been this way for decades and we just screwed it up in the event
 that lseek fails.
 
 The attached patch arranges to preserve errno in the end-of-directory
 case (and in the success case), and to set errno in error cases.
 
 However, I would like to add an automatic test for this.
 
 > Date: Fri, 30 Aug 2024 21:52:34 +0000
 > From: mrg%NetBSD.org@localhost
 >=20
 > we store lseek() into dirp->dd_seek for later usage, and in the case
 > that it returns -1 and sets errno (this PR shows EOPNOTSUPP but most
 > instances i've and others have seen is EINVAL - mine happens on pure
 > FFS obj, not in a chroot.)
 
 Can you share your reproducer for this?  Maybe we can adapt it to an
 atf test.
 
 --=_UO/DuVWULAgwAF6z8mwyycn6j85jhood
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr57145-readdirerrno-v6"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr57145-readdirerrno-v6.patch"
 
 # HG changeset patch
 # User Taylor R Campbell <riastradh%NetBSD.org@localhost>
 # Date 1725116374 0
 #      Sat Aug 31 14:59:34 2024 +0000
 # Branch trunk
 # Node ID f39f4bb8989435150b682bb087c507f41adcaf8d
 # Parent  9aaaa3422f6c6a29e9679ebd97936a4038ca0060
 # EXP-Topic riastradh-pr57145-readdirerrno
 readdir(3): Preserve errno on end-of-directory.
 
 PR pkg/57145: gmake: *** INTERNAL: readdir: Operation not supported.
 Stop.
 
 diff -r 9aaaa3422f6c -r f39f4bb89894 lib/libc/gen/readdir.c
 --- a/lib/libc/gen/readdir.c	Thu Aug 29 13:39:42 2024 +0000
 +++ b/lib/libc/gen/readdir.c	Sat Aug 31 14:59:34 2024 +0000
 @@ -56,35 +56,50 @@ static char sccsid[] =3D "@(#)readdir.c	8.
  struct dirent *
  _readdir_unlocked(DIR *dirp, int skipdeleted)
  {
 +	const int saved_errno =3D errno;
  	struct dirent *dp;
 =20
  	for (;;) {
  		if (dirp->dd_loc >=3D dirp->dd_size) {
  			if (dirp->dd_flags & __DTF_READALL)
 -				return (NULL);
 +				break;
  			dirp->dd_loc =3D 0;
  		}
  		if (dirp->dd_loc =3D=3D 0 && !(dirp->dd_flags & __DTF_READALL)) {
  			dirp->dd_seek =3D lseek(dirp->dd_fd, (off_t)0, SEEK_CUR);
  			dirp->dd_size =3D getdents(dirp->dd_fd,
  			    dirp->dd_buf, (size_t)dirp->dd_len);
 -			if (dirp->dd_size <=3D 0)
 -				return (NULL);
 +			if (dirp->dd_size =3D=3D 0) /* end of directory */
 +				break;
 +			if (dirp->dd_size =3D=3D -1) /* getdents sets errno */
 +				return NULL;
 +			if (dirp->dd_size < 0) { /* paranoia */
 +				errno =3D EIO;
 +				return NULL;
 +			}
  		}
  		dp =3D (struct dirent *)
  		    (void *)(dirp->dd_buf + (size_t)dirp->dd_loc);
 -		if ((intptr_t)dp & _DIRENT_ALIGN(dp))/* bogus pointer check */
 -			return (NULL);
 +		/* bogus pointer check */
 +		if ((intptr_t)dp & _DIRENT_ALIGN(dp)) {
 +			errno =3D EIO;
 +			return NULL;
 +		}
  		/* d_reclen is unsigned; no need to compare it <=3D 0 */
 -		if (dp->d_reclen > dirp->dd_len + 1 - dirp->dd_loc)
 -			return (NULL);
 +		if (dp->d_reclen > dirp->dd_len + 1 - dirp->dd_loc) {
 +			errno =3D EIO;
 +			return NULL;
 +		}
  		dirp->dd_loc +=3D dp->d_reclen;
  		if (dp->d_ino =3D=3D 0 && skipdeleted)
  			continue;
  		if (dp->d_type =3D=3D DT_WHT && (dirp->dd_flags & DTF_HIDEW))
  			continue;
 -		return (dp);
 +		return dp;
  	}
 +
 +	errno =3D saved_errno;
 +	return NULL;
  }
 =20
  struct dirent *
 
 --=_UO/DuVWULAgwAF6z8mwyycn6j85jhood--