From: Otto Moerbeek <otto@drijf.net>
Subject: Re: fsck_ffs error handling fixes
To: tech@openbsd.org
Date: Tue, 4 Mar 2025 14:41:36 +0100

Hi,

after some more analysis (of ktraces) the pledge error is explained by
the /dev/rsd1e node not having the right major/minor pair.
After fixing that, fsck_ffs worked as expected.

The pledge error was the DIOCGDINFO ioctl failing. I was put a bit on
the wrong foot, because the general pledge error of ioctl not being
allowed is "tty".

Plus you don't really expect that /dev nodes have the wrong contents.
This was likely caused by an upgrade interrupted by force (resetting
the machine) at the end.

So no more fixes needed afaiks.

	-Otto

On Tue, Feb 25, 2025 at 01:28:14PM +0100, Otto Moerbeek wrote:

> Hi,
> 
> fsck_ffs(8) can report faulty errors that boil down to sloppy errors path
> handling.
> 
> With any existing block device (the cwd is essential to reproduce):
> 
> # cd /dev
> # fsck_ffs sd0d 
> (null) (Bad address)(Can't stat (null)
> ...
> 
> Also, if the disklabel cannot be read, exit. Avoids:
> 
> # fsck_ffs -y /tmp
> /tmp: CANNOT READ DISKLABEL
> Segmentation fault
> 
> The report I got
> (https://media.bsd.network/cache/media_attachments/files/114/053/346/165/989/905/original/54ce89b8f6e1c4fb.jpeg)
> also shows a tty pledge violation reported after 
> 
> ** /dev/sd1e (sd1e)" 
> 
> is printed but I could not reproduce.  A review of the code also
> revealed nothing so far. So that remains a mystery.
> 
> At least the reproducable errors are fixed below.
> 
> 	-Otto
> 
> Index: fsck/fsutil.c
> ===================================================================
> RCS file: /home/cvs/src/sbin/fsck/fsutil.c,v
> diff -u -p -r1.24 fsutil.c
> --- fsck/fsutil.c	28 Jun 2019 13:32:43 -0000	1.24
> +++ fsck/fsutil.c	25 Feb 2025 12:04:14 -0000
> @@ -208,6 +208,10 @@ retry:
>  		if (stslash.st_dev == stblock.st_rdev)
>  			hot++;
>  		raw = rawname(newname);
> +		if (raw == NULL) {
> +			printf("Can't get raw name of %s\n", newname);
> +			return (origname);
> +		}
>  		if (stat(raw, &stchar) == -1) {
>  			xperror(raw);
>  			printf("Can't stat %s\n", raw);
> Index: fsck_ffs/setup.c
> ===================================================================
> RCS file: /home/cvs/src/sbin/fsck_ffs/setup.c,v
> diff -u -p -r1.70 setup.c
> --- fsck_ffs/setup.c	3 Feb 2024 18:51:57 -0000	1.70
> +++ fsck_ffs/setup.c	25 Feb 2025 12:04:14 -0000
> @@ -620,8 +620,10 @@ calcsb(char *dev, int devfd, struct fs *
>  		return (0);
>  	}
>  	cp--;
> -	if (lp == NULL)
> +	if (lp == NULL) {
>  		pfatal("%s: CANNOT READ DISKLABEL\n", dev);
> +		return (0);
> +	}
>  	if (isdigit((unsigned char)*cp))
>  		pp = &lp->d_partitions[0];
>  	else
> @@ -661,7 +663,7 @@ getdisklabel(char *s, int fd)
>  	if (ioctl(fd, DIOCGDINFO, (char *)&lab) == -1) {
>  		if (s == NULL)
>  			return (NULL);
> -		pwarn("ioctl (GCINFO): %s\n", strerror(errno));
> +		pwarn("ioctl (CGDINFO): %s\n", strerror(errno));
>  		errexit("%s: can't read disk label\n", s);
>  	}
>  	return (&lab);
>