Bug 13667 - Cancelling of SMB2 aio reads and writes returns wrong error NT_STATUS_INTERNAL_ERROR
Summary: Cancelling of SMB2 aio reads and writes returns wrong error NT_STATUS_INTERNA...
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:
Blocks:
 
Reported: 2018-10-26 12:59 UTC by Ralph Böhme
Modified: 2018-11-07 07:44 UTC (History)
3 users (show)

See Also:


Attachments
Patch for 4.8 and 4.9 cherry-picked from master (22.74 KB, patch)
2018-11-03 15:36 UTC, Ralph Böhme
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2018-10-26 12:59:27 UTC
Since ad52dcdf5de6f5f2c2ee156d93ebbb343f39e526 we return NT_STATUS_INTERNAL_ERROR for SMB2 reads and writes that were attempted to be cancelled by the client.

Note that we don't attempt to cancel the IO internally, cf cancel_smb2_aio(). Instead we prepare the request state to be aware of the fact that the request was cancelled.

So in aio_pwrite_smb2_done() when we the "cancelled" condition is met, before ad52dcdf5de6f5f2c2ee156d93ebbb343f39e526 we skipped sending a SMB2 response.

Sine ad52dcdf5de6f5f2c2ee156d93ebbb343f39e526 we send response with an NT_STATUS_INTERNAL_ERROR error code.

I guess both is wrong: the SMB2 response must reflect the result of the internal IO cancellation. As we don't cancel the IO internally, we must simply continue to process the SMB2 request and just ignore the cancel.
Comment 1 Ralph Böhme 2018-11-03 15:36:15 UTC
Created attachment 14564 [details]
Patch for 4.8 and 4.9 cherry-picked from master
Comment 2 Jeremy Allison 2018-11-05 18:47:59 UTC
Comment on attachment 14564 [details]
Patch for 4.8 and 4.9 cherry-picked from master

LGTM for 4.9.next and 4.8.next
Comment 3 Jeremy Allison 2018-11-05 18:48:27 UTC
Re-assigning to Karolin for inclusion in 4.9.next, 4.8.next.
Comment 4 Karolin Seeger 2018-11-06 08:03:10 UTC
(In reply to Jeremy Allison from comment #3)
Pushed to autobuild-v4-{9,8}-test.
Comment 5 Karolin Seeger 2018-11-07 07:44:20 UTC
(In reply to Karolin Seeger from comment #4)
Pushed to both branches.
Closing out bug report.

Thanks!