Subject: kern/1352: The sd drivers do not check transfer lengths
To: None <gnats-bugs@NetBSD.ORG>
From: Eduardo Horvath <eeh@btr.com>
List: netbsd-bugs
Date: 08/12/1995 21:40:16
>Number:         1352
>Category:       kern
>Synopsis:       The sd drivers do not check transfer lengths
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 13 01:20:01 1995
>Last-Modified:
>Originator:     Eduardo Horvath
>Organization:
	Over There
>Release:        NetBSD-current 08-08-95
>Environment:
System: NetBSD home 1.0A NetBSD 1.0A (DEBUG) #305: Sat Jul 29 03:37:25 GMT 1995 root@:/home2/src/sys/arch/amiga/compile/DEBUG amiga

>Description:
	The sd drivers do not check to make sure that all transfer requests are a
	a multiple of the device block size.  Since SCSI disks can only read or 
	write a multiple of the device block size, something not necessarily true
	of SCSI tapes.  The sd driver is the most appropiate place.  Since the
	device insists on transfering complete disk blocks this can cause driver
	lockups, vm-faults, or corrupted memory depending on the lower-level
	driver implementation.
>How-To-Repeat:
	This should do it (unless the lower-level driver does the check itself)
	(I suggest running it only in single-user mode):

	#include <fcntl.h>
	main() {
		int fd, err;
		char buffer[27];
		fd = open("/dev/sd0b",O_RDONLY);
		err = read(fd, buffer, sizeof(buffer));
		perror("read");
	}

	You can also try newlfs on some architectures.
>Fix:

Here's one implementation.  You can probably find a better one.
*** /usr/src/sys/scsi/sd.c	Wed Aug  9 13:52:42 1995
--- sd.c	Sat Aug 12 21:38:06 1995
***************
*** 557,562 ****
--- 557,576 ----
  	dev_t dev;
  	struct uio *uio;
  {
+ 	struct sd_softc *sd;
+ 	int i, unit;
+ 
+ 	unit = SDUNIT(dev);
+ 	if (unit >= sdcd.cd_ndevs)
+ 		return ENXIO;
+ 	sd = sdcd.cd_devs[unit];
+ 	if (!sd)
+ 		return ENXIO;
+ 
+ 	for (i = 0; i < uio->uio_iovcnt; i++)
+ 		if ((uio->uio_iov[i].iov_len % sd->sc_dk.dk_label.d_secsize) 
+ 		    != 0)
+ 			return (EFAULT);
  
  	return (physio(sdstrategy, NULL, dev, B_READ, sdminphys, uio));
  }
***************
*** 566,571 ****
--- 580,599 ----
  	dev_t dev;
  	struct uio *uio;
  {
+ 	struct sd_softc *sd;
+ 	int i, unit;
+ 
+ 	unit = SDUNIT(dev);
+ 	if (unit >= sdcd.cd_ndevs)
+ 		return ENXIO;
+ 	sd = sdcd.cd_devs[unit];
+ 	if (!sd)
+ 		return ENXIO;
+ 
+ 	for (i = 0; i < uio->uio_iovcnt; i++)
+ 		if ((uio->uio_iov[i].iov_len % sd->sc_dk.dk_label.d_secsize) 
+ 		    != 0)
+ 			return (EFAULT);
  
  	return (physio(sdstrategy, NULL, dev, B_WRITE, sdminphys, uio));
  }
>Audit-Trail:
>Unformatted: