Commit 78ed001d authored by Arnd Bergmann's avatar Arnd Bergmann
Browse files

compat: scsi: sg: fix v3 compat read/write interface 



In the v5.4 merge window, a cleanup patch from Al Viro conflicted
with my rework of the compat handling for sg.c read(). Linus Torvalds
did a correct merge but pointed out that the resulting code is still
unsatisfactory.

I later noticed that the sg_new_read() function still gets the compat
mode wrong, when the 'count' argument is large enough to pass a
compat_sg_io_hdr object, but not a nativ sg_io_hdr.

To address both of these, move the definition of compat_sg_io_hdr
into a scsi/sg.h to make it visible to sg.c and rewrite the logic
for reading req_pack_id as well as the size check to a simpler
version that gets the expected results.

Fixes: c35a5cfb ("scsi: sg: sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t")
Fixes: 98aaaec4 ("compat_ioctl: reimplement SG_IO handling")
Reviewed-by: default avatarBen Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
parent 202bf8d7

block/scsi_ioctl.c

+1 −28
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 
#include <scsi/scsi.h>

#include <scsi/scsi_ioctl.h>

#include <scsi/scsi_cmnd.h>

#include <scsi/sg.h>



struct blk_cmd_filter {

	unsigned long read_ok[BLK_SCSI_CMD_PER_LONG];
@@ -550,34 +551,6 @@ static inline int blk_send_start_stop(struct request_queue *q, 
	return __blk_send_generic(q, bd_disk, GPCMD_START_STOP_UNIT, data);

}



#ifdef CONFIG_COMPAT

struct compat_sg_io_hdr {

	compat_int_t interface_id;	/* [i] 'S' for SCSI generic (required) */

	compat_int_t dxfer_direction;	/* [i] data transfer direction  */

	unsigned char cmd_len;		/* [i] SCSI command length ( <= 16 bytes) */

	unsigned char mx_sb_len;	/* [i] max length to write to sbp */

	unsigned short iovec_count;	/* [i] 0 implies no scatter gather */

	compat_uint_t dxfer_len;	/* [i] byte count of data transfer */

	compat_uint_t dxferp;		/* [i], [*io] points to data transfer memory

						or scatter gather list */

	compat_uptr_t cmdp;		/* [i], [*i] points to command to perform */

	compat_uptr_t sbp;		/* [i], [*o] points to sense_buffer memory */

	compat_uint_t timeout;		/* [i] MAX_UINT->no timeout (unit: millisec) */

	compat_uint_t flags;		/* [i] 0 -> default, see SG_FLAG... */

	compat_int_t pack_id;		/* [i->o] unused internally (normally) */

	compat_uptr_t usr_ptr;		/* [i->o] unused internally */

	unsigned char status;		/* [o] scsi status */

	unsigned char masked_status;	/* [o] shifted, masked scsi status */

	unsigned char msg_status;	/* [o] messaging level data (optional) */

	unsigned char sb_len_wr;	/* [o] byte count actually written to sbp */

	unsigned short host_status;	/* [o] errors from host adapter */

	unsigned short driver_status;	/* [o] errors from software driver */

	compat_int_t resid;		/* [o] dxfer_len - actual_transferred */

	compat_uint_t duration;		/* [o] time taken by cmd (unit: millisec) */

	compat_uint_t info;		/* [o] auxiliary information */

};

#endif



int put_sg_io_hdr(const struct sg_io_hdr *hdr, void __user *argp)

{

#ifdef CONFIG_COMPAT

drivers/scsi/sg.c

+59 −67
Original line number Diff line number Diff line
@@ -405,6 +405,38 @@ sg_release(struct inode *inode, struct file *filp) 
	return 0;

}



static int get_sg_io_pack_id(int *pack_id, void __user *buf, size_t count)

{

	struct sg_header __user *old_hdr = buf;

	int reply_len;



	if (count >= SZ_SG_HEADER) {

		/* negative reply_len means v3 format, otherwise v1/v2 */

		if (get_user(reply_len, &old_hdr->reply_len))

			return -EFAULT;



		if (reply_len >= 0)

			return get_user(*pack_id, &old_hdr->pack_id);



		if (in_compat_syscall() &&

		    count >= sizeof(struct compat_sg_io_hdr)) {

			struct compat_sg_io_hdr __user *hp = buf;



			return get_user(*pack_id, &hp->pack_id);

		}



		if (count >= sizeof(struct sg_io_hdr)) {

			struct sg_io_hdr __user *hp = buf;



			return get_user(*pack_id, &hp->pack_id);

		}

	}



	/* no valid header was passed, so ignore the pack_id */

	*pack_id = -1;

	return 0;

}



static ssize_t

sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)

