diff mbox series

[1/3] ksmbd: change security id to the one samba used for posix extension

Message ID 20220920132045.5055-1-linkinjeon@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/3] ksmbd: change security id to the one samba used for posix extension | expand

Commit Message

Namjae Jeon Sept. 20, 2022, 1:20 p.m. UTC 
Samba set SIDOWNER and SIDUNIX_GROUP in create posix context and
set SIDUNIX_USER/GROUP in other sids for posix extension.
This patch change security id to the one samba used.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/oplock.c  | 6 +++---
 fs/ksmbd/smb2pdu.c | 4 ++--
 fs/ksmbd/smb2pdu.h | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

Comments

Tom Talpey Sept. 20, 2022, 8:56 p.m. UTC | #1 
On 9/20/2022 9:20 AM, Namjae Jeon wrote:
> Samba set SIDOWNER and SIDUNIX_GROUP in create posix context and
> set SIDUNIX_USER/GROUP in other sids for posix extension.
> This patch change security id to the one samba used.
> 
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>   fs/ksmbd/oplock.c  | 6 +++---
>   fs/ksmbd/smb2pdu.c | 4 ++--
>   fs/ksmbd/smb2pdu.h | 4 ++--
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
> index 2e56dac1fa6e..c26f02086783 100644
> --- a/fs/ksmbd/oplock.c
> +++ b/fs/ksmbd/oplock.c
> @@ -1616,7 +1616,7 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp)
>   	memset(buf, 0, sizeof(struct create_posix_rsp));
>   	buf->ccontext.DataOffset = cpu_to_le16(offsetof
>   			(struct create_posix_rsp, nlink));
> -	buf->ccontext.DataLength = cpu_to_le32(52);
> +	buf->ccontext.DataLength = cpu_to_le32(56);

"56" is a weird thing to code here. Can it be expressed as an
offsetof or some sort of sizeof, for clarity and robustness?

>   	buf->ccontext.NameOffset = cpu_to_le16(offsetof
>   			(struct create_posix_rsp, Name));
>   	buf->ccontext.NameLength = cpu_to_le16(POSIX_CTXT_DATA_LEN);
> @@ -1642,9 +1642,9 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp)
>   	buf->reparse_tag = cpu_to_le32(fp->volatile_id);
>   	buf->mode = cpu_to_le32(inode->i_mode);
>   	id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)),
> -		  SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]);
> +		  SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]);
>   	id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)),
> -		  SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]);
> +		  SIDUNIX_GROUP, (struct smb_sid *)&buf->SidBuffer[28]);

Same comment for "28". offsetof(2 shorts and a sid), right?

