Bug 11144 - Memory leak in SMB2 notify handling.
Summary: Memory leak in SMB2 notify handling.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.2.0rc4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-09 17:48 UTC by Jeremy Allison
Modified: 2015-03-27 20:29 UTC (History)
3 users (show)

See Also:


Attachments
Backport from master - bring talloc up to 2.1.2 (12.53 KB, patch)
2015-03-09 18:24 UTC, Jeremy Allison
jra: review? (ira)
metze: review+
Details
Patches for v4-1-test (112.51 KB, patch)
2015-03-10 10:53 UTC, Stefan Metzmacher
jra: review+
Details
Patches for v4-0-test (158.62 KB, patch)
2015-03-10 10:53 UTC, Stefan Metzmacher
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2015-03-09 17:48:00 UTC
Due to a bug in talloc, the code in smbd_smb2_notify_smbreq_destructor() doesn't do what the author intended.

static int smbd_smb2_notify_smbreq_destructor(struct smb_request *smbreq)
{
        struct tevent_req *req = talloc_get_type_abort(smbreq->async_priv,
                                                       struct tevent_req);
        struct smbd_smb2_notify_state *state = tevent_req_data(req,
                                               struct smbd_smb2_notify_state);

        /*
         * Our temporary parent from change_notify_add_request()
         * goes away.
         */
        state->has_request = false;

        /*
         * move it back to its original parent,
         * which means we no longer need the destructor
         * to protect it.
         */
        talloc_steal(smbreq->smb2req, smbreq);
        talloc_set_destructor(smbreq, NULL);

        /*
         * We want to keep smbreq!
         */
        return -1;
}

Existing talloc in 4.0.x, 4.1.x, 4.2.x reparents to the NULL context in this case and leaves the existing destructor in place.

Back-port from master will fix this.
Comment 1 Jeremy Allison 2015-03-09 18:24:26 UTC
Created attachment 10831 [details]
Backport from master - bring talloc up to 2.1.2

Ira - this applies cleanly to 4.2.x.
Comment 2 Jeremy Allison 2015-03-09 18:25:03 UTC
Comment on attachment 10831 [details]
Backport from master - bring talloc up to 2.1.2

Metze, the talloc in 4.1.x is only 2.0.8, in 4.0.x it's 2.0.7 so this patch doesn't apply as-is.

How do you want to handle back-ports for 4.1.x and 4.0.x ?
Comment 3 Stefan Metzmacher 2015-03-10 10:53:01 UTC
Created attachment 10837 [details]
Patches for v4-1-test
Comment 4 Stefan Metzmacher 2015-03-10 10:53:31 UTC
Created attachment 10838 [details]
Patches for v4-0-test
Comment 5 Jeremy Allison 2015-03-11 01:17:32 UTC
Comment on attachment 10837 [details]
Patches for v4-1-test

LGTM.
Comment 6 Jeremy Allison 2015-03-11 01:19:39 UTC
Comment on attachment 10838 [details]
Patches for v4-0-test

LGTM.
Comment 7 Jeremy Allison 2015-03-11 01:20:12 UTC
Re-assigning to Karolin for inclusion in 4.0.next, 4.1.next, 4.2.next.
Comment 8 Karolin Seeger 2015-03-27 20:29:33 UTC
Pushed to v4-[0|1|2]-test.
Closing out bug report.

Thanks!