Bug 11819 - The info type SMB2_SETINFO_FS is not been handled
Summary: The info type SMB2_SETINFO_FS is not been handled
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.3.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Stefan Metzmacher
QA Contact: Samba QA Contact
: 9361 (view as bug list)
Depends on:
Reported: 2016-03-31 20:54 UTC by Partha
Modified: 2016-05-02 19:35 UTC (History)
3 users (show)

See Also:

attached the patch to handle the SMB2_SETFS_INFO info type (4.70 KB, patch)
2016-03-31 20:56 UTC, Partha
no flags Details
attached the patch to handle the SMB2_SETFS_INFO info type QUOTA_INFORMATION (7.21 KB, patch)
2016-04-18 01:53 UTC, Partha
no flags Details
Version of your patch I proposed to master. (7.28 KB, patch)
2016-04-26 02:12 UTC, Jeremy Allison
no flags Details
git-am fix for 4.4.next, 4.3.next. (6.76 KB, patch)
2016-04-27 20:43 UTC, Jeremy Allison
uri: review+
packet capture for SMB2 GET INFO and info class 04 with info level 02 (2.93 MB, application/octet-stream)
2016-04-29 19:53 UTC, Partha
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Partha 2016-03-31 20:54:46 UTC
When I tried to set quota(soft limit and hard limit) on a share from Windows2k12R2 Quota management, I  noticed "NT_STATUS_INVALID_PARAMETER" error for the SMB2 setinfo request. 

Attached the patch which fixes this issue.

SAMBA version 4.3.4

The below is the corresponding debug log

[2016/03/25 10:38:21.333815, 10, pid=17987, effective(0, 110000513), real(0, 0)] ../source3/smbd/smb2_setinfo.c:379(smbd_smb2_setinfo_send)
  smbd_smb2_setinfo_send: $Extend/$Quota:$Q:$INDEX_ALLOCATION - fnum 2762575723
[2016/03/25 10:38:21.333824, 10, pid=17987, effective(0, 110000513), real(0, 0)] ../source3/smbd/smb2_server.c:2789(smbd_smb2_request_error_ex)
  smbd_smb2_request_error_ex: idx[1] status[NT_STATUS_INVALID_PARAMETER] || at ../source3/smbd/smb2_setinfo.c:132
[2016/03/25 10:38:21.333831, 10, pid=17987, effective(0, 110000513), real(0, 0)] ../source3/smbd/smb2_server.c:2680(smbd_smb2_request_done_ex)
  smbd_smb2_request_done_ex: idx[1] status[NT_STATUS_INVALID_PARAMETER] body[8] dyn[yes:1] at ../source3/smbd/smb2_server.c:2837

When I looked at the code the

The smbd_smb2_setinfo_send() is handling only the below two info types and its not handling SMB2_0_INFO_FILESYSTEM.

        case 0x01:/* SMB2_SETINFO_FILE */

	case 0x03:/* SMB2_SETINFO_SECURITY */

From the MS-SMB2 spec it looks like there is a specific info type for handling the FILESYSTEM requests as below and in samba this info type is not considered . Is it a known issue or  I am missing anything ? Handling SMB2_0_INFO_FILESYSTEM

The information classes that are supported for setting underlying object store information are listed in section 2.2.39. Documentation for these is provided [MS-FSCC] section 2.5. Requests for information classes not listed in section 2.2.39 but documented in section 2.5 of [MS-FSCC] for Uses of "Set" or "LOCAL" MUST be failed with STATUS_NOT_SUPPORTED. Requests for information classes not documented in section 2.5 of [MS-FSCC] or documented in section 2.5 of [MS-FSCC] for Uses of only "Query" MUST be failed with STATUS_INVALID_INFO_CLASS.

If the object store supports security and the information class is FileFsControlInformation or FileFsObjectIdInformation and Open.GrantedAccess does not include FILE_WRITE_DATA, the server MUST fail the request with STATUS_ACCESS_DENIED.

The server MUST apply the information requested to the underlying object store.<359> If the underlying object store returns an error, the server MUST fail the request with the error code received. Otherwise, the server MUST initialize an SMB2 SET_INFO Response following the syntax given in section 2.2.40. The response MUST then be sent to the client.
Comment 1 Partha 2016-03-31 20:56:43 UTC
Created attachment 11957 [details]
attached the patch to handle the SMB2_SETFS_INFO info type
Comment 2 Partha 2016-03-31 21:08:01 UTC
Jeremy's below comment has been taken care

On Tue, Mar 29, 2016 at 02:33:48PM -0700, Partha Sarathi wrote:
> Thanks Jeremy.
> Please find the updated patch for review.

More changes (sorry). This code:

+        if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) {
+                return NT_STATUS_INVALID_LEVEL;
+        }
+        if (!CAN_WRITE(conn)) {
+                /* Allow POSIX opens. The open path will deny
+                 * any non-readonly opens. */
+                if (info_level != SMB_POSIX_PATH_OPEN) {
+                        return NT_STATUS_DOS(ERRSRV, ERRaccess);
+                }
+        }