>   }
>   
>   /*
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index f33a04e9e458..bc6c7ce17ea8 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -3573,9 +3573,9 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level,
>   		if (d_info->hide_dot_file && d_info->name[0] == '.')
>   			posix_info->DosAttributes |= FILE_ATTRIBUTE_HIDDEN_LE;
>   		id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid),
> -			  SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]);
> +			  SIDUNIX_USER, (struct smb_sid *)&posix_info->SidBuffer[0]);
>   		id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid),
> -			  SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]);
> +			  SIDUNIX_GROUP, (struct smb_sid *)&posix_info->SidBuffer[16]);

And for "16", although now I'm also confused why it's 4 *less* than
before.


>   		memcpy(posix_info->name, conv_name, conv_len);
>   		posix_info->name_len = cpu_to_le32(conv_len);
>   		posix_info->NextEntryOffset = cpu_to_le32(next_entry_offset);
> diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h
> index af455278d005..32c525bf790a 100644
> --- a/fs/ksmbd/smb2pdu.h
> +++ b/fs/ksmbd/smb2pdu.h
> @@ -158,7 +158,7 @@ struct create_posix_rsp {
>   	__le32 nlink;
>   	__le32 reparse_tag;
>   	__le32 mode;
> -	u8 SidBuffer[40];
> +	u8 SidBuffer[44];
>   } __packed;
>   
>   struct smb2_buffer_desc_v1 {
> @@ -439,7 +439,7 @@ struct smb2_posix_info {
>   	__le32 HardLinks;
>   	__le32 ReparseTag;
>   	__le32 Mode;
> -	u8 SidBuffer[40];
> +	u8 SidBuffer[32];

Ok, so it's one buffer, which contains 2 sids? Ick.

>   	__le32 name_len;
>   	u8 name[1];
>   	/*
Namjae Jeon Sept. 20, 2022, 10 p.m. UTC | #2 
2022-09-21 5:56 GMT+09:00, Tom Talpey <tom@talpey.com>:
> On 9/20/2022 9:20 AM, Namjae Jeon wrote:
>> Samba set SIDOWNER and SIDUNIX_GROUP in create posix context and
>> set SIDUNIX_USER/GROUP in other sids for posix extension.
>> This patch change security id to the one samba used.
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   fs/ksmbd/oplock.c  | 6 +++---
>>   fs/ksmbd/smb2pdu.c | 4 ++--
>>   fs/ksmbd/smb2pdu.h | 4 ++--
>>   3 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
>> index 2e56dac1fa6e..c26f02086783 100644
>> --- a/fs/ksmbd/oplock.c
>> +++ b/fs/ksmbd/oplock.c
>> @@ -1616,7 +1616,7 @@ void create_posix_rsp_buf(char *cc, struct
>> ksmbd_file *fp)
>>   	memset(buf, 0, sizeof(struct create_posix_rsp));
>>   	buf->ccontext.DataOffset = cpu_to_le16(offsetof
>>   			(struct create_posix_rsp, nlink));
>> -	buf->ccontext.DataLength = cpu_to_le32(52);
>> +	buf->ccontext.DataLength = cpu_to_le32(56);
>
> "56" is a weird thing to code here. Can it be expressed as an
> offsetof or some sort of sizeof, for clarity and robustness?
I'll add a comment above this code.
>
>>   	buf->ccontext.NameOffset = cpu_to_le16(offsetof
>>   			(struct create_posix_rsp, Name));
>>   	buf->ccontext.NameLength = cpu_to_le16(POSIX_CTXT_DATA_LEN);
>> @@ -1642,9 +1642,9 @@ void create_posix_rsp_buf(char *cc, struct
>> ksmbd_file *fp)
>>   	buf->reparse_tag = cpu_to_le32(fp->volatile_id);
>>   	buf->mode = cpu_to_le32(inode->i_mode);
>>   	id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)),
>> -		  SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]);
>> +		  SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]);
>>   	id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)),
>> -		  SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]);
>> +		  SIDUNIX_GROUP, (struct smb_sid *)&buf->SidBuffer[28]);
>
> Same comment for "28". offsetof(2 shorts and a sid), right?
Ditto.
>
>>   }
>>
>>   /*
>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> index f33a04e9e458..bc6c7ce17ea8 100644
>> --- a/fs/ksmbd/smb2pdu.c
>> +++ b/fs/ksmbd/smb2pdu.c
>> @@ -3573,9 +3573,9 @@ static int smb2_populate_readdir_entry(struct
>> ksmbd_conn *conn, int info_level,
>>   		if (d_info->hide_dot_file && d_info->name[0] == '.')
>>   			posix_info->DosAttributes |= FILE_ATTRIBUTE_HIDDEN_LE;
>>   		id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid),
>> -			  SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]);
>> +			  SIDUNIX_USER, (struct smb_sid *)&posix_info->SidBuffer[0]);
>>   		id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid),
>> -			  SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]);
>> +			  SIDUNIX_GROUP, (struct smb_sid *)&posix_info->SidBuffer[16]);
>
> And for "16", although now I'm also confused why it's 4 *less* than
> before.
Ditto.
>
>
>>   		memcpy(posix_info->name, conv_name, conv_len);
>>   		posix_info->name_len = cpu_to_le32(conv_len);
>>   		posix_info->NextEntryOffset = cpu_to_le32(next_entry_offset);
>> diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h
>> index af455278d005..32c525bf790a 100644
>> --- a/fs/ksmbd/smb2pdu.h
>> +++ b/fs/ksmbd/smb2pdu.h
>> @@ -158,7 +158,7 @@ struct create_posix_rsp {
>>   	__le32 nlink;
>>   	__le32 reparse_tag;
>>   	__le32 mode;
>> -	u8 SidBuffer[40];
>> +	u8 SidBuffer[44];
>>   } __packed;
>>
>>   struct smb2_buffer_desc_v1 {
>> @@ -439,7 +439,7 @@ struct smb2_posix_info {
>>   	__le32 HardLinks;
>>   	__le32 ReparseTag;
>>   	__le32 Mode;
>> -	u8 SidBuffer[40];
>> +	u8 SidBuffer[32];
>
> Ok, so it's one buffer, which contains 2 sids? Ick.
Yes.

Thanks for your review!
>
>>   	__le32 name_len;
>>   	u8 name[1];
>>   	/*
>
diff mbox series

Patch

diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
index 2e56dac1fa6e..c26f02086783 100644
--- a/fs/ksmbd/oplock.c
+++ b/fs/ksmbd/oplock.c
@@ -1616,7 +1616,7 @@  void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp)
 	memset(buf, 0, sizeof(struct create_posix_rsp));
 	buf->ccontext.DataOffset = cpu_to_le16(offsetof
 			(struct create_posix_rsp, nlink));
-	buf->ccontext.DataLength = cpu_to_le32(52);
+	buf->ccontext.DataLength = cpu_to_le32(56);
 	buf->ccontext.NameOffset = cpu_to_le16(offsetof
 			(struct create_posix_rsp, Name));
 	buf->ccontext.NameLength = cpu_to_le16(POSIX_CTXT_DATA_LEN);
@@ -1642,9 +1642,9 @@  void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp)
 	buf->reparse_tag = cpu_to_le32(fp->volatile_id);
 	buf->mode = cpu_to_le32(inode->i_mode);
 	id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)),
-		  SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]);
+		  SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]);
 	id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)),
-		  SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]);
+		  SIDUNIX_GROUP, (struct smb_sid *)&buf->SidBuffer[28]);
 }
 
 /*
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index f33a04e9e458..bc6c7ce17ea8 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -3573,9 +3573,9 @@  static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level,
 		if (d_info->hide_dot_file && d_info->name[0] == '.')
 			posix_info->DosAttributes |= FILE_ATTRIBUTE_HIDDEN_LE;
 		id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid),
-			  SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]);
+			  SIDUNIX_USER, (struct smb_sid *)&posix_info->SidBuffer[0]);
 		id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid),
-			  SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]);
+			  SIDUNIX_GROUP, (struct smb_sid *)&posix_info->SidBuffer[16]);
 		memcpy(posix_info->name, conv_name, conv_len);
 		posix_info->name_len = cpu_to_le32(conv_len);
 		posix_info->NextEntryOffset = cpu_to_le32(next_entry_offset);
diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h
index af455278d005..32c525bf790a 100644
--- a/fs/ksmbd/smb2pdu.h
+++ b/fs/ksmbd/smb2pdu.h
@@ -158,7 +158,7 @@  struct create_posix_rsp {
 	__le32 nlink;
 	__le32 reparse_tag;
 	__le32 mode;
-	u8 SidBuffer[40];
+	u8 SidBuffer[44];
 } __packed;
 
 struct smb2_buffer_desc_v1 {
@@ -439,7 +439,7 @@  struct smb2_posix_info {
 	__le32 HardLinks;
 	__le32 ReparseTag;
 	__le32 Mode;
-	u8 SidBuffer[40];
+	u8 SidBuffer[32];
 	__le32 name_len;
 	u8 name[1];
 	/*