Index | Thread | Search

From:
Florian Obser <florian@openbsd.org>
Subject:
Re: fsck_ffs error handling fixes
To:
tech@openbsd.org
Date:
Tue, 25 Feb 2025 13:52:52 +0100

Download raw body.

Thread
Reads correct to me, OK florian

On 2025-02-25 13:28 +01, Otto Moerbeek <otto@drijf.net> 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);
>

-- 
In my defence, I have been left unsupervised.