isn't needed. It's boilerplate from smbd_do_setfilepathinfo()
and really doesn't apply here.
Comment 3 Christian Ambach 2016-04-01 08:05:21 UTC
This seems to be a duplicate of Bug 9361
Comment 4 Partha 2016-04-04 19:16:28 UTC
(In reply to Partha from comment #1)

Could you please review this patch and push.
Comment 5 Jeremy Allison 2016-04-05 01:42:19 UTC
I haven't forgotten, I'm just a little busy right now dealing with the security issue. Should have some more bandwidth after the 12th.
Comment 6 Partha 2016-04-05 02:08:59 UTC
(In reply to Jeremy Allison from comment #5)

Thanks Jeremy, No issues.
Comment 7 Jeremy Allison 2016-04-13 21:15:38 UTC
*** Bug 9361 has been marked as a duplicate of this bug. ***
Comment 8 Jeremy Allison 2016-04-13 23:58:36 UTC
OK. Sorry for the delay Partha. This patch does the right thing, but needs some more work.

1). Needs to be a git-am format patch containing your 'Signed-off-by:'.

2). It duplicates most of the code in source3/smbd/trans2.c:call_trans2setfsinfo() around the:

                case SMB_FS_QUOTA_INFORMATION:

clause. The correct thing to do here is to abstract this code out
into a new function that takes an incoming fsp pointer and blob,
and call that new function from the case SMB_FS_QUOTA_INFORMATION
clause inside call_trans2setfsinfo() and also from the new case 0x02:/* SMB2_SETINFO_FS */ clause in your patch to smbd_smb2_setinfo_send().

That way we can guarentee the same code path gets done for both the SMB1 and SMB2+ cases.

I am out on vacation and at conferences for the next two weeks, but I'll try and make time to review this if you get it done, or to write it if not.


Comment 9 Partha 2016-04-14 00:20:57 UTC
Thanks Jeremy for the review comments. Sure I will take care of your comments and come with the "git am format patch"
Comment 10 Partha 2016-04-18 01:53:06 UTC
Created attachment 12002 [details]
attached the patch to handle the SMB2_SETFS_INFO info type QUOTA_INFORMATION

Attached the "git format-patch" version patch, please let me know your  comments/feedback.
Comment 11 Jeremy Allison 2016-04-26 02:12:28 UTC
Created attachment 12022 [details]
Version of your patch I proposed to master.
Comment 12 Jeremy Allison 2016-04-27 20:43:18 UTC
Created attachment 12031 [details]
git-am fix for 4.4.next, 4.3.next.

Cherry-pick from master with reference to this bug report added.
Comment 13 Uri Simchoni 2016-04-27 20:57:10 UTC
Comment on attachment 12031 [details]
git-am fix for 4.4.next, 4.3.next.

The patch looks good. Personally I wouldn't classify this as a bug but rather as a lack of feature, hence if I was to adding this code I would not try to back-port.

Jeremy - if you think it should be back-ported please assign to Karonlin.
Comment 14 Uri Simchoni 2016-04-27 21:22:19 UTC
(In reply to Uri Simchoni from comment #13)
OK I can see why this would be a bug - the client has no option of sending this request via SMB1 if it already negotiated SMB2+. So we have this feature which used to work on, say, 3.6.x, and now is not working.

Assigning to Karolin for inclusion in 4.3.next and 4.4.next.
Comment 15 Stefan Metzmacher 2016-04-27 23:59:01 UTC
I think this is a bug as it works with SMB1.

Has anyone noticed that there're more quota related calls
in SMB1, I guess they're all needed in SMB2.

Comment 16 Partha 2016-04-28 01:07:41 UTC
(In reply to Stefan Metzmacher from comment #15)

Thanks Stefan for looking at it. 

The Query Quota fall back to SMB2 Get_info FS_INOF type and SMB2_FS_INFO_06 info level which is already covered as part of calling  "smbd_do_qfsinfo()" as below,

		uint16_t file_info_level;
		char *data = NULL;
		int data_size = 0;
		size_t fixed_portion;

		/* the levels directly map to the passthru levels */
		file_info_level = in_file_info_class + 1000;

		status = smbd_do_qfsinfo(smb2req->xconn, conn, state,
		/* some responses set STATUS_BUFFER_OVERFLOW and return
		   partial, but valid data */


With the current patch we are able to set FS quota and get FS quota/Share Quota. 

The only missing part is "User Quota" TRANSACT_GET_USER_QUOTA_LIST_START, TRANSACT_GET_USER_QUOTA_LIST_CONTINUE and TRANSACT_GET_USER_QUOTA_FOR_SID.  The SMB2 has specific SMB2 Getinfo levels to query user quota. I will post that info levels soon.
Comment 17 Karolin Seeger 2016-04-28 10:35:46 UTC
(In reply to Uri Simchoni from comment #14)
Pushed to autobuild-v4-[4|3]-test.
Comment 18 Karolin Seeger 2016-04-29 09:02:18 UTC
(In reply to Karolin Seeger from comment #17)
Pushed to both branches.
Leaving bug open because of the last comments.
Comment 19 Partha 2016-04-29 19:53:21 UTC
Created attachment 12048 [details]
packet capture for SMB2 GET INFO and info class 04 with info level 02

In the attached packet capture please look at the packet number 1749 and it response saying NT_STATUS_NOT_SUPPORTED which is coming from the below code at smbd_smb2_getinfo_send()

		tevent_req_nterror(req, NT_STATUS_NOT_SUPPORTED);
		return tevent_req_post(req, ev);

		DEBUG(10,("smbd_smb2_getinfo_send: "
			"unknown in_info_type of %u "
			" for file %s\n",
			(unsigned int)in_info_type,
			fsp_str_dbg(fsp) ));

		tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
		return tevent_req_post(req, ev);

	state->status = status;


The info class needs to be supported for handling the user Quota.
Comment 20 Jeremy Allison 2016-04-30 01:40:42 UTC
I know I'm being lazy, but can you do a first cut at a patch for this (so I don't have to :-) ?
Comment 21 Partha 2016-05-02 19:35:22 UTC
(In reply to Jeremy Allison from comment #20)
Sure Jeremy. Let me work on this patch.