[PATCH] Don't modify memory reference provided by value

[Swen Schillig]

On Tue, 2018-02-06 at 12:45 +0100, Stefan Metzmacher wrote:
> Hi Swen,
> 
> what problem are you trying to solve here?
> 
> talloc_memdup() is at least wrong here! (The same applies to the
> push_blocking_lock_request() patch).

Sorry, I tried to explain it as good as I can.

The problem is, that memory is accessed after the ownership moved.
...and even a use-after-free situation could occur.

Here's the possible call stack

void reply_write_and_X(struct smb_request *req)
{
.....
	if(numtowrite == 0) {
		nwritten = 0;
	} else {
		if (req->unread_bytes == 0) {
			status = schedule_aio_write_and_X(conn,
						req,	<==== 
						fsp,
						data,
						startpos,
						numtowrite);

...
out:
	if (req->unread_bytes) {		<=== here we access req which was talloc_moved above
		/* writeX failed. drain socket. */
		if (drain_socket(xconn->transport.sock, req->unread_bytes) !=
				req->unread_bytes) {
			smb_panic("failed to drain pending bytes");
		}
		req->unread_bytes = 0;
	}

}

and within schedule_aio_write_and_X we do what's wrong

NTSTATUS schedule_aio_write_and_X(connection_struct *conn,
			      struct smb_request *smbreq,	<=====
			      files_struct *fsp, const char *data,
			      off_t startpos,
			      size_t numtowrite)
{
...
aio_ex->smbreq = talloc_move(aio_ex, &smbreq); <=== zero'ing smbreq is 
						    only having local 
						    effect but the 
						    stealing part
						    is problematic
...
}

Am I missing something ?
...and why is talloc_memdup wrong ?

Cheers Swen