{
@@ -413,8 +445,8 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos) 
	Sg_request *srp;

	int req_pack_id = -1;

	sg_io_hdr_t *hp;

	struct sg_header *old_hdr = NULL;

	int retval = 0;

	struct sg_header *old_hdr;

	int retval;



	/*

	 * This could cause a response to be stranded. Close the associated
@@ -429,79 +461,34 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos) 
	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,

				      "sg_read: count=%d\n", (int) count));



	if (sfp->force_packid && (count >= SZ_SG_HEADER)) {

		old_hdr = memdup_user(buf, SZ_SG_HEADER);

		if (IS_ERR(old_hdr))

			return PTR_ERR(old_hdr);

		if (old_hdr->reply_len < 0) {

			if (count >= SZ_SG_IO_HDR) {

				/*

				 * This is stupid.

				 *

				 * We're copying the whole sg_io_hdr_t from user

				 * space just to get the 'pack_id' field. But the

				 * field is at different offsets for the compat

				 * case, so we'll use "get_sg_io_hdr()" to copy

				 * the whole thing and convert it.

				 *

				 * We could do something like just calculating the

				 * offset based of 'in_compat_syscall()', but the

				 * 'compat_sg_io_hdr' definition is in the wrong

				 * place for that.

				 */

				sg_io_hdr_t *new_hdr;

				new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL);

				if (!new_hdr) {

					retval = -ENOMEM;

					goto free_old_hdr;

				}

				retval = get_sg_io_hdr(new_hdr, buf);

				req_pack_id = new_hdr->pack_id;

				kfree(new_hdr);

				if (retval) {

					retval = -EFAULT;

					goto free_old_hdr;

				}

			}

		} else

			req_pack_id = old_hdr->pack_id;

	}

	if (sfp->force_packid)

		retval = get_sg_io_pack_id(&req_pack_id, buf, count);

	if (retval)

		return retval;



	srp = sg_get_rq_mark(sfp, req_pack_id);

	if (!srp) {		/* now wait on packet to arrive */

		if (atomic_read(&sdp->detaching)) {

			retval = -ENODEV;

			goto free_old_hdr;

		}

		if (filp->f_flags & O_NONBLOCK) {

			retval = -EAGAIN;

			goto free_old_hdr;

		}

		if (atomic_read(&sdp->detaching))

			return -ENODEV;

		if (filp->f_flags & O_NONBLOCK)

			return -EAGAIN;

		retval = wait_event_interruptible(sfp->read_wait,

			(atomic_read(&sdp->detaching) ||

			(srp = sg_get_rq_mark(sfp, req_pack_id))));

		if (atomic_read(&sdp->detaching)) {

			retval = -ENODEV;

			goto free_old_hdr;

		}

		if (retval) {

		if (atomic_read(&sdp->detaching))

			return -ENODEV;

		if (retval)

			/* -ERESTARTSYS as signal hit process */

			goto free_old_hdr;

		}

	}

	if (srp->header.interface_id != '\0') {

		retval = sg_new_read(sfp, buf, count, srp);

		goto free_old_hdr;

			return retval;

	}

	if (srp->header.interface_id != '\0')

		return sg_new_read(sfp, buf, count, srp);



	hp = &srp->header;

	if (old_hdr == NULL) {

		old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL);

		if (! old_hdr) {

			retval = -ENOMEM;

			goto free_old_hdr;

		}

	}

	memset(old_hdr, 0, SZ_SG_HEADER);

	old_hdr = kzalloc(SZ_SG_HEADER, GFP_KERNEL);

	if (!old_hdr)

		return -ENOMEM;



	old_hdr->reply_len = (int) hp->timeout;

	old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */

	old_hdr->pack_id = hp->pack_id;
@@ -575,7 +562,12 @@ sg_new_read(Sg_fd * sfp, char __user *buf, size_t count, Sg_request * srp) 
	int err = 0, err2;

	int len;



	if (count < SZ_SG_IO_HDR) {

	if (in_compat_syscall()) {

		if (count < sizeof(struct compat_sg_io_hdr)) {

			err = -EINVAL;

			goto err_out;

		}

	} else if (count < SZ_SG_IO_HDR) {

		err = -EINVAL;

		goto err_out;

	}

include/scsi/sg.h

+30 −0
Original line number Diff line number Diff line
@@ -68,6 +68,36 @@ typedef struct sg_io_hdr 
    unsigned int info;          /* [o] auxiliary information */

} sg_io_hdr_t;  /* 64 bytes long (on i386) */



#if defined(__KERNEL__)

#include <linux/compat.h>



struct compat_sg_io_hdr {

	compat_int_t interface_id;	/* [i] 'S' for SCSI generic (required) */

	compat_int_t dxfer_direction;	/* [i] data transfer direction  */

	unsigned char cmd_len;		/* [i] SCSI command length ( <= 16 bytes) */

	unsigned char mx_sb_len;	/* [i] max length to write to sbp */

	unsigned short iovec_count;	/* [i] 0 implies no scatter gather */

	compat_uint_t dxfer_len;	/* [i] byte count of data transfer */

	compat_uint_t dxferp;		/* [i], [*io] points to data transfer memory

						or scatter gather list */

	compat_uptr_t cmdp;		/* [i], [*i] points to command to perform */

	compat_uptr_t sbp;		/* [i], [*o] points to sense_buffer memory */

	compat_uint_t timeout;		/* [i] MAX_UINT->no timeout (unit: millisec) */

	compat_uint_t flags;		/* [i] 0 -> default, see SG_FLAG... */

	compat_int_t pack_id;		/* [i->o] unused internally (normally) */

	compat_uptr_t usr_ptr;		/* [i->o] unused internally */

	unsigned char status;		/* [o] scsi status */

	unsigned char masked_status;	/* [o] shifted, masked scsi status */

	unsigned char msg_status;	/* [o] messaging level data (optional) */

	unsigned char sb_len_wr;	/* [o] byte count actually written to sbp */

	unsigned short host_status;	/* [o] errors from host adapter */

	unsigned short driver_status;	/* [o] errors from software driver */

	compat_int_t resid;		/* [o] dxfer_len - actual_transferred */

	compat_uint_t duration;		/* [o] time taken by cmd (unit: millisec) */

	compat_uint_t info;		/* [o] auxiliary information */

};

#endif



#define SG_INTERFACE_ID_ORIG 'S'



/* Use negative values to flag difference from original sg_header structure */

CRA Git | Maintained and supported by SUSTech CRA and CCSE