Bug 10798 - crash in source3/smbd/notify.c
crash in source3/smbd/notify.c
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services
All Solaris
: P5 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2014-09-06 00:59 UTC by Ken Harris
Modified: 2014-10-13 10:52 UTC (History)
0 users

See Also:

don't sort -1 elements. (672 bytes, text/plain)
2014-09-06 00:59 UTC, Ken Harris
no flags Details
git-am fix for master. (2.54 KB, patch)
2014-09-08 23:22 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ken Harris 2014-09-06 00:59:00 UTC
Created attachment 10260 [details]
don't sort -1 elements.

There is a qsort in notify.c's change_notify_reply() function.

We encountered numerous crashes in that function and after examining some of them, it looked like notify_buf->num_changes was -1 (so, no elements to sort).

It also looks like notify_marshall_changes(), called immediately after qsort, expects/handles when notify_buf->num_changes == -1.

We added protection to avoid calling qsort() when notify_buf->num_changes is less than 1 and the crashes we were experiencing stopped.

Attached is the diff.

Comment 1 Jeremy Allison 2014-09-08 23:22:14 UTC
Created attachment 10266 [details]
git-am fix for master.

Yeah, perfectly correct - we certainly call change_notify_reply() directly after setting notify_buf->num_changes to -1.

As we already have a -1 check inside notify_marshall_changes() I think the best fix is to move the qsort() inside of notify_marshall_changes(), as it's only ever done just before calling notify_marshall_changes().
Comment 2 Jeremy Allison 2014-09-13 00:07:57 UTC
Fix went into master but need back-porting for 4.1.x, 4.0.x - bug me to get this done next week !

Comment 3 Jeremy Allison 2014-10-07 22:14:27 UTC
OK, turns out this code (the timestamp qsort code) never got back-ported to 4.1 or 4.0.

This means the bug only exists in the 4.2.x code, and therefore is already fixed :-).