Re: [PATCH] bsg: allocate response buffer if requested

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/4/19 3:40 PM, Christoph Hellwig wrote:
On Wed, Jan 30, 2019 at 10:44:24AM +0100, Hannes Reinecke wrote:
The 'response' buffer from bsg is mapped onto the SCSI sense buffer,
however after commit 82ed4db499b8 we need to allocate them ourselves
as the bsg queue is _not_ a SCSI queue, and hence the sense buffer
won't be allocated from the scsi stack.

I don't think this is the full story.  Plain old bsg nodes are on SCSI
(or legacy IDE) request queues, so this should be initialized, and
your patch creates a memory leak by overwriting the sense pointer.

bsg-lib nodes aren't on scsi request queues, but they don't use the code
path your patch to start with.

What exactly is the reproducer for this problem?

The problem is here:

static int bsg_scsi_complete_rq(struct request *rq, struct sg_io_v4 *hdr)
{
	struct scsi_request *sreq = scsi_req(rq);
	int ret = 0;

	/*
	 * fill in all the output members
	 */
	hdr->device_status = sreq->result & 0xff;
	hdr->transport_status = host_byte(sreq->result);
	hdr->driver_status = driver_byte(sreq->result);
	hdr->info = 0;
	if (hdr->device_status || hdr->transport_status || hdr->driver_status)
		hdr->info |= SG_INFO_CHECK;
	hdr->response_len = 0;

->	if (sreq->sense_len && hdr->response) {
		int len = min_t(unsigned int, hdr->max_response_len,
					sreq->sense_len);

		if (copy_to_user(uptr64(hdr->response), sreq->sense, len))
			ret = -EFAULT;
		else
			hdr->response_len = len;
	}

This expects the 'response' to be allocated.
Yet nowhere in the block/bsg.c we actually _do_ allocate the 'response' field. And as the header is pretty much copied from userspace we don't really have any control about the contents of the 'response' nor the 'response_len' parameter.

These fields used to be filled by mpt3sas (to hold the sense code), but with commit 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP passthrough") the sense code handling got removed.

Alternatively we might kill 'response' handling altogether, but then that might have an impact on userland.

Cheers,

Hannes
--
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@xxxxxxx			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux