svn commit: r360836 - head/stand/libsa/zfs

[Toomas Soome]

> On 9. May 2020, at 11:23, Ronald Klop <ronald-lists at klop.ws> wrote:
> 
> On Sat, 09 May 2020 09:25:29 +0200, Toomas Soome <tsoome at me.com <mailto:tsoome at me.com>> wrote:
> 
>> 
>> 
>>> On 9. May 2020, at 09:57, Ronald Klop <ronald-lists at klop.ws> wrote:
>>> 
>>> Hi Toomas,
>>> 
>>> Could this fix this issue https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=144234 ?
>>> 
>>> Regards,
>>> Ronald.
>> 
>> 
>> I doubt a bit unless you have GELI encryption or 4kn disk (which we can not boot with BIOS, only with UEFI). That issue was reported 2010 agains 9.0? is it still the case?
>> 
>> rgds,
>> toomas
> 
> 
> Clear answer. I don't use the computer I had this problem with anymore. (It is in the attic somewhere,) And the problem disappeared for me in 2017 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=144234#c33 <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=144234#c33>). But the issue apparently happens for other people in 12.1 still as I read in the replies to the issue.
> 
> Because of the bogus LBA numbers I suspected some memory corruption. But never found further evidence for this.
> 
> Regards,
> Ronald.


Ok, We just need to check such errors case by case. We know pretty well how to debug those, even if the process can be time consuming.

rgds,
toomas

> 
> 
>>> 
>>> 
>>> On Sat, 09 May 2020 08:25:21 +0200, Toomas Soome <tsoome at freebsd.org> wrote:
>>> 
>>>> Author: tsoome
>>>> Date: Sat May  9 06:25:20 2020
>>>> New Revision: 360836
>>>> URL: https://svnweb.freebsd.org/changeset/base/360836
>>>> 
>>>> Log:
>>>> loader: vdev_read() can corrupt memory
>>>> When reading less than sector size but from sector boundary,
>>>> the vdev_read() will read full sector into the provided buffer
>>>> and therefore corrupting memory past buffer end.
>>>> MFC after:	2 days
>>>> 
>>>> Modified:
>>>> head/stand/libsa/zfs/zfs.c
>>>> 
>>>> Modified: head/stand/libsa/zfs/zfs.c
>>>> ==============================================================================
>>>> --- head/stand/libsa/zfs/zfs.c	Sat May  9 05:04:02 2020	(r360835)
>>>> +++ head/stand/libsa/zfs/zfs.c	Sat May  9 06:25:20 2020	(r360836)
>>>> @@ -418,7 +418,7 @@ vdev_read(vdev_t *vdev, void *priv, off_t offset, void
>>>> 		full_sec_size -= secsz;
>>>> 	/* Return of partial sector data requires a bounce buffer. */
>>>> -	if ((head > 0) || do_tail_read) {
>>>> +	if ((head > 0) || do_tail_read || bytes < secsz) {
>>>> 		bouncebuf = malloc(secsz);
>>>> 		if (bouncebuf == NULL) {
>>>> 			printf("vdev_read: out of memory\n");
>>>> @@ -442,14 +442,28 @@ vdev_read(vdev_t *vdev, void *priv, off_t offset, void
>>>> 		outbuf += min(secsz - head, bytes);
>>>> 	}
>>>> -	/* Full data return from read sectors */
>>>> +	/*
>>>> +	 * Full data return from read sectors.
>>>> +	 * Note, there is still corner case where we read
>>>> +	 * from sector boundary, but less than sector size, e.g. reading 512B
>>>> +	 * from 4k sector.
>>>> +	 */
>>>> 	if (full_sec_size > 0) {
>>>> -		res = read(fd, outbuf, full_sec_size);
>>>> -		if (res != full_sec_size) {
>>>> -			ret = EIO;
>>>> -			goto error;
>>>> +		if (bytes < full_sec_size) {
>>>> +			res = read(fd, bouncebuf, secsz);
>>>> +			if (res != secsz) {
>>>> +				ret = EIO;
>>>> +				goto error;
>>>> +			}
>>>> +			memcpy(outbuf, bouncebuf, bytes);
>>>> +		} else {
>>>> +			res = read(fd, outbuf, full_sec_size);
>>>> +			if (res != full_sec_size) {
>>>> +				ret = EIO;
>>>> +				goto error;
>>>> +			}
>>>> +			outbuf += full_sec_size;
>>>> 		}
>>>> -		outbuf += full_sec_size;
>>>> 	}
>>>> 	/* Partial data return from last sector */
>>>> _______________________________________________
>>>> svn-src-all at freebsd.org mailing list
>>>> https://lists.freebsd.org/mailman/listinfo/svn-src-all
>>>> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"