Bug 10344 - SessionLogoff on a signed connection with an outstanding notify request crashes smbd.
Summary: SessionLogoff on a signed connection with an outstanding notify request crash...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on: 13796
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-24 18:11 UTC by Jeremy Allison
Modified: 2020-08-19 00:26 UTC (History)
2 users (show)

See Also:


Attachments
Work-in-progress. (13.13 KB, patch)
2013-12-24 18:11 UTC, Jeremy Allison
no flags Details
WIP-part2 (11.89 KB, patch)
2014-01-28 01:16 UTC, Jeremy Allison
no flags Details
WIP-part3 (21.46 KB, patch)
2014-01-28 22:55 UTC, Jeremy Allison
no flags Details
git-am fix for master (26.94 KB, patch)
2014-02-28 00:56 UTC, Jeremy Allison
no flags Details
Replacement fix after metze gave it a going-over :-). (57.76 KB, patch)
2014-03-08 01:04 UTC, Jeremy Allison
no flags Details
Proposed patch for master (27.51 KB, patch)
2014-03-11 21:58 UTC, Jeremy Allison
no flags Details
Backport of master patchset to 4.1.x (36.51 KB, patch)
2014-03-13 17:19 UTC, Jeremy Allison
metze: review-
Details
Backport of master patchset to 4.0.x (36.50 KB, patch)
2014-03-13 18:50 UTC, Jeremy Allison
metze: review-
Details
git-am patch for 4.0.x. Backport from master. (37.17 KB, patch)
2014-03-17 19:03 UTC, Jeremy Allison
metze: review-
Details
git-am 4.0.x patchset containing metze's change. (36.99 KB, patch)
2014-03-27 19:15 UTC, Jeremy Allison
metze: review-
Details
git-am 4.1.x patchset containing metze's change. (36.33 KB, patch)
2014-03-27 19:15 UTC, Jeremy Allison
metze: review-
Details
git-am 4.0.x patchset. (36.99 KB, patch)
2014-03-27 21:01 UTC, Jeremy Allison
metze: review+
Details
git-am 4.1.x patchset (36.33 KB, patch)
2014-03-27 21:02 UTC, Jeremy Allison
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2013-12-24 18:11:03 UTC
Created attachment 9547 [details]
Work-in-progress.

Found by Joonas Kuorilehto <joneskoo@codenomicon.com>.

This is because we don't cancel the outstanding notify (or lock) requests, either would cause the crash - before removing the outstanding session data, so when we try and sign the notify requests afterwards smbd crashes.

Fix is to make sessionlogoff and tdis async, and cancel any outstanding requests before replying.

Attached is a prototype patch for the issue on sessionlogoff, along with a test case. Needs expanding to cover async tdis, but I'm attaching here so I don't lose the patch.

Jeremy.
Comment 1 Jeremy Allison 2014-01-28 01:16:17 UTC
Created attachment 9619 [details]
WIP-part2
Comment 2 Jeremy Allison 2014-01-28 22:55:08 UTC
Created attachment 9620 [details]
WIP-part3

Nearly finished, just need to add a torture test for the notify+tcon.
Comment 3 Jeremy Allison 2014-02-28 00:56:23 UTC
Created attachment 9736 [details]
git-am fix for master

Full fix I'm trying to get into master :-).
Comment 4 Jeremy Allison 2014-03-08 01:04:43 UTC
Created attachment 9761 [details]
Replacement fix after metze gave it a going-over :-).
Comment 5 Jeremy Allison 2014-03-11 21:58:32 UTC
Created attachment 9770 [details]
Proposed patch for master
Comment 6 Jeremy Allison 2014-03-13 17:19:39 UTC
Created attachment 9772 [details]
Backport of master patchset to 4.1.x
Comment 7 Jeremy Allison 2014-03-13 18:50:59 UTC
Created attachment 9773 [details]
Backport of master patchset to 4.0.x
Comment 8 Stefan Metzmacher 2014-03-17 12:38:23 UTC
Comment on attachment 9773 [details]
Backport of master patchset to 4.0.x

