Be more careful when copying response payloads on firewire-core

When faced with bogus config ROM read responses from an audio device
that did not support block requests as advertized, libffado's csr1212
code was able to recover when running on top of raw1394 but corrupted
its config ROM cache when running on top of firewire-core.
http://subversion.ffado.org/ticket/299

While the actual cause was a combination of firmware bug of the device
and flaw in csr1212.c of libffado, the much less graceful behavior when
running on firewire-core was obviously due to libraw1394's
firewire-core backend.  Hence,
  - do not write into the client's buffer if rcode != RCODE_COMPLETE,
  - do not copy more data than the actual response contained.

The latter safeguard is not overly effective though.  The libraw1394 API
has no means to inform a client about the error case that a responder
node sent less bytes than were requested.  (The case that the responder
sent more bytes than requested is covered up by the kernel already.)
Should we synthesize an I/O failure?  Does not sound ideal either.
However, such a size mismatch should never happen; the important part of
this change is the RCODE_COMPLETE check.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

2 files changed, 7 insertions, 9 deletions

diff --git a/src/fw.c b/src/fw.c
index d53a50e..9e942da 100644
--- a/src/fw.c
+++ b/src/fw.c
@@ -303,8 +303,9 @@ handle_device_event(raw1394handle_t handle,
 	case FW_CDEV_EVENT_RESPONSE:
 		rc = u64_to_ptr(u->response.closure);
 
-		if (rc->data != NULL)
-			memcpy(rc->data, u->response.data, rc->length);
+		/* Kernel ensures that u->response.length does not overflow. */
+		if (rc->data != NULL && u->response.rcode == RCODE_COMPLETE)
+			memcpy(rc->data, u->response.data, u->response.length);
 
 		errcode = fw_to_raw1394_errcode(u->response.rcode);
 		tag = rc->tag;
@@ -1058,9 +1059,8 @@ node_id_ok:
 		return -1;
 	}
 
-	closure->data   = out;
-	closure->length = out_length;
-	closure->tag    = tag;
+	closure->data = out;
+	closure->tag  = tag;
 
 	request = (struct fw_cdev_send_request *) handle->buffer;
 	request->tcode      = tcode;
@@ -1239,9 +1239,8 @@ fw_start_phy_packet_write(fw_handle_t handle, quadlet_t data, unsigned long tag)
 		return -1;
 	}
 
-	closure->data   = NULL;
-	closure->length = 0;
-	closure->tag    = tag;
+	closure->data = NULL;
+	closure->tag  = tag;
 
 	send_phy_packet.closure    = ptr_to_u64(closure);
 	send_phy_packet.data[0]    =  be32_to_cpu(data);
diff --git a/src/fw.h b/src/fw.h
index bc374d5..b17d857 100644
--- a/src/fw.h
+++ b/src/fw.h
@@ -69,7 +69,6 @@ struct device {
 
 struct request_closure {
 	void *data;
-	size_t length;
 	unsigned long tag;
 };