lib/smbd_tevent_queue.o isn't added to Makefile.in
Comment 9 Jeremy Allison 2014-03-17 19:03:18 UTC
Created attachment 9780 [details]
git-am patch for 4.0.x. Backport from master.

Contains Makefile.in fix Metze requested.
Comment 10 Karolin Seeger 2014-03-25 09:58:40 UTC
(In reply to comment #9)
> Created attachment 9780 [details]
> git-am patch for 4.0.x. Backport from master.
> 
> Contains Makefile.in fix Metze requested.

Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Comment 11 Stefan Metzmacher 2014-03-27 11:03:00 UTC
Comment on attachment 9772 [details]
Backport of master patchset to 4.1.x

source3/lib/smbd_tevent_queue.c should only include <tevent.h> the internal headers are not required
Comment 12 Stefan Metzmacher 2014-03-27 11:03:17 UTC
Comment on attachment 9780 [details]
git-am patch for 4.0.x. Backport from master.

source3/lib/smbd_tevent_queue.c should only include <tevent.h> the internal headers are not required
Comment 13 Jeremy Allison 2014-03-27 19:15:07 UTC
Created attachment 9815 [details]
git-am 4.0.x patchset containing metze's change.
Comment 14 Jeremy Allison 2014-03-27 19:15:39 UTC
Created attachment 9816 [details]
git-am 4.1.x patchset containing metze's change.
Comment 15 Stefan Metzmacher 2014-03-27 20:17:06 UTC
Comment on attachment 9815 [details]
git-am 4.0.x patchset containing metze's change.

<tevent.h> not "tevent.h" please, we may use the system header
Comment 16 Stefan Metzmacher 2014-03-27 20:17:28 UTC
Comment on attachment 9816 [details]
git-am 4.1.x patchset containing metze's change.

<tevent.h> not "tevent.h" please, we may use the system header
Comment 17 Jeremy Allison 2014-03-27 21:01:27 UTC
Created attachment 9817 [details]
git-am 4.0.x patchset.

Done.
Comment 18 Jeremy Allison 2014-03-27 21:02:15 UTC
Created attachment 9818 [details]
git-am 4.1.x patchset

Done.
Comment 19 Stefan Metzmacher 2014-03-27 21:02:58 UTC
Comment on attachment 9817 [details]
git-am 4.0.x patchset.

Looks good, thanks!
Comment 20 Stefan Metzmacher 2014-03-27 21:03:31 UTC
Comment on attachment 9818 [details]
git-am 4.1.x patchset

Looks good, thanks!
Comment 21 Karolin Seeger 2014-04-01 07:25:14 UTC
Pushed new patchsets to autobuilds.
Comment 22 Karolin Seeger 2014-04-04 18:50:19 UTC
Pushed to v4-0-test and v4-1-test.
Closing out bug report.

Thanks!
Comment 23 Stefan Metzmacher 2014-04-11 07:28:18 UTC
Karolin, please revert fc185a5f4cb34f4a2488eb336844c32812f930e7 in v4-0-test,
this somehow went in twice. 87a02403ee4fcc404dc3b887a851c421660cb4d8 is the first commit.

It's not a real problem to have the same check twice, but it's a bit confusing
and may generate problems with future backports.
Comment 24 Karolin Seeger 2014-04-29 08:24:01 UTC
(In reply to comment #23)
> Karolin, please revert fc185a5f4cb34f4a2488eb336844c32812f930e7 in v4-0-test,
> this somehow went in twice. 87a02403ee4fcc404dc3b887a851c421660cb4d8 is the
> first commit.
> 
> It's not a real problem to have the same check twice, but it's a bit confusing
> and may generate problems with future backports.

Pushed to autobuild-v4-0-test.
Comment 25 Karolin Seeger 2014-05-19 10:05:39 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 26 Stefan Metzmacher 2019-04-15 13:58:30 UTC
The patches on bug #13796 are related...